From 596eeb8bfde9b4c0cd1995eceb396d84a9edf8d6 Mon Sep 17 00:00:00 2001 From: Andrzej Jarmoniuk Date: Fri, 24 Feb 2023 18:49:51 +0100 Subject: [PATCH] Resolves #916: Partially reverted #799. - Reviewed the code and the semantics of processAllModules, groupId, artifactId, oldVersion params - Dropped support for partially interpolating the properties to be matched with groupId:artifactId:oldVersion - Matching occurs always unless *processAllModules* is set - If *processAllModules* is not set, *oldVersion* will be respected - Included an additional test case from #916 testing child version --- versions-maven-plugin/pom.xml | 3 + .../it-set-005-properties/invoker.properties | 2 + .../it-set-005-properties/module-a1/pom.xml | 15 ++ .../it-set-005-properties/module-a2/pom.xml | 15 ++ .../src/it/it-set-005-properties/pom.xml | 104 ++++++++++++++ .../it/it-set-005-properties/verify.groovy | 12 ++ .../src/it/it-set-005/verify.bsh | 88 ------------ .../src/it/it-set-005/verify.groovy | 11 ++ .../invoker.properties | 2 + .../it/it-set-issue-916-properties/pom.xml | 10 ++ .../it-set-issue-916-properties/verify.groovy | 4 + .../org/codehaus/mojo/versions/SetMojo.java | 128 +++++++++--------- .../codehaus/mojo/versions/SetMojoTest.java | 36 ++--- .../mojo/set/issue-855/pom-revision.xml | 24 ++++ 14 files changed, 278 insertions(+), 176 deletions(-) create mode 100644 versions-maven-plugin/src/it/it-set-005-properties/invoker.properties create mode 100644 versions-maven-plugin/src/it/it-set-005-properties/module-a1/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-005-properties/module-a2/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-005-properties/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-005-properties/verify.groovy delete mode 100644 versions-maven-plugin/src/it/it-set-005/verify.bsh create mode 100644 versions-maven-plugin/src/it/it-set-005/verify.groovy create mode 100644 versions-maven-plugin/src/it/it-set-issue-916-properties/invoker.properties create mode 100644 versions-maven-plugin/src/it/it-set-issue-916-properties/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-issue-916-properties/verify.groovy create mode 100644 versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-855/pom-revision.xml diff --git a/versions-maven-plugin/pom.xml b/versions-maven-plugin/pom.xml index 1025a803c..40c02df50 100644 --- a/versions-maven-plugin/pom.xml +++ b/versions-maven-plugin/pom.xml @@ -289,6 +289,9 @@ false true + + + diff --git a/versions-maven-plugin/src/it/it-set-005-properties/invoker.properties b/versions-maven-plugin/src/it/it-set-005-properties/invoker.properties new file mode 100644 index 000000000..856b847db --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-005-properties/invoker.properties @@ -0,0 +1,2 @@ +invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:set +invoker.mavenOpts = -DnewVersion=1.2.1-SNAPSHOT -Drevision=1.2.0 diff --git a/versions-maven-plugin/src/it/it-set-005-properties/module-a1/pom.xml b/versions-maven-plugin/src/it/it-set-005-properties/module-a1/pom.xml new file mode 100644 index 000000000..cff5ec29e --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-005-properties/module-a1/pom.xml @@ -0,0 +1,15 @@ + + + + localdomain.localhost + project-a + ${revision} + + + 4.0.0 + localdomain.localhost + module-a1 + pom + 2.0.7-SNAPSHOT + + diff --git a/versions-maven-plugin/src/it/it-set-005-properties/module-a2/pom.xml b/versions-maven-plugin/src/it/it-set-005-properties/module-a2/pom.xml new file mode 100644 index 000000000..db808c507 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-005-properties/module-a2/pom.xml @@ -0,0 +1,15 @@ + + + + localdomain.localhost + project-a + ${revision} + + + 4.0.0 + localdomain.localhost + module-a2 + pom + ${revision} + + diff --git a/versions-maven-plugin/src/it/it-set-005-properties/pom.xml b/versions-maven-plugin/src/it/it-set-005-properties/pom.xml new file mode 100644 index 000000000..6b8590b85 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-005-properties/pom.xml @@ -0,0 +1,104 @@ + + + + 4.0.0 + localdomain.localhost + project-a + pom + ${revision} + mversions-71 + + + invoking versions:set on root module should not update child module versions + unless they are the same as the parent old version + + + + module-a1 + module-a2 + + + + + + + maven-antrun-plugin + 1.1 + + + maven-assembly-plugin + 2.2-beta-2 + + + maven-clean-plugin + 2.2 + + + maven-compiler-plugin + 2.0.2 + + + maven-dependency-plugin + 2.0 + + + maven-deploy-plugin + 2.3 + + + maven-ear-plugin + 2.3.1 + + + maven-ejb-plugin + 2.1 + + + maven-install-plugin + 2.2 + + + maven-jar-plugin + 2.2 + + + maven-javadoc-plugin + 2.4 + + + maven-plugin-plugin + 2.4.1 + + + maven-rar-plugin + 2.2 + + + maven-release-plugin + 2.0-beta-7 + + + maven-resources-plugin + 2.2 + + + maven-site-plugin + 2.0 + + + maven-source-plugin + 2.0.4 + + + maven-surefire-plugin + 2.4.2 + + + maven-war-plugin + 2.1-alpha-1 + + + + + + diff --git a/versions-maven-plugin/src/it/it-set-005-properties/verify.groovy b/versions-maven-plugin/src/it/it-set-005-properties/verify.groovy new file mode 100644 index 000000000..58de3b954 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-005-properties/verify.groovy @@ -0,0 +1,12 @@ +import groovy.xml.XmlSlurper + +def parentPom = new XmlSlurper().parse( new File( basedir, 'pom.xml' ) ) +assert parentPom.version == '1.2.1-SNAPSHOT' + +def moduleA1 = new XmlSlurper().parse( new File( basedir, 'module-a1/pom.xml' ) ) +assert moduleA1.parent.version == '1.2.1-SNAPSHOT' + +def moduleA2 = new XmlSlurper().parse( new File( basedir, 'module-a2/pom.xml' ) ) +assert moduleA2.parent.version == '1.2.1-SNAPSHOT' +assert moduleA2.version == '1.2.1-SNAPSHOT' + diff --git a/versions-maven-plugin/src/it/it-set-005/verify.bsh b/versions-maven-plugin/src/it/it-set-005/verify.bsh deleted file mode 100644 index 666283885..000000000 --- a/versions-maven-plugin/src/it/it-set-005/verify.bsh +++ /dev/null @@ -1,88 +0,0 @@ -import java.io.*; -import org.codehaus.plexus.util.FileUtils; -import java.util.regex.*; - -try -{ - File file = new File( basedir, "pom.xml" ); - - BufferedReader in = new BufferedReader( new InputStreamReader( new FileInputStream( file ), "UTF-8" ) ); - StringBuilder buf = new StringBuilder(); - String line = in.readLine(); - while ( line != null ) - { - buf.append( line ); - buf.append( " " ); - line = in.readLine(); - } - - Pattern p = Pattern.compile( "\\Q\\E\\s*1\\.2\\.1-SNAPSHOT\\s*\\Q\\E" ); - Matcher m = p.matcher( buf.toString() ); - if ( !m.find() ) - { - System.out.println( "Did not set new version in parent pom" ); - return false; - } - - file = new File( basedir, "module-a1/pom.xml" ); - - in = new BufferedReader( new InputStreamReader( new FileInputStream( file ), "UTF-8" ) ); - buf = new StringBuilder(); - line = in.readLine(); - while ( line != null ) - { - buf.append( line ); - buf.append( " " ); - line = in.readLine(); - } - - p = Pattern.compile( "\\Q\\E.*\\Q\\E\\s*1\\.2\\.1-SNAPSHOT\\s*\\Q\\E.*\\Q\\E" ); - m = p.matcher( buf.toString() ); - if ( !m.find() ) - { - System.out.println( "Did not update parent version in module-a1/pom" ); - return false; - } - p = Pattern.compile( "\\Q\\E.*\\Q\\E\\s*2\\.0\\.7-SNAPSHOT\\s*\\Q\\E" ); - m = p.matcher( buf.toString() ); - if ( !m.find() ) - { - System.out.println( "Did update child version in module-a1/pom" ); - return false; - } - - file = new File( basedir, "module-a2/pom.xml" ); - - in = new BufferedReader( new InputStreamReader( new FileInputStream( file ), "UTF-8" ) ); - buf = new StringBuilder(); - line = in.readLine(); - while ( line != null ) - { - buf.append( line ); - buf.append( " " ); - line = in.readLine(); - } - - p = Pattern.compile( "\\Q\\E.*\\Q\\E\\s*1\\.2\\.1-SNAPSHOT\\s*\\Q\\E.*\\Q\\E" ); - m = p.matcher( buf.toString() ); - if ( !m.find() ) - { - System.out.println( "Did not update parent version in module-a2/pom" ); - return false; - } - p = Pattern.compile( "\\Q\\E.*\\Q\\E\\s*1\\.2\\.1-SNAPSHOT\\s*\\Q\\E" ); - m = p.matcher( buf.toString() ); - if ( !m.find() ) - { - System.out.println( "Did not update child version in module-a2/pom" ); - return false; - } - -} -catch( Throwable t ) -{ - t.printStackTrace(); - return false; -} - -return true; diff --git a/versions-maven-plugin/src/it/it-set-005/verify.groovy b/versions-maven-plugin/src/it/it-set-005/verify.groovy new file mode 100644 index 000000000..958c1c389 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-005/verify.groovy @@ -0,0 +1,11 @@ +import groovy.xml.XmlSlurper + +def parentPom = new XmlSlurper().parse( new File( basedir, 'pom.xml' ) ) +assert parentPom.version == '1.2.1-SNAPSHOT' + +def moduleA1 = new XmlSlurper().parse( new File( basedir, 'module-a1/pom.xml' ) ) +assert moduleA1.parent.version == '1.2.1-SNAPSHOT' + +def moduleA2 = new XmlSlurper().parse( new File( basedir, 'module-a2/pom.xml' ) ) +assert moduleA2.parent.version == '1.2.1-SNAPSHOT' + diff --git a/versions-maven-plugin/src/it/it-set-issue-916-properties/invoker.properties b/versions-maven-plugin/src/it/it-set-issue-916-properties/invoker.properties new file mode 100644 index 000000000..0f968fa27 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-916-properties/invoker.properties @@ -0,0 +1,2 @@ +invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:set +invoker.mavenOpts = -Drevision=1.0 -DnewVersion=TEST diff --git a/versions-maven-plugin/src/it/it-set-issue-916-properties/pom.xml b/versions-maven-plugin/src/it/it-set-issue-916-properties/pom.xml new file mode 100644 index 000000000..3da8c85c1 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-916-properties/pom.xml @@ -0,0 +1,10 @@ + + 4.0.0 + + default-group + default-artifact + ${revision}-SNAPSHOT + pom + + diff --git a/versions-maven-plugin/src/it/it-set-issue-916-properties/verify.groovy b/versions-maven-plugin/src/it/it-set-issue-916-properties/verify.groovy new file mode 100644 index 000000000..efdb6445a --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-916-properties/verify.groovy @@ -0,0 +1,4 @@ +import groovy.xml.XmlSlurper + +def project = new XmlSlurper().parse( new File( basedir, 'pom.xml' ) ) +assert project.version == 'TEST' diff --git a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java index 4ae1df526..93f56e271 100644 --- a/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java +++ b/versions-maven-plugin/src/main/java/org/codehaus/mojo/versions/SetMojo.java @@ -35,7 +35,6 @@ import java.util.TimeZone; import java.util.TreeMap; import java.util.regex.Pattern; -import java.util.stream.Collectors; import org.apache.maven.artifact.ArtifactUtils; import org.apache.maven.model.Model; @@ -85,42 +84,54 @@ public class SetMojo extends AbstractVersionsUpdaterMojo { private String newVersion; /** - * The groupId of the dependency/module to update. - * If you like to update modules of a aggregator you - * should set -DgroupId='*' to ignore the - * group of the current project. On Windows you can omit - * the single quotes on Linux they are necessary to prevent - * expansion through the shell. + * If set to {@code true}, will process all modules regardless whether they + * match {@code groupId:artifactId:oldVersion}. + * + * @since 2.5 + */ + @Parameter(property = "processAllModules", defaultValue = "false") + private boolean processAllModules; + + /** + *

The non-interpolated groupId of the dependency/module to be selected for update.

+ *

If not set, will be equal to the non-interpolated groupId of the project file.

+ *

If you wish to update modules of a aggregator regardless of the groupId, you + * should set {@code -DgroupId='*'} to ignore the groupId of the current project.

+ *

Alternatively, you can use {@code -DprocessAllModules=true}

+ *

The goal does not interpolate the properties used in groupId used in the pom.xml file.

+ *

The single quotes are only necessary on POSIX-compatible shells (Linux, MacOS, etc.).

* * @since 1.2 */ - @Parameter(property = "groupId", defaultValue = "${project.groupId}") + @Parameter(property = "groupId") private String groupId; /** - * The artifactId of the dependency/module to update. - * If you like to update modules of a aggregator you - * should set -DartifactId='*' to ignore the - * artifactId of the current project. On Windows you can omit - * the single quotes on Linux they are necessary to prevent - * expansion through the shell. + *

The non-interpolated artifactId of the dependency/module to be selected for update.

+ *

If not set, will be equal to the non-interpolated artifactId of the project file.

+ *

If you wish to update modules of a aggregator regardless of the artifactId, you + * should set {@code -DartifactId='*'} to ignore the artifactId of the current project.

+ *

Alternatively, you can use {@code -DprocessAllModules=true}

+ *

The goal does not interpolate the properties used in artifactId used in the pom.xml file.

+ *

The single quotes are only necessary on POSIX-compatible shells (Linux, MacOS, etc.).

* * @since 1.2 */ - @Parameter(property = "artifactId", defaultValue = "${project.artifactId}") + @Parameter(property = "artifactId") private String artifactId; /** - * The version of the dependency/module to update. - * If you are changing an aggregator you should give - * -DoldVersion='*' to suppress the check against the - * version of the current project. On Windows you can omit - * the single quotes on Linux they are necessary to prevent - * expansion through the shell. + *

The non-interpolated version of the dependency/module to be selected for update.

+ *

If not set, will be equal to the non-interpolated version of the project file.

+ *

If you wish to update modules of a aggregator regardless of the version, you + * should set {@code -Dversion='*'} to ignore the version of the current project.

+ *

Alternatively, you can use {@code -DprocessAllModules=true}

+ *

The goal does not interpolate the properties used in version used in the pom.xml file.

+ *

The single quotes are only necessary on POSIX-compatible shells (Linux, MacOS, etc.).

* * @since 1.2 */ - @Parameter(property = "oldVersion", defaultValue = "${project.version}") + @Parameter(property = "oldVersion") private String oldVersion; /** @@ -198,14 +209,6 @@ public class SetMojo extends AbstractVersionsUpdaterMojo { @Parameter(property = "nextSnapshotIndexToIncrement") protected Integer nextSnapshotIndexToIncrement; - /** - * Whether to process all modules whereas they have parent/child or not. - * - * @since 2.5 - */ - @Parameter(property = "processAllModules", defaultValue = "false") - private boolean processAllModules; - /** * Whether to start processing at the local aggregation root (which might be a parent module * of that module where Maven is executed in, and the version change may affect parent and sibling modules). @@ -247,15 +250,6 @@ public class SetMojo extends AbstractVersionsUpdaterMojo { */ protected final ProjectBuilder projectBuilder; - /** - * If set to {@code false}, the plugin will not interpolate property values when looking for versions - * to be changed, but will instead operate on raw model. - * - * @since 2.15.0 - */ - @Parameter(property = "interpolateProperties", defaultValue = "true") - protected boolean interpolateProperties = true; - @Inject public SetMojo( RepositorySystem repositorySystem, @@ -344,35 +338,45 @@ public void execute() throws MojoExecutionException, MojoFailureException { // set of files to update final Set files = new LinkedHashSet<>(); - getLog().info("Processing change of " + groupId + ":" + artifactId + ":" + oldVersion + " -> " - + newVersion); - Pattern groupIdRegex = - Pattern.compile(RegexUtils.convertWildcardsToRegex(fixNullOrEmpty(groupId, "*"), true)); - Pattern artifactIdRegex = - Pattern.compile(RegexUtils.convertWildcardsToRegex(fixNullOrEmpty(artifactId, "*"), true)); - Pattern oldVersionIdRegex = - Pattern.compile(RegexUtils.convertWildcardsToRegex(fixNullOrEmpty(oldVersion, "*"), true)); + // groupId, artifactId, oldVersion are matched against every module of the project to see if the module + // needs to be changed as well + // setting them to the main project coordinates in case they are not set by the user, + // so that the main project can be selected + Model rootModel = reactorModels.get(session.getCurrentProject().getFile()); + if (groupId == null) { + groupId = rootModel.getGroupId(); + } + if (artifactId == null) { + artifactId = rootModel.getArtifactId(); + } + if (oldVersion == null) { + oldVersion = rootModel.getVersion(); + } + + getLog().info(String.format( + "Processing change of %s:%s:%s -> %s", groupId, artifactId, oldVersion, newVersion)); + + Pattern groupIdRegex = processAllModules || StringUtils.isBlank(groupId) || "*".equals(groupId) + ? null + : Pattern.compile(RegexUtils.convertWildcardsToRegex(groupId, true)); + Pattern artifactIdRegex = processAllModules || StringUtils.isBlank(artifactId) || "*".equals(artifactId) + ? null + : Pattern.compile(RegexUtils.convertWildcardsToRegex(artifactId, true)); + Pattern oldVersionIdRegex = processAllModules || StringUtils.isBlank(oldVersion) || "*".equals(oldVersion) + ? null + : Pattern.compile(RegexUtils.convertWildcardsToRegex(oldVersion, true)); for (Model m : reactor.values()) { String mGroupId = PomHelper.getGroupId(m); String mArtifactId = PomHelper.getArtifactId(m); String mVersion = PomHelper.getVersion(m); - if (interpolateProperties) { - assert m.getProperties() != null; // always non-null - Map properties = m.getProperties().entrySet().stream() - .collect(Collectors.toMap(e -> e.getKey().toString(), e -> e.getValue() - .toString())); - - mGroupId = PomHelper.evaluate(mGroupId, properties); - mArtifactId = PomHelper.evaluate(mArtifactId, properties); - mVersion = PomHelper.evaluate(mVersion, properties); - } - - if ((processAllModules - || groupIdRegex.matcher(mGroupId).matches() - && artifactIdRegex.matcher(mArtifactId).matches()) - && oldVersionIdRegex.matcher(mVersion).matches() + if ((groupIdRegex == null || groupIdRegex.matcher(mGroupId).matches()) + && (artifactIdRegex == null + || artifactIdRegex.matcher(mArtifactId).matches()) + && (mVersion == null + || oldVersionIdRegex == null + || oldVersionIdRegex.matcher(mVersion).matches()) && !newVersion.equals(mVersion)) { applyChange( project, diff --git a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java b/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java index 9bec54884..4ffcdf081 100644 --- a/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java +++ b/versions-maven-plugin/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java @@ -156,9 +156,11 @@ public void testSetOldVersionMismatchProcessAllModules() throws Exception { setVariableValueToObject(mojo, "newVersion", "bar"); setVariableValueToObject(mojo, "processAllModules", true); mojo.execute(); + // with processAllModules, the module *should* be selected for update + // regardless of the version assertThat( String.join("", Files.readAllLines(tempDir.resolve("pom.xml"))), - not(containsString("bar"))); + containsString("bar")); } private void testSetParameterValue(String filename, Consumer... initializers) throws Exception { @@ -168,9 +170,8 @@ private void testSetParameterValue(String filename, Consumer... initial SetMojo mojo = (SetMojo) mojoRule.lookupConfiguredMojo(tempDir.toFile(), "set"); Stream.of(initializers).forEachOrdered(i -> i.accept(mojo)); mojo.execute(); - assertThat( - String.join("", Files.readAllLines(tempDir.resolve("pom.xml"))), - containsString("testing")); + String output = String.join("", Files.readAllLines(tempDir.resolve("pom.xml"))); + assertThat(output, containsString("testing")); } @Test @@ -178,28 +179,6 @@ public void testSetParameterValueSimple() throws Exception { testSetParameterValue("pom-simple.xml"); } - @Test - public void testSetParameterValueSimpleNoInterpolation() throws Exception { - try { - testSetParameterValue("pom-simple.xml", mojo -> mojo.interpolateProperties = false); - fail(); - } catch (AssertionError e) { - assertThat(e.getMessage(), containsString("Expected: a string containing \"testing")); - } - } - - @Test - public void testSetParameterValueSimpleNoInterpolationWildcard() throws Exception { - testSetParameterValue("pom-simple.xml", mojo -> { - mojo.interpolateProperties = false; - try { - setVariableValueToObject(mojo, "oldVersion", "*"); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } - }); - } - @Test public void testSetParameterValueBuildNumber() throws Exception { testSetParameterValue("pom-build-number.xml"); @@ -215,6 +194,11 @@ public void testSetParameterValueMultipleProps() throws Exception { testSetParameterValue("pom-multiple-props.xml"); } + @Test + public void testSetParameterValuePartial() throws Exception { + testSetParameterValue("pom-revision.xml"); + } + @Test public void testParentWithProperty() throws Exception { TestUtils.copyDir( diff --git a/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-855/pom-revision.xml b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-855/pom-revision.xml new file mode 100644 index 000000000..ff0806fe9 --- /dev/null +++ b/versions-maven-plugin/src/test/resources/org/codehaus/mojo/set/issue-855/pom-revision.xml @@ -0,0 +1,24 @@ + + org.example + test-versions + ${revision}-SNAPSHOT + 4.0.0 + + + + + org.codehaus.mojo + versions-maven-plugin + + set + + + testing + false + + + + +