Skip to content

Jackson JLink Build Tests and JPMS POM base updates #1

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

Merged
merged 1 commit into from
May 29, 2019
Merged

Jackson JLink Build Tests and JPMS POM base updates #1

merged 1 commit into from
May 29, 2019

Conversation

GedMarc
Copy link
Collaborator

@GedMarc GedMarc commented May 29, 2019

@GedMarc
Copy link
Collaborator Author

GedMarc commented May 29, 2019

Looks like just making the dependencies static in module-info's (to allow custom implementations)
Added in todo's for guice modules and links for examples to complete them,

All Jackson modules build fine - it's the dependencies of the modules:)

Error: Module ion.java not found, required by com.fasterxml.jackson.dataformat.ion

requires static ion.java

Unfortunately this is a one by one excersize,

@GedMarc
Copy link
Collaborator Author

GedMarc commented May 29, 2019

mvn package -Pjlink

@GedMarc
Copy link
Collaborator Author

GedMarc commented May 29, 2019

toolchains.zip

@cowtowncoder cowtowncoder merged commit 1824c3c into FasterXML:master May 29, 2019
@cowtowncoder
Copy link
Member

Excellent, thank you!

Ion-java may be tricky, although I can see if I could contact them (I know or at least some of the authors).
Is it enough for it to have Automatic Module name, or does it need full module-info?

Guice is hopefully bit simpler to tackle. On plus side neither of these jackson modules is super widely used.

@cowtowncoder
Copy link
Member

cowtowncoder commented Jul 16, 2019

Quick question... what was this change meant to do? It does not seem to import on Eclipse (complains about unknown packaging, jlink), and from command-line I think it skips running actual tests...
(yes probably should have asked before merging :) )

@GedMarc
Copy link
Collaborator Author

GedMarc commented Jul 16, 2019

Definitely supposed to do the merging! Let me take another squiz, I run it as mvn package then get a zip file out, let me check if something is a miss

It builds a running JLink JRE artifact (Or supposed to I guess).
It's purpose is to test that all the modules are JPMS compliant - e.g., if it builds, test complete.

@cowtowncoder
Copy link
Member

Ok. Yeah makes sense. I hope I did not botch things too badly; trying to get command-line/maven AND eclipse to work ok. Still not quite sure why Woodstox did not fail before changes.

On Woodstox, I did release 6.0.0.pr1 for bit easier verification of goodness/problems.
And now hoping to really be close to Jackson 2.10.0.pr1. Guice module remains a problem, and quite possibly Hibernate. JAX-RS is an open question as well I suppose. But overall good progress.

@GedMarc
Copy link
Collaborator Author

GedMarc commented Jul 16, 2019

Right...
So Close!

java.lang.module.FindException: Unable to derive module descriptor for \Maven Repository\com\fasterxml\woodstox\woodstox-core\6.0.0-SNAPSHOT\woodstox-core-6.0.0-SNAPSHOT.jar
Caused by: java.lang.module.InvalidModuleDescriptorException: Provider class com.ctc.wstx.shaded.msv-core.datatype.xsd.ngimpl.DataTypeLibraryImpl not in module

You can only "provides" from the module containing the actual class, this one hit the RestEasy project as well when they tried to split them out into their own spi jar -

@GedMarc
Copy link
Collaborator Author

GedMarc commented Jul 17, 2019

oh the shade is removing the module-info from the final packaged jar

@GedMarc
Copy link
Collaborator Author

GedMarc commented Jul 17, 2019

When I build locally I do find the module-info in the packaged artifact,

On maven though, this is the attached artifact - didn't pull the module info through

woodstox-core-6.0.0-20190715.005931-1.zip

@GedMarc
Copy link
Collaborator Author

GedMarc commented Jul 17, 2019

[DEBUG]  module: jackson.compat11test automatic: false
[DEBUG]  module: com.fasterxml.jackson.annotation automatic: false
[DEBUG]  module: com.fasterxml.jackson.core automatic: false
[DEBUG]  module: com.fasterxml.jackson.databind automatic: false
[DEBUG]  module: com.fasterxml.jackson.module.jaxb automatic: false
[DEBUG]  module: java.xml.bind automatic: false
[DEBUG]  module: jakarta.activation automatic: true
[DEBUG]  module: com.fasterxml.jackson.dataformat.avro automatic: false
[DEBUG]  module: avro automatic: true
[DEBUG]  module: jackson.core.asl automatic: true
[DEBUG]  module: jackson.mapper.asl automatic: true
[DEBUG]  module: paranamer automatic: true
[DEBUG]  module: snappy.java automatic: true
[DEBUG]  module: commons.compress automatic: true
[DEBUG]  module: xz automatic: true
[DEBUG]  module: slf4j.api automatic: true
[DEBUG]  module: com.fasterxml.jackson.dataformat.cbor automatic: false
[DEBUG]  module: com.fasterxml.jackson.dataformat.csv automatic: false
[DEBUG]  module: com.fasterxml.jackson.dataformat.javaprop automatic: false
[DEBUG]  module: com.fasterxml.jackson.dataformat.protobuf automatic: false
[DEBUG]  module: protoparser automatic: true
[DEBUG]  module: com.fasterxml.jackson.dataformat.smile automatic: false
[DEBUG]  module: com.fasterxml.jackson.dataformat.xml automatic: false
[DEBUG]  module: org.codehaus.stax2 automatic: false
[DEBUG]  module: com.fasterxml.jackson.dataformat.yaml automatic: false
[DEBUG]  module: org.yaml.snakeyaml automatic: true
[DEBUG]  module: com.ctc.wstx automatic: false
[DEBUG]  module: com.fasterxml.jackson.module.mrbean automatic: false
[DEBUG]  module: org.objectweb.asm automatic: false
[DEBUG]  module: com.fasterxml.jackson.module.afterburner automatic: false
[DEBUG]  module: com.fasterxml.jackson.datatype.joda automatic: false
[DEBUG]  module: joda.time automatic: true
[DEBUG]  module: com.fasterxml.jackson.datatype.guava automatic: false
[DEBUG]  module: guava automatic: true
[DEBUG]  module: junit automatic: true
[DEBUG]  module: hamcrest.core automatic: true

I'll sort out the jakarta.activation requirement - the rest are all the 3rd parties waiting for them to update and these two

[DEBUG]  module: jackson.core.asl automatic: true
[DEBUG]  module: jackson.mapper.asl automatic: true

Otherwise 2.10 is 100% JPMS

Errors presented when building JLink/JMod

 Error: automatic module cannot be used with jlink: snappy.java from .../Maven%20Repository/org/xerial/snappy/snappy-java/1.1.1.3/snappy-java-1.1.1.3.jar

@GedMarc
Copy link
Collaborator Author

GedMarc commented Jul 17, 2019

@cowtowncoder I don't think it's worth having those actual test classes in this, as this test is literally the building of the jlink artifact - the dependencies seem to be the only issue now.

@GedMarc GedMarc mentioned this pull request Jul 17, 2019
@cowtowncoder
Copy link
Member

My thinking was that while building a package with JLink does exercise big part, there are dynamic lookups, initialization, that might fail during runtime. And so very simple unit tests that call actual functionality could catch additional problem cases as well.
A good example might be that of Woodstox doing W3C Schema validation; or Jackson using Mr Bean or Afterburner?

Or is this something where JUnit and/or IDEs can not properly try loading of modules the way that actual applications do?

@cowtowncoder
Copy link
Member

note on [DEBUG] module: jackson.core.asl automatic: true -- this and the other are, I suppose, Jackson 1.x. I don't actually have means to publish new versions of 1.9.x, although even if I had, I'd probably just add Automatic Module name. I think current Avro (1.8.x) dependency brings those in; although in near future Avro 1.9.x actually upgrades to Jackson 2.x. Which is kind of good, but actually slightly problematic due to... compatibility reasons.

cowtowncoder pushed a commit that referenced this pull request Nov 18, 2019
cowtowncoder pushed a commit that referenced this pull request Nov 15, 2020
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

Successfully merging this pull request may close these issues.

2 participants