diff --git a/maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java b/maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java index d17865ecf..76747c563 100644 --- a/maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java +++ b/maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java @@ -23,12 +23,15 @@ import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Date; import java.util.List; import java.util.Map; import java.util.Properties; import java.util.TimeZone; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.maven.artifact.Artifact; import org.apache.maven.artifact.ArtifactUtils; @@ -89,6 +92,18 @@ public abstract class AbstractRewritePomsPhase extends AbstractReleasePhase impl */ private String modelETL = JDomModelETLFactory.NAME; + /** + * Regular expression pattern matching Maven expressions (i.e. references to Maven properties). + * The first group selects the property name the expression refers to. + */ + private static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{(.+)\\}"); + + /** + * All Maven properties allowed to be referenced in parent versions via expressions + * @see CI-Friendly Versions + */ + private static final List CI_FRIENDLY_PROPERTIES = Arrays.asList("revision", "sha1", "changelist"); + private long startTime = -1 * 1000; protected AbstractRewritePomsPhase( @@ -266,7 +281,7 @@ private void transformDocument( Properties properties = modelTarget.getProperties(); - String parentVersion = rewriteParent(project, modelTarget, releaseDescriptor, simulate); + String parentVersion = rewriteParent(project, modelTarget, result, releaseDescriptor, simulate); String projectId = ArtifactUtils.versionlessKey(project.getGroupId(), project.getArtifactId()); @@ -440,8 +455,29 @@ private void rewriteVersion( modelTarget.setVersion(version); } + /** + * Extracts the Maven property name from a given expression. + * @param expression the expression + * @return either {@code null} if value is no expression otherwise the property referenced in the expression + */ + public static String extractPropertyFromExpression(String expression) { + Matcher matcher = EXPRESSION_PATTERN.matcher(expression); + if (!matcher.matches()) { + return null; + } + return matcher.group(1); + } + + public static boolean isCiFriendlyVersion(String version) { + return CI_FRIENDLY_PROPERTIES.contains(extractPropertyFromExpression(version)); + } + private String rewriteParent( - MavenProject project, Model targetModel, ReleaseDescriptor releaseDescriptor, boolean simulate) + MavenProject project, + Model targetModel, + ReleaseResult result, + ReleaseDescriptor releaseDescriptor, + boolean simulate) throws ReleaseFailureException { String parentVersion = null; if (project.hasParent()) { @@ -458,7 +494,13 @@ private String rewriteParent( throw new ReleaseFailureException("Version for parent '" + parent.getName() + "' was not mapped"); } } else { - targetModel.getParent().setVersion(parentVersion); + if (!isCiFriendlyVersion(targetModel.getParent().getVersion())) { + targetModel.getParent().setVersion(parentVersion); + } else { + logInfo( + result, + " Ignoring parent version update for CI friendly expression " + parent.getVersion()); + } } } return parentVersion; @@ -521,61 +563,73 @@ private void rewriteArtifactVersions( if (rawVersion.equals(originalVersion)) { logInfo(result, " Updating " + artifactId + " to " + mappedVersion); coordinate.setVersion(mappedVersion); - } else if (rawVersion.matches("\\$\\{.+\\}")) { - String expression = rawVersion.substring(2, rawVersion.length() - 1); - - if (expression.startsWith("project.") - || expression.startsWith("pom.") - || "version".equals(expression)) { - if (!mappedVersion.equals(getNextVersion(releaseDescriptor, projectId))) { - logInfo(result, " Updating " + artifactId + " to " + mappedVersion); - coordinate.setVersion(mappedVersion); - } else { - logInfo(result, " Ignoring artifact version update for expression " + rawVersion); - } - } else if (properties != null) { - // version is an expression, check for properties to update instead - - String propertyValue = properties.getProperty(expression); - - if (propertyValue != null) { - if (propertyValue.equals(originalVersion)) { - logInfo(result, " Updating " + rawVersion + " to " + mappedVersion); - // change the property only if the property is the same as what's in the reactor - properties.setProperty(expression, mappedVersion); - } else if (mappedVersion.equals(propertyValue)) { - // this property may have been updated during processing a sibling. - logInfo( - result, - " Ignoring artifact version update for expression " + rawVersion - + " because it is already updated"); - } else if (!mappedVersion.equals(rawVersion)) { - // WARNING: ${pom.*} prefix support and ${version} is about to be dropped in mvn4! - // https://issues.apache.org/jira/browse/MNG-7404 - // https://issues.apache.org/jira/browse/MNG-7244 - if (mappedVersion.matches("\\$\\{project.+\\}") - || mappedVersion.matches("\\$\\{pom.+\\}") - || "${version}".equals(mappedVersion)) { + } else { + String property = extractPropertyFromExpression(rawVersion); + if (property != null) { + if (property.startsWith("project.") + || property.startsWith("pom.") + || "version".equals(property)) { + if (!mappedVersion.equals(getNextVersion(releaseDescriptor, projectId))) { + logInfo(result, " Updating " + artifactId + " to " + mappedVersion); + coordinate.setVersion(mappedVersion); + } else { + logInfo(result, " Ignoring artifact version update for expression " + rawVersion); + } + } else if (properties != null) { + // version is an expression, check for properties to update instead + String propertyValue = properties.getProperty(property); + if (propertyValue != null) { + if (propertyValue.equals(originalVersion)) { + logInfo(result, " Updating " + rawVersion + " to " + mappedVersion); + // change the property only if the property is the same as what's in the reactor + properties.setProperty(property, mappedVersion); + } else if (mappedVersion.equals(propertyValue)) { + // this property may have been updated during processing a sibling. logInfo( result, - " Ignoring artifact version update for expression " + mappedVersion); - // ignore... we cannot update this expression + " Ignoring artifact version update for expression " + rawVersion + + " because it is already updated"); + } else if (!mappedVersion.equals(rawVersion)) { + // WARNING: ${pom.*} prefix support and ${version} is about to be dropped in mvn4! + // https://issues.apache.org/jira/browse/MNG-7404 + // https://issues.apache.org/jira/browse/MNG-7244 + if (mappedVersion.matches("\\$\\{project.+\\}") + || mappedVersion.matches("\\$\\{pom.+\\}") + || "${version}".equals(mappedVersion)) { + logInfo( + result, + " Ignoring artifact version update for expression " + mappedVersion); + // ignore... we cannot update this expression + } else { + // the value of the expression conflicts with what the user wanted to release + throw new ReleaseFailureException("The artifact (" + key + ") requires a " + + "different version (" + mappedVersion + ") than what is found (" + + propertyValue + ") for the expression (" + rawVersion + ") in the " + + "project (" + projectId + ")."); + } + } + } else { + if (CI_FRIENDLY_PROPERTIES.contains(property)) { + logInfo( + result, + " Ignoring artifact version update for CI friendly expression " + + rawVersion); } else { - // the value of the expression conflicts with what the user wanted to release - throw new ReleaseFailureException("The artifact (" + key + ") requires a " - + "different version (" + mappedVersion + ") than what is found (" - + propertyValue + ") for the expression (" + expression + ") in the " - + "project (" + projectId + ")."); + // the expression used to define the version of this artifact may be inherited + // TODO needs a better error message, what pom? what dependency? + throw new ReleaseFailureException( + "Could not find property resolving version expression: " + rawVersion); } } } else { // the expression used to define the version of this artifact may be inherited // TODO needs a better error message, what pom? what dependency? - throw new ReleaseFailureException("The version could not be updated: " + rawVersion); + throw new ReleaseFailureException( + "Could not find properties resolving version expression : " + rawVersion); } + } else { + // different/previous version not related to current release } - } else { - // different/previous version not related to current release } } else if (resolvedSnapshotVersion != null) { logInfo(result, " Updating " + artifactId + " to " + resolvedSnapshotVersion); diff --git a/maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomModel.java b/maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomModel.java index d219b6cdd..6198ee73d 100644 --- a/maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomModel.java +++ b/maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomModel.java @@ -31,6 +31,7 @@ import org.apache.maven.model.Profile; import org.apache.maven.model.Reporting; import org.apache.maven.model.Scm; +import org.apache.maven.shared.release.phase.AbstractRewritePomsPhase; import org.jdom2.Document; import org.jdom2.Element; import org.jdom2.Text; @@ -178,7 +179,9 @@ public void setVersion(String version) { } if (versionElement == null) { - if (!version.equals(parentVersion)) { + // never add version when parent references CI friendly property + if (!(parentVersion != null && AbstractRewritePomsPhase.isCiFriendlyVersion(parentVersion)) + && !version.equals(parentVersion)) { // we will add this after artifactId, since it was missing but different from the inherited version Element artifactIdElement = project.getChild("artifactId", project.getNamespace()); int index = project.indexOf(artifactIdElement); @@ -189,7 +192,17 @@ public void setVersion(String version) { project.addContent(index + 2, versionElement); } } else { - JDomUtils.rewriteValue(versionElement, version); + if (AbstractRewritePomsPhase.isCiFriendlyVersion(versionElement.getTextNormalize())) { + // try to rewrite property if CI friendly expression is used + String ciFriendlyPropertyName = + AbstractRewritePomsPhase.extractPropertyFromExpression(versionElement.getTextNormalize()); + Properties properties = getProperties(); + if (properties != null) { + properties.computeIfPresent(ciFriendlyPropertyName, (k, v) -> version); + } + } else { + JDomUtils.rewriteValue(versionElement, version); + } } } } diff --git a/maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomParent.java b/maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomParent.java index bbfc1ab78..bfda08fc1 100644 --- a/maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomParent.java +++ b/maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomParent.java @@ -41,7 +41,7 @@ public JDomParent(Element parent) { @Override public String getVersion() { - throw new UnsupportedOperationException(); + return parent.getChildText("version", parent.getNamespace()); } @Override diff --git a/maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/RewritePomsForReleasePhaseTest.java b/maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/RewritePomsForReleasePhaseTest.java index 138f4bfc1..71515aed7 100644 --- a/maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/RewritePomsForReleasePhaseTest.java +++ b/maven-release-manager/src/test/java/org/apache/maven/shared/release/phase/RewritePomsForReleasePhaseTest.java @@ -320,6 +320,20 @@ public void testRewritePomWithParentAndProperties() throws Exception { assertTrue(comparePomFiles(reactorProjects)); } + // MRELEASE-1109 + @Test + public void testRewritePomWithCiFriendlyReactor() throws Exception { + List reactorProjects = createReactorProjects("pom-with-parent-and-cifriendly-expressions"); + + ReleaseDescriptorBuilder builder = + createDescriptorFromProjects(reactorProjects, "pom-with-parent-and-cifriendly-expressions"); + builder.addReleaseVersion("groupId:artifactId", NEXT_VERSION); + builder.addReleaseVersion("groupId:subproject1", NEXT_VERSION); + phase.execute(ReleaseUtils.buildReleaseDescriptor(builder), new DefaultReleaseEnvironment(), reactorProjects); + + assertTrue(comparePomFiles(reactorProjects)); + } + // MRELEASE-311 @Test public void testRewritePomWithDependencyPropertyCoordinate() throws Exception { diff --git a/maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomModelTest.java b/maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomModelTest.java index 43dec7e06..ebd0ec282 100644 --- a/maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomModelTest.java +++ b/maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomModelTest.java @@ -68,7 +68,14 @@ public void testSetVersion() throws Exception { model.setVersion(null); assertNull(model.getVersion()); - // inherit from parent + // inherit from parent via CI friendly + content = "${revision}"; + projectElm = builder.build(new StringReader(content)).getRootElement(); + model = new JDomModel(projectElm); + assertNull(model.getVersion()); + model.setVersion("PARENT_VERSION"); + assertNull(getVersion(projectElm)); + // this business logic might need to moved. content = "PARENT_VERSION"; projectElm = builder.build(new StringReader(content)).getRootElement(); diff --git a/maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomParentTest.java b/maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomParentTest.java index 5f2c88ee4..036811ef6 100644 --- a/maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomParentTest.java +++ b/maven-release-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomParentTest.java @@ -18,9 +18,11 @@ */ package org.apache.maven.shared.release.transform.jdom2; +import java.io.IOException; import java.io.StringReader; import org.jdom2.Element; +import org.jdom2.JDOMException; import org.jdom2.input.SAXBuilder; import org.junit.Test; @@ -44,9 +46,12 @@ public void testGetRelativePath() { new JDomParent(null).getRelativePath(); } - @Test(expected = UnsupportedOperationException.class) - public void testGetVersion() { - new JDomParent(null).getVersion(); + @Test + public void testGetVersion() throws JDOMException, IOException { + String content = "1.0"; + Element parentElm = builder.build(new StringReader(content)).getRootElement(); + + assertEquals("1.0", new JDomParent(parentElm).getVersion()); } @Test(expected = UnsupportedOperationException.class) diff --git a/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/expected-pom.xml b/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/expected-pom.xml new file mode 100644 index 000000000..c75040633 --- /dev/null +++ b/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/expected-pom.xml @@ -0,0 +1,39 @@ + + + + + + 4.0.0 + groupId + artifactId + ${revision} + pom + + + scm:svn:file://localhost/tmp/scm-repo/tags/release-label + scm:svn:file://localhost/tmp/scm-repo/tags/release-label + file://localhost/tmp/scm-repo/tags/release-label + + + + 1.0-SNAPSHOT + + + + subproject1 + + diff --git a/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/pom.xml b/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/pom.xml new file mode 100644 index 000000000..0c9bd8b28 --- /dev/null +++ b/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/pom.xml @@ -0,0 +1,39 @@ + + + + + + 4.0.0 + groupId + artifactId + ${revision} + pom + + + scm:svn:file://localhost/tmp/scm-repo/trunk + scm:svn:file://localhost/tmp/scm-repo/trunk + file://localhost/tmp/scm-repo/trunk + + + + 1.0-SNAPSHOT + + + + subproject1 + + diff --git a/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/subproject1/expected-pom.xml b/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/subproject1/expected-pom.xml new file mode 100644 index 000000000..7b2fa88c1 --- /dev/null +++ b/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/subproject1/expected-pom.xml @@ -0,0 +1,28 @@ + + + + + + 4.0.0 + + groupId + artifactId + ${revision} + + + subproject1 + diff --git a/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/subproject1/pom.xml b/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/subproject1/pom.xml new file mode 100644 index 000000000..7b2fa88c1 --- /dev/null +++ b/maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/subproject1/pom.xml @@ -0,0 +1,28 @@ + + + + + + 4.0.0 + + groupId + artifactId + ${revision} + + + subproject1 +