Fix shading of isorelax dependency (and other msv-core components)

[norrisjeremy]

Is it intended that isorelax is no longer shaded into the final artifact published to Maven Central?

It appears this started with the 6.6.0 release, because net.java.dev.msv:msv-core:2022.7 now depends upon isorelax:isorelax:20030108, unlike the previous net.java.dev.msv:msv-core:2013.6.1 that instead depended upon com.sun.xml.bind.jaxb:isorelax:20090621.

The isorelax:isorelax:20030108 artifact no longer matches the artifactSet include declaration in the POM file here, therefore it is not processed by the relocation declarations, and thus is not shaded in the final artifact that is published to Maven Central.

[norrisjeremy]

Also, I'm not sure if the shading of isorelax should just be blindly changed to isorelax:isorelax:20030108 instead of com.sun.xml.bind.jaxb:isorelax:20090621.

I noticed while browsing Maven Central that the isorelax:isorelax:20030108 does not have any published source code artifact, so a better solution to this may be to manually exclude the isorelax:isorelax:20030108 transitive dependency pulled in by net.java.dev.msv:msv-core:2022.7 and instead add an explicit dependency upon com.sun.xml.bind.jaxb:isorelax:20090621, so that a version of isorelax that includes a source code artifact is what ultimately ends up being shaded into Woodstox.

I can submit a PR if that would help?

[cowtowncoder]

@norrisjeremy I am 99% sure change is unintentional, and based on misunderstanding that the included Java packages were same between artifacts.
However: it also sounds like msv-core release had potential issue: I am not 100% sure if the change was intentional there wrt isorelaxdependency -- it might (trying to avoid ref to Sun-specific release), or it might not be. I don't know.

Be all of that as it may, yes, a PR to make MSV and its dependencies properly shaded would be very helpful -- probably against 6.6 branch so it could go in 6.6.2 (I think), as well as master for upcoming 7.0.0 release.

[cowtowncoder]

Btw, sounds like #193 might be related?

[norrisjeremy]

@cowtowncoder Do you have any sort of CLA that you would need from me to contribute a PR for this? There are no code related changes, it's simply a few changes to the pom.xml.

[norrisjeremy]

Yes, this may well be actual cause of #193.
Was a simple reproducer provided for it that could be used to test and confirm if my proposed changes in #202 would resolve it with 5a26399 reverted?

[norrisjeremy]

Yes, this may well be actual cause of #193. Was a simple reproducer provided for it that could be used to test and confirm if my proposed changes in #202 would resolve it with 5a26399 reverted?

On the other hand, looking at the newer msv-core-2022.7.jar release, it does not define a org.iso_relax.verifier.VerifierFactoryLoader service like the older msv-core-2013.6.1.jar release does, so perhaps 5a26399 shouldn't be reverted even with my proposed changes from #202?

$ jar -tvf msv-core-2013.6.1.jar META-INF/services/
     0 Mon Jun 10 08:24:28 CDT 2013 META-INF/services/
    43 Mon Jun 10 08:24:28 CDT 2013 META-INF/services/org.iso_relax.verifier.VerifierFactoryLoader

$ jar -tvf msv-core-2022.7.jar META-INF/services/
[no output since no services are defined]

[cowtowncoder]

Unfortunately there was no reproducer for #193: one practical problem being that the JDK baseline for building things is JDK 8, and there is no good way (AFAIK) to test module-info before JDK 9 (where JPMS was introduced).
It's not impossible to make work with multi-module build (or separate repo etc), just complicated enough that I don't have time to dig into.
But we did get a developer to try out a build to manually verify work-around.

As to VerifierFactoryLoader... I think that as long as all Woodstox unit tests pass, I'd be ok in assuming SPI information was not necessary; these dependencies are strictly for Woodstox private use.

[cowtowncoder]

@norrisjeremy One thing I did forget to ask -- why does this matter? As in, what problem is being fixed? I was assuming this is due to something breaking (validation not working); exception being thrown or so? But that would be good to include since inclusion of various components is only to support XML Schema / RelaxNG validation and nothing else.

[norrisjeremy]

@norrisjeremy One thing I did forget to ask -- why does this matter? As in, what problem is being fixed? I was assuming this is due to something breaking (validation not working); exception being thrown or so? But that would be good to include since inclusion of various components is only to support XML Schema / RelaxNG validation and nothing else.

We routinely audit various library dependencies that we utilize for our internally built applications and noticed that the 6.6 Woodstox releases no longer included the shaded copy of isorelax.

I also completed an analysis and concluded that not shading isorelax could break at least the following three classes:

1. com.ctc.wstx.msv.RelaxNGSchemaFactory: uses com.sun.msv.reader.trex.ng.RELAXNGReader from msv-core, which in turn uses org.iso_relax.verifier.Schema from isorelax.
2. com.ctc.wstx.msv.W3CSchemaFactory: uses com.sun.msv.reader.xmlschema.XMLSchemaReader from msv-core, which in turn uses org.iso_relax.verifier.Schema from isorelax.
3. com.ctc.wstx.msv.W3CMultiSchemaFactory: uses com.sun.msv.reader.xmlschema.XMLSchemaReader from msv-core, which in turn uses org.iso_relax.verifier.Schema from isorelax.

[cowtowncoder]

Ok that makes sense. I will add another question on PR itself then.

[cowtowncoder]

Merged; I also released Woodstox 6.6.2 with just this one change.

[norrisjeremy]

Merged; I also released Woodstox 6.6.2 with just this one change.

Great, thanks!