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

Java 11 and inclusion of dependencies in release jar #622

Closed
andrewthomas299792 opened this issue Mar 2, 2020 · 13 comments · Fixed by #623
Closed

Java 11 and inclusion of dependencies in release jar #622

andrewthomas299792 opened this issue Mar 2, 2020 · 13 comments · Fixed by #623

Comments

@andrewthomas299792
Copy link

Java 11 compilation fails if the same package is visible from more than one module.

The CDK release jars include dependencies whose packages may also be visible from other modules. Before Java 11, this was a nice convenience. Now, if I understand correctly, it seems harmful.

As an example, the packge org.w3c.dom is included in both the JRE system library and the CDK. This leads to the following compilation error:

[javac] import org.w3c.dom.Node;
[javac]        ^^^^^^^^^^^
[javac] The package org.w3c.dom is accessible from more than one module: <unnamed>, java.xml
@johnmay
Copy link
Member

johnmay commented Mar 3, 2020

We could disable the assembly for JDK11.. thoughts?

I already have in other places the --illegal-access work around (otherwise Surefire tests break too)

   <profile>
      <id>jdk11-build-flags</id>
      <activation>
        <jdk>[11,)</jdk>
      </activation>
      <properties>
        <argline>--illegal-access=permit</argline>
      </properties>
    </profile>

@johnmay
Copy link
Member

johnmay commented Mar 3, 2020

In general my experience is there has been little improvement since JDK 8 which is why I still tend use that. Jigsaw/modules kind of just get in the way with existing projects. However it should at least build/work.

@johnmay
Copy link
Member

johnmay commented Mar 3, 2020

Wait I'm confused, is this a build error in your project?

We currently don't have any errors on JDK 8, 11, or 13
https://travis-ci.org/cdk/cdk/builds/654937633?utm_source=github_status&utm_medium=notification

@andrewthomas299792
Copy link
Author

@johnmay - Yes, the redundant inclusion of system library packages in the CDK release jars is causing a build error in my project when built with Java 11.

@johnmay
Copy link
Member

johnmay commented Mar 4, 2020

Is it just import org.w3c.dom.Node?

$ zip -d cdk.jar org/w3c/dom/Node.class

@johnmay
Copy link
Member

johnmay commented Mar 4, 2020

Here is the full dependency tree, I can't workout where it's coming from...

[INFO] org.openscience.cdk:cdk-bundle:jar:2.4-SNAPSHOT
[INFO] +- org.openscience.cdk:cdk-interfaces:jar:2.4-SNAPSHOT:compile
[INFO] |  \- javax.vecmath:vecmath:jar:1.5.2:compile
[INFO] +- org.openscience.cdk:cdk-core:jar:2.4-SNAPSHOT:compile
[INFO] |  \- com.google.guava:guava:jar:17.0:compile
[INFO] +- org.openscience.cdk:cdk-standard:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-atomtype:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-valencycheck:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-diff:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-data:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-ioformats:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-silent:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-isomorphism:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-datadebug:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-io:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-formula:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-dict:jar:2.4-SNAPSHOT:compile
[INFO] |  +- xom:xom:jar:1.3.2:compile
[INFO] |  |  +- xerces:xercesImpl:jar:2.8.0:compile
[INFO] |  |  \- xalan:xalan:jar:2.7.0:compile
[INFO] |  \- xml-apis:xml-apis:jar:1.3.03:compile
[INFO] +- org.openscience.cdk:cdk-pdb:jar:2.4-SNAPSHOT:compile
[INFO] |  \- org.openscience.cdk:cdk-ctab:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-libiocml:jar:2.4-SNAPSHOT:compile
[INFO] |  \- org.xml-cml:cmlxom:jar:3.1:compile
[INFO] |     +- org.xml-cml:euclid:jar:1.0.1:compile
[INFO] |     |  \- org.apache.commons:commons-math:jar:2.2:compile
[INFO] |     +- org.ccil.cowan.tagsoup:tagsoup:jar:1.2:compile
[INFO] |     +- log4j:log4j:jar:1.2.16:compile
[INFO] |     +- joda-time:joda-time:jar:1.6.2:compile
[INFO] |     \- commons-io:commons-io:jar:2.0.1:compile
[INFO] +- org.openscience.cdk:cdk-smiles:jar:2.4-SNAPSHOT:compile
[INFO] |  +- uk.ac.ebi.beam:beam-core:jar:1.3.3:compile
[INFO] |  +- uk.ac.ebi.beam:beam-func:jar:1.3.3:compile
[INFO] |  \- org.hamcrest:hamcrest:jar:2.2:compile
[INFO] +- org.openscience.cdk:cdk-extra:jar:2.4-SNAPSHOT:compile
[INFO] |  \- gov.nist.math:jama:jar:1.0.3:compile
[INFO] +- org.openscience.cdk:cdk-smarts:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-reaction:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-fingerprint:jar:2.4-SNAPSHOT:compile
[INFO] |  \- org.apache.commons:commons-math3:jar:3.1.1:compile
[INFO] +- org.openscience.cdk:cdk-cip:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-signature:jar:2.4-SNAPSHOT:compile
[INFO] |  \- com.github.gilleain.signatures:signatures:jar:1.1:compile
[INFO] +- org.openscience.cdk:cdk-charges:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-forcefield:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-sdg:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-builder3dtools:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-builder3d:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-qsar:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-ionpot:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-qsaratomic:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-qsarbond:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-hash:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-fragment:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-model:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-qsarmolecular:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-qsarcml:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-qsarionpot:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-qsarprotein:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-render:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-renderbasic:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-renderawt:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-renderextra:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-control:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-qm:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-iordf:jar:2.4-SNAPSHOT:compile
[INFO] |  \- org.apache.jena:jena-core:jar:2.7.4:compile
[INFO] |     +- org.apache.jena:jena-iri:jar:0.9.4:compile
[INFO] |     \- org.slf4j:slf4j-api:jar:1.6.4:compile
[INFO] +- org.openscience.cdk:cdk-libiomd:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-pdbcml:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-inchi:jar:2.4-SNAPSHOT:compile
[INFO] |  \- net.sf.jni-inchi:jni-inchi:jar:0.8:compile
[INFO] |     \- net.sf.jnati:jnati-deploy:jar:0.4:compile
[INFO] |        \- net.sf.jnati:jnati-core:jar:0.4:compile
[INFO] +- org.openscience.cdk:cdk-group:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-pcore:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-smsd:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-structgen:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-tautomer:jar:2.4-SNAPSHOT:compile
[INFO] +- org.openscience.cdk:cdk-legacy:jar:2.4-SNAPSHOT:compile
[INFO] |  \- jgrapht:jgrapht:jar:0.6.0:compile
[INFO] +- org.openscience.cdk:cdk-depict:jar:2.4-SNAPSHOT:compile
[INFO] |  +- org.freehep:freehep-graphicsio-svg:jar:2.4:compile
[INFO] |  |  +- org.freehep:freehep-graphics2d:jar:2.4:compile
[INFO] |  |  +- org.freehep:freehep-graphicsio:jar:2.4:compile
[INFO] |  |  |  \- org.freehep:freehep-io:jar:2.2.2:compile
[INFO] |  |  +- org.freehep:freehep-graphicsio-tests:jar:2.4:compile
[INFO] |  |  \- org.freehep:freehep-graphicsbase:jar:2.4:compile
[INFO] |  +- org.freehep:freehep-graphicsio-pdf:jar:2.4:compile
[INFO] |  \- org.freehep:freehep-graphicsio-ps:jar:2.4:compile
[INFO] \- junit:junit:jar:4.13:test
[INFO]    \- org.hamcrest:hamcrest-core:jar:2.2:test

It's coming from xml-apis-1.3.03.jar - now to work out why we included that

@egonw
Copy link
Member

egonw commented Mar 4, 2020

@johnmay, there is 20 years of legacy here. CDK was around when Java did not have XML parsing functionality, and all we had was Xerces. I think have a look at this, if you like.

@johnmay
Copy link
Member

johnmay commented Mar 4, 2020

Nah it's okay... I think we just don't need to xml-apis. Or we don't need it on newer JDK which we can tweak via Maven.

@egonw
Copy link
Member

egonw commented Mar 4, 2020

the following patch does not give compile errors here:

diff --git a/base/dict/pom.xml b/base/dict/pom.xml
index 4a40b977c5..4943c9c6e7 100644
--- a/base/dict/pom.xml
+++ b/base/dict/pom.xml
@@ -19,11 +19,6 @@
             <artifactId>xom</artifactId>
             <version>1.3.2</version>
         </dependency>
-        <dependency>
-            <groupId>xml-apis</groupId>
-            <artifactId>xml-apis</artifactId>
-            <version>1.3.03</version>
-        </dependency>
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>

@andrewthomas299792
Copy link
Author

Is it just import org.w3c.dom.Node?

No. That's just one example compilation error.

If I understand correctly, the inclusion of dependencies in the release jar will cause compilation errors whenever those dependencies includes packages that are visible from more than one module. The classpath is an unnamed module.

I ran into it first with org.w3c.dom, because it was both in a Java system library module and on the classpath from the CDK. I think the same problem could happen with org.xml.sax. And it could happen with Guava and the Apache libraries if they were distributed or put into modules. That may be a likely future.

Candidate solutions include:

  • Do nothing. Require users of the library to remove dependencies that cause compilation trouble -- after figuring out the need.
  • For now, remove just the standard library dependencies from the release jar. In the short-term, these will be the most troublesome.
  • Remove all the dependencies from the release jar. Include only this project. This is fairly common.
  • Ship the release jar as a module, including the dependencies only as implementation details, without exporting them.

@johnmay
Copy link
Member

johnmay commented Mar 4, 2020

Remove all the dependencies from the release jar. Include only this project. This is fairly common.
Ship the release jar as a module, including the dependencies only as implementation details, without exporting them.

The release.jar is primarily for users who don't have a proper build system... and just need something quick to get up and running. We mainly recommend pulling in the library from Maven central using Maven/Gradle/Ant+Ivy in which case this is a non-issue.

How are you building your project?

@andrewthomas299792
Copy link
Author

The release.jar is primarily for users who don't have a proper build system... and just need something quick to get up and running.

I'm using ant. But I think the problem I'm encountering will also be encountered by others trying to both use the release jar and Java 11.

@johnmay
Copy link
Member

johnmay commented Mar 5, 2020

You should use Ivy and pull in cdk-bundle, however you should really think about which features you actually need, for example if you're not using InChI's you don't need cdk-inchi. Essentially you just include the what you need.

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 a pull request may close this issue.

3 participants