Skip to content

Commit

Permalink
[MRELEASE-1154] [REGRESSION] MRELEASE-1109 breaks release of Maven Su…
Browse files Browse the repository at this point in the history 
…refire

This reverts commit 9e0713b (MRELEASE-1109).

This closes #229
  • Loading branch information
@michael-o
michael-o committed Aug 23, 2024
1 parent befc778 commit b87eeb6
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 282 deletions.
150 changes: 48 additions & 102 deletions ...manager/src/main/java/org/apache/maven/shared/release/phase/AbstractRewritePomsPhase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,12 @@
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;
Expand Down Expand Up @@ -92,18 +89,6 @@ 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(
Expand Down Expand Up @@ -281,7 +266,7 @@ private void transformDocument(

Properties properties = modelTarget.getProperties();

String parentVersion = rewriteParent(project, modelTarget, result, releaseDescriptor, simulate);
String parentVersion = rewriteParent(project, modelTarget, releaseDescriptor, simulate);

String projectId = ArtifactUtils.versionlessKey(project.getGroupId(), project.getArtifactId());

Expand Down Expand Up @@ -455,29 +440,8 @@ 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,
ReleaseResult result,
ReleaseDescriptor releaseDescriptor,
boolean simulate)
MavenProject project, Model targetModel, ReleaseDescriptor releaseDescriptor, boolean simulate)
throws ReleaseFailureException {
String parentVersion = null;
if (project.hasParent()) {
Expand All @@ -494,13 +458,7 @@ private String rewriteParent(
throw new ReleaseFailureException("Version for parent '" + parent.getName() + "' was not mapped");
}
} else {
if (!isCiFriendlyVersion(targetModel.getParent().getVersion())) {
targetModel.getParent().setVersion(parentVersion);
} else {
logInfo(
result,
" Ignoring parent version update for CI friendly expression " + parent.getVersion());
}
targetModel.getParent().setVersion(parentVersion);
}
}
return parentVersion;
Expand Down Expand Up @@ -563,73 +521,61 @@ private void rewriteArtifactVersions(
if (rawVersion.equals(originalVersion)) {
logInfo(result, " Updating " + artifactId + " to " + mappedVersion);
coordinate.setVersion(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 " + 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)) {
} 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)) {
logInfo(
result,
" Ignoring artifact version update for CI friendly expression "
+ rawVersion);
" Ignoring artifact version update for expression " + mappedVersion);
// ignore... we cannot update this expression
} 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(
"Could not find property resolving version expression: " + rawVersion);
// 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 + ").");
}
}
} 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(
"Could not find properties resolving version expression : " + rawVersion);
throw new ReleaseFailureException("The version could not be updated: " + 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);
Expand Down
17 changes: 2 additions & 15 deletions ...ease-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomModel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
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;
Expand Down Expand Up @@ -179,9 +178,7 @@ public void setVersion(String version) {
}

if (versionElement == null) {
// never add version when parent references CI friendly property
if (!(parentVersion != null && AbstractRewritePomsPhase.isCiFriendlyVersion(parentVersion))
&& !version.equals(parentVersion)) {
if (!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);
Expand All @@ -192,17 +189,7 @@ public void setVersion(String version) {
project.addContent(index + 2, versionElement);
}
} else {
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);
}
JDomUtils.rewriteValue(versionElement, version);
}
}
}
2 changes: 1 addition & 1 deletion ...ase-manager/src/main/java/org/apache/maven/shared/release/transform/jdom2/JDomParent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public JDomParent(Element parent) {

@Override
public String getVersion() {
return parent.getChildText("version", parent.getNamespace());
throw new UnsupportedOperationException();
}

@Override
Expand Down
14 changes: 0 additions & 14 deletions ...r/src/test/java/org/apache/maven/shared/release/phase/RewritePomsForReleasePhaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,20 +320,6 @@ 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 {
Expand Down
9 changes: 1 addition & 8 deletions ...-manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomModelTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,7 @@ public void testSetVersion() throws Exception {
model.setVersion(null);
assertNull(model.getVersion());

// 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));

// inherit from parent
// this business logic might need to moved.
content = "<project><parent><version>PARENT_VERSION</version></parent></project>";
projectElm = builder.build(new StringReader(content)).getRootElement();
Expand Down
11 changes: 3 additions & 8 deletions ...manager/src/test/java/org/apache/maven/shared/release/transform/jdom2/JDomParentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@
*/
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;

Expand All @@ -46,12 +44,9 @@ public void testGetRelativePath() {
new JDomParent(null).getRelativePath();
}

@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)
public void testGetVersion() {
new JDomParent(null).getVersion();
}

@Test(expected = UnsupportedOperationException.class)
Expand Down
39 changes: 0 additions & 39 deletions .../projects/rewrite-for-release/pom-with-parent-and-cifriendly-expressions/expected-pom.xml
View file

This file was deleted.

Loading

0 comments on commit b87eeb6

Please sign in to comment.