Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Java 9+ module info using Moditect #70

Closed
cowtowncoder opened this issue Mar 13, 2019 · 12 comments
Closed

Add Java 9+ module info using Moditect #70

cowtowncoder opened this issue Mar 13, 2019 · 12 comments
Milestone

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Mar 13, 2019

Although there is no urgent need or benefit from making Woodstox use Java 9 or later (currently Java 11), it would be useful for some users if Woodstox did have full Module info settings, above and beyond automatic module name.
This should be possible using Moditect:

https://github.com/moditect/moditect

as long as we build using JDK 8.

@cowtowncoder
Copy link
Member Author

Ok, so looks like there's bit of a snag wrt MSV-core, relaxng converter/validator dependencies, as there's some overlap.

@cowtowncoder
Copy link
Member Author

@GedMarc So, here's another bit of a tough nut: Woodstox itself is simple (and stax2-api should have module-info). But there are a few legacy dependency to old Sun Multi-Schema validator which leads to issues.

I think I also better switch Module Name for 5.3.0, thinking use of maven group/artifact id makes bit more sense than physical Java package names. WDYT?

@GedMarc
Copy link
Contributor

GedMarc commented May 29, 2019

This is probably going to be another of my famous long ones - so just quickly -

The only xerces reference is in Driver class in an enclosed package that isn't used, a quick check the exact class reference in the static void main() is part of jdk now, removing it is 100%
https://stackoverflow.com/questions/3889308/xml-catalog-resolving-resolver-jar-vs-com-sun-org-apache-xml-internal-resolver

@GedMarc
Copy link
Contributor

GedMarc commented May 29, 2019

And Oooo

RelaxngDatatype is actually part of the java.xml suite - the module file is right at the bottom of the commit -

javaee/jaxb-v2@593f1fb

      <dependency>
            <groupId>com.sun.xml.bind.external</groupId>
            <artifactId>relaxng-datatype</artifactId>
            <version>2.3.2</version>
            <scope>compile</scope>
        </dependency>

@GedMarc
Copy link
Contributor

GedMarc commented May 31, 2019

Hey @cowtowncoder
I've tried a few things over the years, and in the process destroyed what my maven central repository views like :) I ended up with root package as the module name, which has led to quite a nice and easy pattern to remember and implement, specially when you start getting to really large scale enterprise applications (using the "lowest module is open" strat (so everything else stays locked except the base app)
: this - https://github.com/GedMarc/JWebMPHomePage/blob/master/src/jre11/java/module-info.java)

This way browsing, implementing and identifying module names and their associated packages is managable, even across 250 odd modules.
I think it comes out nice really.

https://mvnrepository.com/artifact/com.jwebmp.plugins.graphing
https://mvnrepository.com/artifact/com.jwebmp.plugins

@GedMarc
Copy link
Contributor

GedMarc commented May 31, 2019

Now about your tough nut.... And it is a really horrible tough nut.

What route do you want to go? It's a isorelax 1.5 library build so in my mind it can't keep the same maven groupid, also as a long lasting, finalized item. 1.5 classes don't really run on 1.9 and up as well.
I can deploy iso-relax under a new group id, but I can only deploy to com.jwebmp.
Is this ok to do?

Is MSV also going to be restricted to 1.5? I can update your base net.java github project, I can also deploy the updated ones under the same central group, but again that 1.5 :)

Here's the big problem - rngcoverter is actually just an app, and I can't move it across, or rather I'm not sure how to as yet.

In a nut shell, I can port woodstox to work, but without rngconverter app, the data type that you use is 100% there under the new com. that I sent earlier.

Ways forward

  1. Do I send you the code base for iso-relax to place into a good java maven group?
  2. Do you mind removing the converter application from the rn-
  3. I can place them under my maven group and have woodstox reference then for the quick win
  4. The msv code base - When I do the pull requests, it's touching everything, it's dangerous, and I'm honestly not sure xD

@cowtowncoder
Copy link
Member Author

cowtowncoder commented Jun 4, 2019

Hmmh. So... sounds like a complex mess, and not totally surprising considering history. Although then again, since Kohsuke of Jenkins wrote it, in theory it should be possible to get repackaged "official" versions available.

But let's start with rngconverter. So yeah, it is odd, I assumed it was needed for MSV. But none of unit tests fail if I leave it out so not sure what function it was to serve.
I'll comment that out first.

@cowtowncoder
Copy link
Member Author

Now on RelaxngDatatype: looks like jar you point to is repackaged under com.sun.tools.rngdatatype instead of earlier org.relaxng.datatype. So it is not a direct plug-n-play subsitute but would require recompilation of msv-core I think (which is probably what JAXB RI then does... probably just embeds msv-core?).
I am not sure what to do with this.

But back to questions... so, Woodstox is 1.6, and I don't see why MSV could not at least go to that level. If need be I suppose Java 8 would do if that was absolutely required (although I'd have to think of what to do with Jackson 2.10 XML module -- maybe switch default impl to Aalto).

I don't really care/mind what the module name/package is, so com.jwebmp but be fine.
I am not too interested in packaging msv-core, but not sure what other options there are.

@cowtowncoder
Copy link
Member Author

Actually: would solution be as simple as to just wholesale shade msv-core and all its dependencies? That way they would be baked in, and since usage is (I think) fully contained within Woodstox' (well, stax2-api's actually) wrappers, no shaded types need to be exposed (exception types may be observed unless caught, but that should not matter a lot).

@cowtowncoder
Copy link
Member Author

@GedMarc as per #78, now shades the whole MSV to by-pass problem. Now need to go back to creating suitable package info.

@GedMarc
Copy link
Contributor

GedMarc commented Jul 12, 2019

On it bud :)
That is an ingenious solution!

@cowtowncoder cowtowncoder added this to the 6.0.0.pr1 milestone Jul 16, 2019
@cowtowncoder
Copy link
Member Author

Relasing 6.0.0.pr1 (pre-release) just to get bit more time to verify goodness (or lack thereof), to help with Jackson 2.10 XML module. Testing so far hasn't found problems but I don't really trust I test module info quite properly... :)

Earlier 5.3.0 had other minor fixes; also created 5.3 branch just in case there's need to retrofit something there, into non-shaded woodstox-core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants