From 83212118cb8cea2c2b68cfa3e20dbc92a69bcebf Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Sat, 13 Jul 2024 10:34:43 +0200 Subject: [PATCH] Revert #1051 - fix #1110, #1103 --- .../it/it-set-issue-1103/invoker.properties | 1 + .../parent/child/grandchild/pom.xml | 18 +++ .../it/it-set-issue-1103/parent/child/pom.xml | 25 ++++ .../src/it/it-set-issue-1103/parent/pom.xml | 26 ++++ .../src/it/it-set-issue-1103/pom.xml | 22 +++ .../src/it/it-set-issue-1103/verify.groovy | 14 ++ .../org/codehaus/mojo/versions/SetMojo.java | 127 +++++++++--------- .../codehaus/mojo/versions/SetMojoTest.java | 57 -------- 8 files changed, 167 insertions(+), 123 deletions(-) create mode 100644 versions-maven-plugin/src/it/it-set-issue-1103/invoker.properties create mode 100644 versions-maven-plugin/src/it/it-set-issue-1103/parent/child/grandchild/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-issue-1103/parent/child/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-issue-1103/parent/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-issue-1103/pom.xml create mode 100644 versions-maven-plugin/src/it/it-set-issue-1103/verify.groovy diff --git a/versions-maven-plugin/src/it/it-set-issue-1103/invoker.properties b/versions-maven-plugin/src/it/it-set-issue-1103/invoker.properties new file mode 100644 index 0000000000..a6d1bc5f1d --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-1103/invoker.properties @@ -0,0 +1 @@ +invoker.goals = ${project.groupId}:${project.artifactId}:${project.version}:set -DnewVersion=2 diff --git a/versions-maven-plugin/src/it/it-set-issue-1103/parent/child/grandchild/pom.xml b/versions-maven-plugin/src/it/it-set-issue-1103/parent/child/grandchild/pom.xml new file mode 100644 index 0000000000..8e1e377ce1 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-1103/parent/child/grandchild/pom.xml @@ -0,0 +1,18 @@ + + + + + + 4.0.0 + + + com.company.test + child + 100-SNAPSHOT + + + grandchild + + diff --git a/versions-maven-plugin/src/it/it-set-issue-1103/parent/child/pom.xml b/versions-maven-plugin/src/it/it-set-issue-1103/parent/child/pom.xml new file mode 100644 index 0000000000..5924fd47af --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-1103/parent/child/pom.xml @@ -0,0 +1,25 @@ + + + + + 4.0.0 + + + com.company.test + parent + 100-SNAPSHOT + + + child + pom + + + + grandchild + + + diff --git a/versions-maven-plugin/src/it/it-set-issue-1103/parent/pom.xml b/versions-maven-plugin/src/it/it-set-issue-1103/parent/pom.xml new file mode 100644 index 0000000000..5b409ad351 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-1103/parent/pom.xml @@ -0,0 +1,26 @@ + + + + + 4.0.0 + + + com.company.test + grandparent + 1 + + + parent + 100-SNAPSHOT + pom + + + + child + + + diff --git a/versions-maven-plugin/src/it/it-set-issue-1103/pom.xml b/versions-maven-plugin/src/it/it-set-issue-1103/pom.xml new file mode 100644 index 0000000000..00caba9b1d --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-1103/pom.xml @@ -0,0 +1,22 @@ + + + + 4.0.0 + + com.company.test + grandparent + 1 + pom + + https://github.com/mojohaus/versions/issues/1103 + + + + parent + + + diff --git a/versions-maven-plugin/src/it/it-set-issue-1103/verify.groovy b/versions-maven-plugin/src/it/it-set-issue-1103/verify.groovy new file mode 100644 index 0000000000..07d35f3e86 --- /dev/null +++ b/versions-maven-plugin/src/it/it-set-issue-1103/verify.groovy @@ -0,0 +1,14 @@ +import groovy.xml.XmlSlurper + +def project = new XmlSlurper().parse(new File(basedir, 'pom.xml')) +assert project.version == '2' + +def parent = new XmlSlurper().parse(new File(basedir, 'parent/pom.xml')) +assert parent.parent.version == '2' +assert parent.version == '100-SNAPSHOT' + +def child = new XmlSlurper().parse(new File(basedir, 'parent/child/pom.xml')) +assert child.parent.version == '100-SNAPSHOT' + +def grandchild = new XmlSlurper().parse(new File(basedir, 'parent/child/grandchild/pom.xml')) +assert grandchild.parent.version == '100-SNAPSHOT' 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 c35ad37a88..2be7d472b5 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 @@ -25,14 +25,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Date; -import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Set; import java.util.SortedMap; import java.util.TimeZone; @@ -40,11 +37,10 @@ import java.util.regex.Pattern; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.tuple.ImmutablePair; -import org.apache.commons.lang3.tuple.Pair; import org.apache.maven.artifact.ArtifactUtils; import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager; import org.apache.maven.model.Model; +import org.apache.maven.model.Parent; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugins.annotations.Mojo; @@ -339,7 +335,6 @@ public void execute() throws MojoExecutionException, MojoFailureException { Map reactorModels = PomHelper.getChildModels(project, getLog()); final SortedMap reactor = new TreeMap<>(new ReactorDepthComparator(reactorModels)); reactor.putAll(reactorModels); - final Map, Set> children = computeChildren(reactorModels); // set of files to update final Set files = new LinkedHashSet<>(); @@ -386,7 +381,6 @@ public void execute() throws MojoExecutionException, MojoFailureException { && !newVersion.equals(mVersion)) { applyChange( reactor, - children, files, mGroupId, m.getArtifactId(), @@ -413,34 +407,6 @@ public void execute() throws MojoExecutionException, MojoFailureException { } } - static Map, Set> computeChildren(Map reactor) { - return reactor.values().stream() - .filter(v -> v.getParent() != null) - .reduce( - new HashMap<>(), - (map, child) -> { - map.compute( - new ImmutablePair<>( - child.getParent().getGroupId(), - child.getParent().getArtifactId()), - (pair, set) -> Optional.ofNullable(set) - .map(children -> { - children.add(child); - return children; - }) - .orElse(new HashSet() { - { - add(child); - } - })); - return map; - }, - (m1, m2) -> { - m1.putAll(m2); - return m1; - }); - } - /** * Returns the incremented version, with the nextSnapshotIndexToIncrement indicating the 1-based index, * from the left, or the most major version component, of the version string. @@ -471,20 +437,20 @@ protected String getIncrementedVersion(String version, Integer nextSnapshotIndex } private void applyChange( - SortedMap reactor, - Map, Set> children, - Set files, - String groupId, - String artifactId, - String oldVersion) { + SortedMap reactor, Set files, String groupId, String artifactId, String oldVersion) { getLog().debug("Applying change " + groupId + ":" + artifactId + ":" + oldVersion + " -> " + newVersion); + // this is a triggering change addChange(groupId, artifactId, oldVersion, newVersion); + // now fake out the triggering change + + Map.Entry current = PomHelper.getModelEntry(reactor, groupId, artifactId); + if (current != null) { + current.getValue().setVersion(newVersion); + files.add(current.getValue().getPomFile()); + } - Optional.ofNullable(PomHelper.getModelEntry(reactor, groupId, artifactId)) - .map(Map.Entry::getValue) - .map(Model::getPomFile) - .ifPresent(files::add); + // TODO there is for in for by reactor - should be refactored for (Map.Entry sourceEntry : reactor.entrySet()) { final File sourcePath = sourceEntry.getKey(); @@ -515,27 +481,56 @@ private void applyChange( getLog().debug("Looking for modules which use " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + " as their parent"); - Optional.ofNullable(children.get(new ImmutablePair<>(sourceGroupId, sourceArtifactId))) - .ifPresent(set -> set.forEach(model -> { - final boolean hasExplicitVersion = PomHelper.isExplicitVersion(model); - if ((updateMatchingVersions || !hasExplicitVersion) - && (StringUtils.equals(sourceVersion, PomHelper.getVersion(model)))) { - getLog().debug(" module is " - + ArtifactUtils.versionlessKey( - PomHelper.getGroupId(model), PomHelper.getArtifactId(model)) - + ":" - + PomHelper.getVersion(model)); - getLog().debug(" will become " - + ArtifactUtils.versionlessKey( - PomHelper.getGroupId(model), PomHelper.getArtifactId(model)) - + ":" + newVersion); - addChange( - PomHelper.getGroupId(model), - PomHelper.getArtifactId(model), - PomHelper.getVersion(model), - newVersion); - } - })); + + for (Map.Entry stringModelEntry : processAllModules + ? reactor.entrySet() + : PomHelper.getChildModels(reactor, sourceGroupId, sourceArtifactId) + .entrySet()) { + final Model targetModel = stringModelEntry.getValue(); + + if (Objects.equals(PomHelper.getGroupId(targetModel), groupId) + && Objects.equals(PomHelper.getArtifactId(targetModel), artifactId)) { + // skip updating the same pom again + continue; + } + + final Parent parent = targetModel.getParent(); + getLog().debug("Module: " + stringModelEntry.getKey()); + if (parent != null && sourceVersion.equals(parent.getVersion())) { + getLog().debug(" parent already is " + + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + ":" + sourceVersion); + } else { + getLog().debug(" parent is " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + + ":" + (parent == null ? "" : parent.getVersion())); + getLog().debug(" will become " + ArtifactUtils.versionlessKey(sourceGroupId, sourceArtifactId) + + ":" + sourceVersion); + } + final boolean targetExplicit = PomHelper.isExplicitVersion(targetModel); + if ((updateMatchingVersions || !targetExplicit) // + && (parent != null && Objects.equals(parent.getVersion(), PomHelper.getVersion(targetModel)))) { + getLog().debug(" module is " + + ArtifactUtils.versionlessKey( + PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel)) + + ":" + + PomHelper.getVersion(targetModel)); + getLog().debug(" will become " + + ArtifactUtils.versionlessKey( + PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel)) + + ":" + sourceVersion); + addChange( + PomHelper.getGroupId(targetModel), + PomHelper.getArtifactId(targetModel), + PomHelper.getVersion(targetModel), + sourceVersion); + targetModel.setVersion(sourceVersion); + } else { + getLog().debug(" module is " + + ArtifactUtils.versionlessKey( + PomHelper.getGroupId(targetModel), PomHelper.getArtifactId(targetModel)) + + ":" + + PomHelper.getVersion(targetModel)); + } + } } } 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 1c5bdfc279..898d895bd8 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 @@ -5,16 +5,10 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; import java.util.function.Consumer; import java.util.stream.Stream; -import org.apache.commons.lang3.tuple.ImmutablePair; -import org.apache.commons.lang3.tuple.Pair; import org.apache.maven.model.Model; -import org.apache.maven.model.Parent; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugin.testing.AbstractMojoTestCase; @@ -29,12 +23,10 @@ import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.matchesPattern; import static org.hamcrest.Matchers.matchesRegex; import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; public class SetMojoTest extends AbstractMojoTestCase { @Rule @@ -239,53 +231,4 @@ public void testIssue1042() throws Exception { String.join("", Files.readAllLines(tempDir.resolve("child-webapp/pom.xml"))), matchesRegex(".*\\Qchild-webapp\\E\\s*" + "\\Q1.0\\E.*")); } - - @Test - public void testComputeChildren() { - Map, Set> children = SetMojo.computeChildren(new HashMap() { - { - put(new File("parent"), new Model() { - { - setArtifactId("parent"); - setParent(new Parent() { - { - setGroupId("default"); - setArtifactId("superParent"); - } - }); - } - }); - put(new File("child2"), new Model() { - { - setArtifactId("child2"); - setParent(new Parent() { - { - setGroupId("default"); - setArtifactId("superParent"); - } - }); - } - }); - put(new File("superParent"), new Model() { - { - setArtifactId("superParent"); - } - }); - put(new File("child"), new Model() { - { - setArtifactId("child"); - setParent(new Parent() { - { - setGroupId("default"); - setArtifactId("parent"); - } - }); - } - }); - } - }); - assertThat(children.get(new ImmutablePair<>("default", "superParent")), hasSize(2)); - assertThat(children.get(new ImmutablePair<>("default", "parent")), hasSize(1)); - assertThat(children.get(new ImmutablePair<>("default", "child")), is(nullValue())); - } }