Nuance the message about filename-based automodules detected on the module path#11857
Nuance the message about filename-based automodules detected on the module path#11857desruisseaux wants to merge 1 commit into
Conversation
…odule path. Instead of "Please don't publish this project to a public artifact repository", said "This project may not work in applications that do not use these dependencies with exactly the same filenames as listed above".
| @@ -236,7 +236,7 @@ public JavaPathType getPathType() { | |||
| /** | |||
| * If the module has no name, adds the filename of the JAR file in the given collection. | |||
| * This method should be invoked for dependencies placed on {@link JavaPathType#MODULES} | |||
There was a problem hiding this comment.
not new here, but this doc comment really is hard to follow or understand. It should be rewritten.
| fileSeparator, | ||
| "Filename-based automodules detected on the module path: " + fileSeparator, | ||
| lineSeparator + "This project may not work in applications that do not use " | ||
| + "these dependencies with exactly the same filenames as listed above."); |
There was a problem hiding this comment.
Is this only an issue when JPMS is used?
There was a problem hiding this comment.
Yes, this is an issue if non-modular JARs are placed on the --module-path option. While not recommended, this is not forbidden neither in order to allow gradual transition, because the effect is not the same as keeping those JARs on --class-path. The javac tool already emits a warning for such cases, and developers may accept the risk if they understand the condition in which it works, which is to keep the JAR filenames unchanged.
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Claude Code on behalf of Guillaume Nodet
The rationale is sound: the old message was overly prescriptive ("don't publish") when in practice Maven artifact filenames are deterministic. The new wording correctly shifts from a prohibition to an informational warning about the fragility.
Two minor observations below.
| "Filename-based automodules detected on the module path: " + fileSeparator, | ||
| lineSeparator + "This project may not work in applications that do not use " | ||
| + "these dependencies with exactly the same filenames as listed above."); | ||
| automodulesDetected.forEach(joiner::add); |
There was a problem hiding this comment.
The new suffix is fairly long and may be hard to parse in a log. Consider shortening it slightly, e.g.:
| automodulesDetected.forEach(joiner::add); | |
| lineSeparator + "This project may not work if consumers use " | |
| + "these dependencies with different filenames."); |
The original phrasing ("do not use these dependencies with exactly the same filenames as listed above") requires a double-negative mental parse ("not ... exactly the same"). "Different filenames" says the same thing more directly.
Feel free to ignore if you prefer the current wording.
| * If the module has no name, adds the filename of the JAR file in the given collection. | ||
| * This method should be invoked for dependencies placed on {@link JavaPathType#MODULES} | ||
| * for preparing a warning asking to not deploy the build artifact on a public repository. | ||
| * for preparing a warning saying that this project may fail to resolve these modules. |
There was a problem hiding this comment.
Nit: the Javadoc on this method still says "for preparing a warning asking to not deploy the build artifact on a public repository", which no longer matches what the warning actually says. Should be updated to stay consistent with the new message.
| * for preparing a warning saying that this project may fail to resolve these modules. | |
| * for preparing a warning saying that this project may fail to resolve these modules. |
3 participants
Instead of "Please don't publish this project to a public artifact repository", said "This project may not work in applications that do not use these dependencies with exactly the same filenames as listed above".
Rational: it can be considered okay to publish the project on a public Maven repository, because the artifact filenames are quite deterministic with Maven. For a pure-Maven project and for bundle generated by the Maven assembly plugin, there is no reason why these filenames would not be stable. Therefore, we can warn the users about the fragility of filename-based automodules but let them decide whether they want to publish anyway.