You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/08/31 07:35:13 UTC
(maven) branch backport/12938-to-maven-4.0.x created (now b7a7103e24)
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a change to branch backport/12938-to-maven-4.0.x
in repository https://gitbox.apache.org/repos/asf/maven.git
at b7a7103e24 Fix #12931: do not rewrite unmodified POMs in mvnup apply (#12938)
This branch includes the following new commits:
new b7a7103e24 Fix #12931: do not rewrite unmodified POMs in mvnup apply (#12938)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(maven) 01/01: Fix #12931: do not rewrite unmodified POMs in mvnup apply (#12938)
Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch backport/12938-to-maven-4.0.x
in repository https://gitbox.apache.org/repos/asf/maven.git
commit b7a7103e245c640e42c634ec2635ae9c082f87bc
Author: Burak Kalaycı <ka...@gmail.com>
AuthorDate: Mon Aug 31 10:34:24 2026 +0300
Fix #12931: do not rewrite unmodified POMs in mvnup apply (#12938)
* Fix #12931: do not rewrite unmodified POMs in mvnup apply
mvnup apply always logged "Saving modified POMs..." and wrote every
discovered POM, even when no strategy reported changes. Save only the
POMs in UpgradeResult.modifiedPoms and skip the save report when that
set is empty.
* Don't call saveModifications when no POM was modified
Skip the save path in execute when modifiedPoms is empty, instead of
returning early inside saveModifications.
---
.../invoker/mvnup/goals/AbstractUpgradeGoal.java | 26 ++++---
.../mvnup/goals/AbstractUpgradeGoalTest.java | 80 +++++++++++++++++++++-
2 files changed, 96 insertions(+), 10 deletions(-)
diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java
index 0dc73c968c..bb2966edb2 100644
--- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java
+++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoal.java
@@ -25,6 +25,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import eu.maveniverse.domtrip.Document;
import eu.maveniverse.domtrip.DomTripException;
@@ -189,21 +190,23 @@ public int execute(UpgradeContext context) throws Exception {
context.info("Found " + pomMap.size() + " POM file(s)");
// Perform the upgrade logic
- int result = doUpgrade(context, targetModel, pomMap);
+ UpgradeResult result = doUpgrade(context, targetModel, pomMap);
- // Save modifications if this is an apply goal
- if (shouldSaveModifications() && result == 0) {
- saveModifications(context, pomMap);
+ // Save modifications if this is an apply goal and anything actually changed
+ if (shouldSaveModifications()
+ && result.success()
+ && !result.modifiedPoms().isEmpty()) {
+ saveModifications(context, pomMap, result.modifiedPoms());
}
- return result;
+ return result.success() ? 0 : 1;
}
/**
* Performs the upgrade logic using the strategy pattern.
* Delegates to StrategyOrchestrator for coordinated strategy execution.
*/
- protected int doUpgrade(UpgradeContext context, String targetModel, Map<Path, Document> pomMap) {
+ protected UpgradeResult doUpgrade(UpgradeContext context, String targetModel, Map<Path, Document> pomMap) {
// Execute strategies using the orchestrator
try {
UpgradeResult result = orchestrator.executeStrategies(context, pomMap);
@@ -215,10 +218,11 @@ protected int doUpgrade(UpgradeContext context, String targetModel, Map<Path, Do
// Fix incompatible extensions in .mvn/extensions.xml
fixIncompatibleExtensions(context);
- return result.errorPoms().isEmpty() ? 0 : 1;
+ return result;
} catch (Exception e) {
context.failure("Strategy execution failed: " + e.getMessage());
- return 1;
+ Set<Path> errorPoms = pomMap.keySet().isEmpty() ? Set.of(Path.of(".")) : pomMap.keySet();
+ return UpgradeResult.failure(pomMap.keySet(), errorPoms);
}
}
@@ -230,13 +234,17 @@ protected int doUpgrade(UpgradeContext context, String targetModel, Map<Path, Do
/**
* Saves the modified documents to disk using domtrip's perfect formatting preservation.
+ * Unmodified POMs are left untouched and are not reported as saved.
*/
- protected void saveModifications(UpgradeContext context, Map<Path, Document> pomMap) {
+ protected void saveModifications(UpgradeContext context, Map<Path, Document> pomMap, Set<Path> modifiedPoms) {
context.info("");
context.info("Saving modified POMs...");
for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
Path pomPath = entry.getKey();
+ if (!modifiedPoms.contains(pomPath)) {
+ continue;
+ }
Document document = entry.getValue();
try {
// Use domtrip for perfect formatting preservation
diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoalTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoalTest.java
index a3b7a86b2a..16f07b467e 100644
--- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoalTest.java
+++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeGoalTest.java
@@ -20,8 +20,10 @@
import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.attribute.FileTime;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.stream.Stream;
import eu.maveniverse.domtrip.Document;
@@ -41,6 +43,8 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
/**
@@ -533,6 +537,80 @@ void shouldBeNoOpWhenNoIncompatibleExtensions() throws Exception {
}
}
+ @Nested
+ @DisplayName("POM Saving")
+ class PomSavingTests {
+
+ @Test
+ @DisplayName("should not rewrite or report POMs when none were modified")
+ void shouldNotRewriteOrReportWhenNoneModified() throws Exception {
+ Path pom = tempDir.resolve("pom.xml");
+ Files.writeString(pom, minimalPom("root"));
+ Files.createDirectories(tempDir.resolve(".mvn"));
+ Files.setLastModifiedTime(pom, FileTime.fromMillis(0));
+
+ UpgradeContext context = createMockContext(tempDir);
+ when(mockOrchestrator.executeStrategies(Mockito.any(), Mockito.any()))
+ .thenReturn(UpgradeResult.success(Set.of(pom), Set.of()));
+
+ int result = upgradeGoal.execute(context);
+
+ assertEquals(0, result);
+ assertEquals(0, Files.getLastModifiedTime(pom).toMillis(), "Unmodified POM must not be rewritten");
+ verify(context.logger, never()).info("Saving modified POMs...");
+ }
+
+ @Test
+ @DisplayName("should write and report only the POMs that were modified")
+ void shouldWriteOnlyModifiedPoms() throws Exception {
+ Path parentPom = tempDir.resolve("pom.xml");
+ Path childDir = tempDir.resolve("child");
+ Path childPom = childDir.resolve("pom.xml");
+ Files.createDirectories(childDir);
+ Files.writeString(parentPom, """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>parent</artifactId>
+ <version>1.0.0</version>
+ <packaging>pom</packaging>
+ <modules>
+ <module>child</module>
+ </modules>
+ </project>
+ """);
+ Files.writeString(childPom, minimalPom("child"));
+ Files.createDirectories(tempDir.resolve(".mvn"));
+ Files.setLastModifiedTime(parentPom, FileTime.fromMillis(0));
+ Files.setLastModifiedTime(childPom, FileTime.fromMillis(0));
+
+ UpgradeContext context = createMockContext(tempDir);
+ when(mockOrchestrator.executeStrategies(Mockito.any(), Mockito.any()))
+ .thenReturn(UpgradeResult.success(Set.of(parentPom, childPom), Set.of(childPom)));
+
+ int result = upgradeGoal.execute(context);
+
+ assertEquals(0, result);
+ assertEquals(
+ 0, Files.getLastModifiedTime(parentPom).toMillis(), "Unmodified parent POM must not be rewritten");
+ assertTrue(Files.getLastModifiedTime(childPom).toMillis() > 0, "Modified child POM must be saved");
+ verify(context.logger).info("Saving modified POMs...");
+ }
+
+ private String minimalPom(String artifactId) {
+ return """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>test</groupId>
+ <artifactId>%s</artifactId>
+ <version>1.0.0</version>
+ </project>
+ """.formatted(artifactId);
+ }
+ }
+
/**
* Testable subclass that exposes protected methods for testing.
*/
@@ -567,7 +645,7 @@ public boolean testIsPluginsEnabled(UpgradeContext context) {
public int testExecuteWithTargetModel(UpgradeContext context, String targetModel) {
try {
Map<Path, Document> pomMap = Map.of(); // Empty for this test
- return doUpgrade(context, targetModel, pomMap);
+ return doUpgrade(context, targetModel, pomMap).success() ? 0 : 1;
} catch (Exception e) {
throw new RuntimeException(e);
}