-
Notifications
You must be signed in to change notification settings - Fork 49
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
JarToolModularJarArchiver uses the manifest's main class when, in some cases, perhaps it shouldn't #310
Comments
Hi, Would you share more details about the specific use case? Is my understanding correct that you want to have If I recall correctly (but some time has passed so I could be wrong), the decision to use the |
The "use case" is an example project: you, the user, receive an (executable-jar-producing) example project that is not modular. You discover that some of its dependencies are modular, so you think, ah, I will go Full Java Module System™! It is The Future™! So you add a (That's the user experience.) So yes, right before this user adds a Again, this is because the |
Thanks for the details, makes sense. I wonder if we can do such check without reading the |
For reference: https://github.com/codehaus-plexus/plexus-archiver/blob/plexus-archiver-4.9.0/src/main/java/org/codehaus/plexus/archiver/jar/JarToolModularJarArchiver.java#L210
Scenario: we had a Maven build of a non-modular project whose
pom.xml
features a<mainClass>
element in the<manifest>
portion of the archive configuration. Let's say the main class in question iscom.foo.other.Main
.Now we add a
module-info.java
to the project, thus making it a modular project. Let's say this project exportscom.foo
. (Clearly it cannot export or containcom.foo.other
because then you'd have a package split.)Even if the project housing
com.foo.other.Main
is modular (in the, say,com.foo.other
module), and even if the newmodule-info.java
requirescom.foo.other
, when the Plexus archiver goes to update the jar file into a modular jar file, thejar --update
operation will fail:As far as I can tell, this is because the manifest's main class is used as if it were this module's main class, i.e. as if it were "owned" by this module.
My guess is the
jar
tool's update operation is adding in theModulePackages
classfile attribute into the existingmodule-info.class
, as it should, computed from the known packages of the current project'smodule-info.class
. You can see that here: https://github.com/openjdk/jdk/blob/a1e7a302c8a3d7a1069659653042476b20becabe/src/java.base/share/classes/jdk/internal/module/ModuleInfo.java#L306-L334This low-level
jar
error is the only indication that the main class specified in thepom.xml
is incorrect in some way. As I mentioned above, I think that it is incorrect because, I guess, a main class whose name will be stored in amodule-info.class
file, must (again, a guess) be "from" the module in question. In this example, it is not.Perhaps—and I'm not an expert here—if the
JarToolModularJarArchiver
realizes that there is no module main class set, it should not fall back on the manifest main class, or at least should see if the manifest main class is from an "OK" package.I believe it is incorrect for a module descriptor's binary format to encode a main class that does not belong to the module, but as I said I'm not really an expert here.
Thanks for all the work you do to support Maven and the open source Java community. ❤️
The text was updated successfully, but these errors were encountered: