[
https://issues.apache.org/jira/browse/MRELEASE-1109?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17851956#comment-17851956
]
ASF GitHub Bot commented on MRELEASE-1109:
------------------------------------------
michael-o commented on code in PR #202:
URL: https://github.com/apache/maven-release/pull/202#discussion_r1625608386
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/util/CiFriendlyVersion.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.apache.maven.shared.release.util;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.maven.artifact.ArtifactUtils;
+
+public class CiFriendlyVersion {
+
+ /**
+ * 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 String REVISION = "revision";
+
+ private static final String SHA_1 = "sha1";
Review Comment:
Please make it `SHA1`. We have this style everywhere.
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomProperties.java:
##########
@@ -51,10 +51,13 @@ public JDomProperties(Element properties) {
}
@Override
- public synchronized Object setProperty(String key, String value) {
- Element property = properties.getChild(key, properties.getNamespace());
+ public synchronized Object put(Object key, Object value) {
+ Element property = properties.getChild((String) key,
properties.getNamespace());
- JDomUtils.rewriteValue(property, value);
+ if (property == null) {
+ property = new Element((String) key, properties.getNamespace());
Review Comment:
I do not understand this logic. Why is this a `put` here?
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/util/CiFriendlyVersion.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.apache.maven.shared.release.util;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.maven.artifact.ArtifactUtils;
+
+public class CiFriendlyVersion {
+
+ /**
+ * 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 String REVISION = "revision";
+
+ private static final String SHA_1 = "sha1";
+ private static final String CHANGELIST = "changelist";
+ private static final List<String> CI_FRIENDLY_PROPERTIES =
Arrays.asList(REVISION, SHA_1, CHANGELIST);
Review Comment:
This should be a `Collection` of type `HashSet`, no?
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/util/CiFriendlyVersion.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.apache.maven.shared.release.util;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.maven.artifact.ArtifactUtils;
+
+public class CiFriendlyVersion {
+
+ /**
+ * 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 String REVISION = "revision";
+
+ private static final String SHA_1 = "sha1";
+ private static final String CHANGELIST = "changelist";
+ private static final List<String> CI_FRIENDLY_PROPERTIES =
Arrays.asList(REVISION, SHA_1, CHANGELIST);
+
+ private static final String SNAPSHOT = "-SNAPSHOT";
+
+ private CiFriendlyVersion() {}
+
+ /**
+ * 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.find()) {
+ return null;
+ }
+ return matcher.group(1);
+ }
+
+ public static boolean isCiFriendlyVersion(String version) {
+ return
containsCiFriendlyProperties(extractPropertyFromExpression(version));
+ }
+
+ public static boolean containsCiFriendlyProperties(String property) {
+ return CI_FRIENDLY_PROPERTIES.contains(property);
+ }
+
+ public static void rewriteVersionAndProperties(String version, String
versionElement, Properties properties) {
+ // try to rewrite property if CI friendly expression is used
+ String ciFriendlyPropertyName =
extractPropertyFromExpression(versionElement);
+ if (properties != null) {
+ String sha1 = properties.getProperty(SHA_1,
System.getProperty(SHA_1, ""));
Review Comment:
Was `System.getProperty()` used before? We tend *not* to rely on system
properties, but only on Maven properties.
@cstamas @slawekjaranowski
> update-versions removes the CI-friendly ${revisions}
> ----------------------------------------------------
>
> Key: MRELEASE-1109
> URL: https://issues.apache.org/jira/browse/MRELEASE-1109
> Project: Maven Release Plugin
> Issue Type: Bug
> Components: prepare, update-versions
> Affects Versions: 2.5.3, 3.0.0-M7
> Reporter: Marcel Stör
> Assignee: Konrad Windszus
> Priority: Major
> Fix For: next-release
>
>
> Given: a project using CI-friendly versions as per
> [https://maven.apache.org/maven-ci-friendly.html]
> {code:xml}
> <version>${revision}</version>
> ...
> <properties>
> <revision>1.0.0-SNAPSHOT</revision>
> </properties>
> {code}
> If I run {{mvn release:update-versions}} (with or without
> {{{}-DautoVersionSubmodules=true{}}}) I expect the release plugin to change
> the {{$revision}} property. Instead it blindly replaces
> {{<version>${revision}</version>}} with the hard-coded version set on the CLI.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
