feat(maven-wrapper): support wrapperVersion #28670
Conversation
… standard after 3.3.0
| @@ -2,11 +2,19 @@ import { extractPackageFile } from '.'; | |||
|
|
|||
| const onlyWrapperProperties = | |||
| 'wrapperUrl=https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar'; | |||
|
|
|||
| const onlyWrapperVersionProperties = | |||
| 'wrapperVersion=https://internal.artifactory.acme.org/artifactory/maven-bol/org/apache/maven/wrapper/maven-wrapper/3.3.0/maven-wrapper-3.3.0.jar'; | |||
There was a problem hiding this comment.
The value of the wrapperVersion property should be just a version number, so for example: wrapperVersion=3.3.1. (wrapperVersion was introduced in Maven Wrapper 3.3.1, so using 3.3.0 in the test example might be a little confusing.)
See the real-world examples attached to this comment. For instance maven-wrapper-3.3.1-only-script has these properties:
wrapperVersion=3.3.1
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
There was a problem hiding this comment.
The trickiest case is maven-wrapper-3.3.0-only-script, because the has wrapperUrl nor wrapperVersion in .mvn/wrapper/maven-wrapper.properties:
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.6/apache-maven-3.9.6-bin.zip
But maven-wrapper-3.3.0-only-script also represents the state that projects are currently in if they got updated to Maven Wrapper 3.3.0 by Renovate, because only-script is the new default installation type in Maven Wrapper 3.3.0.
I believe the Maven Wrapper version for maven-wrapper-3.3.0-only-script can only be detected by looking for the string Apache Maven Wrapper startup batch script, version 3.3.0 in mvnw (or mvnw.cmd if mvnw is not present).
There was a problem hiding this comment.
I'm currently trying to find out if Maven Wrapper provides a way to update Maven Wrapper using the existing installation type, without having to explicitly specify (and thus determine) it. If so, Renovate should update Maven Wrapper like that.
If upgrading using the existing installation type can only be done by specifying the type in the update command (-Dtype=${type}), then I plan to request a property for that too (e.g. wrapperType). But for older versions that don't explicitly communicate their installation type, Renovate might still need to detect the installation type. I suggest keeping this installation type business out of scope for this current PR. That could be added later if needed.
There was a problem hiding this comment.
How does it figure out on which url it should get the maven-wrapper ? Or is that now all covered in mvn ?
There was a problem hiding this comment.
The only-script type doesn’t use the Maven Wrapper JAR. The script uses curl or wget to download Maven and calls mvn directly. See https://maven.apache.org/wrapper/#usage-with-or-without-binary-jar for more info about the different types.
Renovate doesn’t need to know how this works, right?
| @@ -15,10 +15,17 @@ const WRAPPER_URL_REGEX = regEx( | |||
| '^(?:wrapperUrl\\s*=\\s*)(?<url>\\S*-(?<version>\\d+\\.\\d+(?:\\.\\d+)?(?:-\\w+)*)(?:.jar))', | |||
| ); | |||
|
|
|||
| const WRAPPER_VERSION_REGEX = regEx( | |||
| '^(?:wrapperVersion\\s*=\\s*)(?<url>\\S*-(?<version>\\d+\\.\\d+(?:\\.\\d+)?(?:-\\w+)*)(?:.jar))', | |||
There was a problem hiding this comment.
The value of the wrapperVersion property should be just a version number, so for example: wrapperVersion=3.3.0. This shouldn't need any regular expression or other parsing. See the real-world examples attached to this comment.
There was a problem hiding this comment.
it seems we only need the version group? 🤔
rarkins
changed the title
rarkins
changed the title
| const onlyMavenProperties = | ||
| 'distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.5.4/apache-maven-3.5.4-bin.zip'; | ||
|
|
||
| const wrapperAndMavenProperties = `distributionUrl=https://internal.artifactory.acme.org/artifactory/maven-bol/org/apache/maven/apache-maven/3.8.4/apache-maven-3.8.4-bin.zip\nwrapperUrl=https://internal.artifactory.acme.org/artifactory/maven-bol/org/apache/maven/wrapper/maven-wrapper/3.1.0/maven-wrapper-3.1.0.jar`; | ||
|
|
||
| const wrapperVersionAndMavenProperties = `distributionUrl=https://internal.artifactory.acme.org/artifactory/maven-bol/org/apache/maven/apache-maven/3.8.4/apache-maven-3.8.4-bin.zip\nwrapperVersion=https://internal.artifactory.acme.org/artifactory/maven-bol/org/apache/maven/wrapper/maven-wrapper/3.3.0/maven-wrapper-3.3.0.jar`; |
There was a problem hiding this comment.
please use codeBlock template helper for better readability. use only for your new tests
| @@ -15,10 +15,17 @@ const WRAPPER_URL_REGEX = regEx( | |||
| '^(?:wrapperUrl\\s*=\\s*)(?<url>\\S*-(?<version>\\d+\\.\\d+(?:\\.\\d+)?(?:-\\w+)*)(?:.jar))', | |||
| ); | |||
|
|
|||
| const WRAPPER_VERSION_REGEX = regEx( | |||
| '^(?:wrapperVersion\\s*=\\s*)(?<url>\\S*-(?<version>\\d+\\.\\d+(?:\\.\\d+)?(?:-\\w+)*)(?:.jar))', | |||
There was a problem hiding this comment.
it seems we only need the version group? 🤔
|
@breun thanks again for this PR. What can we do to help get it mergeable? |
I’ve reached out to @trystian1, and I do believe he is still willing to help, but he hasn’t yet been able to find time to work on this further. I also believe he wouldn’t mind if someone else implemented the changes required for the desired behavior I described. |
… standard after 3.3.0
Changes
Context
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: