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

Default output directory for module-info.class should be META-INF/versions/9 #67

Closed
lukehutch opened this issue Oct 1, 2018 · 19 comments
Milestone

Comments

@lukehutch
Copy link
Contributor

lukehutch commented Oct 1, 2018

There are still a lot of old Java tools that choke when they hit a module-info.class file. It may take a long time for this problem to be solved, since old libraries pull in even older libraries in their transitive dependency graph.

META-INF/versions/9 was the location recommended by Uwe Schindler as the maximally backwards-compatible location for the module descriptor, pointed out by Alan Bateman on the jigsaw-dev mailing list recently:

http://mail.openjdk.java.net/pipermail/jigsaw-dev/2018-October/013940.html

Some tools (such as the Android toolchain) are being updated to ignore classes found in META-INF/versions for this reason:

https://issuetracker.google.com/u/0/issues/77587908

I tried using <outputDirectory> to specify where the module descriptor would be saved, but that affects the output jarfiles, not the location of module-info.class. Is there a way to save the module descriptor anywhere other than the root of the build target? If so, it would be a good idea to have the default be target/META-INF/versions/9, at least for the foreseeable future.

@gunnarmorling
Copy link
Member

Can you try specifying the jvmVersion option as described here? This should cause the descriptor to end up under META-INF/VERSIONS/... as you suggest.

@lukehutch
Copy link
Contributor Author

lukehutch commented Oct 1, 2018

@gunnarmorling Sorry, I totally missed that option! But I just tried it, and the descriptor was still put in the root of the build. I have:

<plugin>
	<groupId>org.moditect</groupId>
	<artifactId>moditect-maven-plugin</artifactId>
	<version>1.0.0.Beta1</version>
	<executions>
		<execution>
			<id>add-module-infos</id>
			<phase>package</phase>
			<goals>
				<goal>add-module-info</goal>
			</goals>
			<configuration>
			        <jvmVersion>9</jvmVersion>
				<overwriteExistingFiles>true</overwriteExistingFiles>
				<module>
					<moduleInfoFile>
						src/moditect/module-info.java
					</moduleInfoFile>
				</module>
			</configuration>
		</execution>
	</executions>
</plugin>

@lukehutch
Copy link
Contributor Author

PS the docs are a bit hard to parse right now... this is not the first time I missed something important in the docs. I don't really know how I would format them differently though...

PPS Based on Alan's recommendation, maybe the default for jvmVersion should be 9? That actually always works, because (1) multi-release jars are supported on all JVMs that support JPMS; and (2) the version resolution order is the following:

http://mail.openjdk.java.net/pipermail/jigsaw-dev/2018-September/013935.html

This means that putting the module descriptor in META-INF/versions/9 has the maximal chance of working with all future JDK versions, while staying out of the way of JDK 8 and earlier, by not polluting the "base layer" (the standard package root).

@lukehutch
Copy link
Contributor Author

And actually the jvmVersion option is never shown in the debug logs for the ModiTect plugin:

[INFO] --- moditect-maven-plugin:1.0.0.Beta1:add-module-info (add-module-infos) @ classgraph ---
[DEBUG] Dependency collection stats: {ConflictMarker.analyzeTime=31311, ConflictMarker.markTime=23721, ConflictMarker.nodeCount=6, ConflictIdSorter.graphTime=11140, ConflictIdSorter.topsortTime=19021, ConflictIdSorter.conflictIdCount=6, ConflictIdSorter.conflictIdCycleCount=0, ConflictResolver.totalTime=180725, ConflictResolver.conflictItemCount=6, DefaultDependencyCollector.collectTime=6688860, DefaultDependencyCollector.transformTime=379780}
[DEBUG] org.moditect:moditect-maven-plugin:jar:1.0.0.Beta1:
[DEBUG]    org.eclipse.aether:aether-util:jar:1.0.2.v20150114:compile
[DEBUG]       org.eclipse.aether:aether-api:jar:1.0.2.v20150114:compile
[DEBUG]    org.moditect:moditect:jar:1.0.0.Beta1:compile
[DEBUG]       com.github.javaparser:javaparser-core:jar:3.5.0:compile
[DEBUG]       com.beust:jcommander:jar:1.60:compile
[DEBUG]    org.codehaus.plexus:plexus-utils:jar:1.1:runtime
[DEBUG] Created new class realm plugin>org.moditect:moditect-maven-plugin:1.0.0.Beta1
[DEBUG] Importing foreign packages into class realm plugin>org.moditect:moditect-maven-plugin:1.0.0.Beta1
[DEBUG]   Imported:  < project>io.github.classgraph:classgraph:4.2.6-SNAPSHOT
[DEBUG] Populating class realm plugin>org.moditect:moditect-maven-plugin:1.0.0.Beta1
[DEBUG]   Included: org.moditect:moditect-maven-plugin:jar:1.0.0.Beta1
[DEBUG]   Included: org.eclipse.aether:aether-util:jar:1.0.2.v20150114
[DEBUG]   Included: org.moditect:moditect:jar:1.0.0.Beta1
[DEBUG]   Included: com.github.javaparser:javaparser-core:jar:3.5.0
[DEBUG]   Included: com.beust:jcommander:jar:1.60
[DEBUG]   Included: org.codehaus.plexus:plexus-utils:jar:1.1
[DEBUG] Configuring mojo org.moditect:moditect-maven-plugin:1.0.0.Beta1:add-module-info from plugin realm ClassRealm[plugin>org.moditect:moditect-maven-plugin:1.0.0.Beta1, parent: jdk.internal.loader.ClassLoaders$AppClassLoader@799f7e29]
[DEBUG] Configuring mojo 'org.moditect:moditect-maven-plugin:1.0.0.Beta1:add-module-info' with basic configurator -->
[DEBUG]   (f) artifactId = classgraph
[DEBUG]   (f) buildDirectory = /home/luke/Work/classgraph/target
[DEBUG]   (s) moduleInfoFile = /home/luke/Work/classgraph/src/moditect/module-info.java
[DEBUG]   (f) module = MainModuleConfiguration [ moduleInfo=null, moduleInfoFile=/home/luke/Work/classgraph/src/moditect/module-info.java, moduleInfoSource=null, mainClass=null]
[DEBUG]   (f) outputDirectory = /home/luke/Work/classgraph/target/modules
[DEBUG]   (f) overwriteExistingFiles = true
[DEBUG]   (f) project = MavenProject: io.github.classgraph:classgraph:4.2.6-SNAPSHOT @ /home/luke/Work/classgraph/pom.xml
[DEBUG]   (f) remoteRepos = [jboss (https://repository.jboss.org/, default, releases), central (http://jcenter.bintray.com, default, releases)]
[DEBUG]   (f) repoSession = org.eclipse.aether.DefaultRepositorySystemSession@5d9d8e46
[DEBUG]   (f) version = 4.2.6-SNAPSHOT
[DEBUG]   (f) workingDirectory = /home/luke/Work/classgraph/target/moditect
[DEBUG] -- end configuration --```

@gunnarmorling
Copy link
Member

gunnarmorling commented Oct 1, 2018 via email

@lukehutch
Copy link
Contributor Author

Ah, OK. No worries, I'll wait. Please consider the suggestion to default this option to 9 though. I know it seems weird on the surface to put just that one file into a version directory, but that provides maximum compatibility.

@gunnarmorling
Copy link
Member

Hum, hum. It seems a bit weird on first thought indeed, but perhaps it's really best compat-wise. The question then is how to use that option to put the file into root if really wanted. Perhaps <jvmVersion>NONE</jvmVersion>?

@gunnarmorling
Copy link
Member

PS the docs are a bit hard to parse right now... this is not the first time I missed something important in the docs. I don't really know how I would format them differently though...

I hear you. Open for any suggestions for re-organizing them. Or better yet, a PR :)

@lukehutch
Copy link
Contributor Author

lukehutch commented Oct 1, 2018

Yes, I think NONE is fine.

The issues I have hit with a few users of my library (and I have hit it myself) is that older classpath scanners will throw an NPE when they run into the module descriptor. It's not necissarily true that older scanners will ignore classfiles in META-INF, but the thinking is that that's the best chance you have at putting classfiles somewhere that an older scanner might intentionally not scan. (And some scanners ignore classfiles whose packages don't match their path, which hopefully happens before the NPE or similar is thrown).

@lukehutch
Copy link
Contributor Author

Plus it's the location of the classfile we're talking about, not the Java source, so it doesn't matter if the target location is not where most humans would expect it to be, as long as it is the most robust location.

@gunnarmorling
Copy link
Member

gunnarmorling commented Oct 1, 2018 via email

@gunnarmorling
Copy link
Member

Resolved via #68.

@gunnarmorling gunnarmorling added this to the 1.0.0.Beta2 milestone Oct 3, 2018
@lukehutch
Copy link
Contributor Author

In case anyone is referring back to this issue at some point, I found direct support for this in JEP 238:

A multi-release modular need not have a module descriptor at the located root. In this respect a module descriptor would be treated no differently to any other class or resource file. This can ensure that, for example, only Java 8 versioned classes are present in the root area while Java 9 versioned classes (including the module descriptor) are present in the 9 versioned area.

@lukehutch
Copy link
Contributor Author

@gunnarmorling Any idea when you plan to release Beta2? I have a user who has been hitting up against problems using ClassGraph because the Android build tools can't handle module-info.class in the root directory of a jarfile...

@lukehutch
Copy link
Contributor Author

@gunnarmorling Any chance you can please push out a new release? I got yet another report of problems from third party runtime environments dying when they find a module-info.class in the root of the jar. (This time it's Tomcat BCEL, called by Apache Catalina.) This is probably the third or fourth time I have run into this. I could manually move the module-info.class into a versioned section of the jar, but I'd rather automate it with the new Moditect features. Thanks!

@gunnarmorling
Copy link
Member

Hey @lukehutch, thanks for the reminder! Apologies for the delay (and just ignoring your earlier question about the release, I somehow missed that). I'll try and push out a release over the weekend.

@lukehutch
Copy link
Contributor Author

Thank you!

@gunnarmorling
Copy link
Member

Agh, so I ran into some issues with the set-up for the release today :( I'll keep working on this in the course of the next days.

lukehutch added a commit to classgraph/classgraph that referenced this issue Dec 25, 2018
@lukehutch
Copy link
Contributor Author

<jvmVersion>9</jvmVersion> seems to work fine out of the box for my build of the ClassGraph library. Thank you @gunnarmorling!

remkop referenced this issue in Warkdev/picocli Apr 6, 2019
Addong a JMPS 9 compliant (hopefully) subproject. This subproject needs the root project to work as source files are copied to this subproject before compilation.

It assumes that tests are run from the root project and not against this subproject, the objective being only to release a compliant Java 9 JPMS module.
Warkdev added a commit to Warkdev/picocli that referenced this issue Apr 6, 2019
- Add org.beryx.jar plugin to allow META-INF/versions/9 folder to be created when doing a jar of this subproject. (see moditect/moditect#67 & https://dzone.com/articles/how-to-create-modular-jars-that-target-a-java-rele)
- Change compatibility level to Java 1.8 to trigger the plugin.
- Update of the Manifest title.
remkop pushed a commit to remkop/picocli that referenced this issue Apr 12, 2019
- Add org.beryx.jar plugin to allow META-INF/versions/9 folder to be created when doing a jar of this subproject. (see moditect/moditect#67 & https://dzone.com/articles/how-to-create-modular-jars-that-target-a-java-rele)
- Change compatibility level to Java 1.8 to trigger the plugin.
- Update of the Manifest title.
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