[MRELEASE-1109] Support CI friendly versions

maven-release-manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java

@@ -23,12 +23,16 @@
 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.Optional;
 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 +93,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 <a href="https://maven.apache.org/maven-ci-friendly.html">CI-Friendly Versions</a>
+     */
+    private static final List<String> CI_FRIENDLY_PROPERTIES = Arrays.asList("revision", "sha1", "changelist");
+
     private long startTime = -1 * 1000;
 
     protected AbstractRewritePomsPhase(
@@ -266,7 +282,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 +456,31 @@ 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 Optional<String> extractPropertyFromExpression(String expression) {
+        Matcher matcher = EXPRESSION_PATTERN.matcher(expression);
+        if (!matcher.matches()) {
+            return Optional.empty();
+        }
+        return Optional.of(matcher.group(1));
+    }
+
+    public static boolean isCiFriendlyVersion(String version) {
+        return extractPropertyFromExpression(version)
+                .map(CI_FRIENDLY_PROPERTIES::contains)
+                .orElse(false);
+    }
+
     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 +497,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 +566,74 @@ 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 {
+                    Optional<String> optionalProperty = extractPropertyFromExpression(rawVersion);
+                    if (optionalProperty.isPresent()) {
+                        String property = optionalProperty.get();
+                        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);

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,19 @@ 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())
+                        .orElseThrow(() -> new IllegalArgumentException(
+                                "Could not retrieve property from CI friendly expression"));
+                Properties properties = getProperties();
+                if (properties != null) {
+                    properties.computeIfPresent(ciFriendlyPropertyName, (k, v) -> version);
+                }
+            } else {
+                JDomUtils.rewriteValue(versionElement, version);
+            }
         }
     }
 }

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

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<MavenProject> 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 {

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 = "<project><parent><version>${revision}</version></parent></project>";
+        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 = "<project><parent><version>PARENT_VERSION</version></parent></project>";
         projectElm = builder.build(new StringReader(content)).getRootElement();

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 = "<parent><version>1.0</version></parent>";
+        Element parentElm = builder.build(new StringReader(content)).getRootElement();
+
+        assertEquals("1.0", new JDomParent(parentElm).getVersion());
     }
 
     @Test(expected = UnsupportedOperationException.class)

maven-release-manager/src/test/resources/projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/expected-pom.xml

@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+  ~ Copyright 2005-2006 The Apache Software Foundation.
+  ~
+  ~ Licensed under the Apache License, Version 2.0 (the "License");
+  ~ you may not use this file except in compliance with the License.
+  ~ You may obtain a copy of the License at
+  ~
+  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+
+<project>
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>groupId</groupId>
+  <artifactId>artifactId</artifactId>
+  <version>${revision}</version>
+  <packaging>pom</packaging>
+
+  <scm>
+    <connection>scm:svn:file://localhost/tmp/scm-repo/tags/release-label</connection>
+    <developerConnection>scm:svn:file://localhost/tmp/scm-repo/tags/release-label</developerConnection>
+    <url>file://localhost/tmp/scm-repo/tags/release-label</url>
+  </scm>
+
+  <properties>
+    <revision>1.0-SNAPSHOT</revision>
+  </properties>
+
+  <modules>
+    <module>subproject1</module>
+  </modules>
+</project>