Drop the version from libraries packaged under WEB-INF/lib
#9228
Conversation
The default mapping requires `mvn clean`ing every time whenever a library is updated, otherwise it ends up being packaged twice. It also eases deep war comparison.
OTOH makes it more difficult to determine for a given war, which version of a library is included. |
Some tests look broken, back to draft. |
There was a problem hiding this comment.
-0 from me. I don't feel strongly enough to block the change, but I somewhat like the status quo, as it improves observability (e.g. when listing open file descriptors with pfiles or ls -lah /proc/$PID/fd). Admittedly, I only ever run mvn clean verify (never mvn verify, for exactly this reason), but my experience is that the former is good enough. I have a natural bias for observability over performance, and I recognize that others have different priorities, which is why I am not blocking this PR, but I would prefer to keep this as-is and to close this PR.
There was a problem hiding this comment.
I have a preference (+0) for stability in the list of ZIP entries.
Regarding version lookups, jenkins.war!/META-INF/maven/org.jenkins-ci.main/jenkins-war/pom.xml and jenkins.war!/WEB-INF/lib/jenkins-core.jar!/META-INF/maven/org.jenkins-ci.main/jenkins-core/pom.xml contain resolved version numbers.
For what reason do you have this preference?
My complaint is that these aren't casually visible to system-level debugging tools and require more effort to peel open. |
Same as in PR description. Also easier to maintain scripts which pick out a member by name. No strong feeling, but have generally encountered fewer problems with Maven projects configured to suppress the default behavior of injecting |
That seems like a pretty contrived use case to me—better to construct the WAR file as desired rather than pick out a member in a script after the fact.
No strong feeling either, but from the other perpsective one could also make the argument that deviating from the default behavior for any reason is an increase in code that has to be maintained (even a small one). |
|
Talking about technical debt, I had issues in the past with a setup relying on a custom Could be qualified as an exotic use case, but that still exists in real life. This was leading to test errors like Dropping the version from packaging would have avoided that kind of issue. |
Maven defaults are not always sane TBH. |
That case looks more like a reason to keep the version, seems like the error was fairly explicit, and resulting behavior discrepancies between jars would be far more difficult to diagnose? |
Actually not so much, added clarification in One more protection that could be added would be for maven-hpi-plugin to validate that the jenkins core version in classpath is the same as the core version provided part of the custom jenkinsWarId.
|
The default mapping requires
mvn cleaning every time whenever a library is updated, otherwise it ends up being packaged twice. It also eases deep war comparison.Con is that it is slightly more difficult to determine library version, since you need to look into
lib.jar/META-INF/MANIFEST.MFrather than look at the file name itself.Testing done
/jnlpJars/agent.jaris reachableProposed changelog entries
N/A
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@mention
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered (see query).