gnodet commented on code in PR #13059:
URL: https://github.com/apache/maven/pull/13059#discussion_r3960557441
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java:
##########
@@ -523,49 +526,81 @@ && isPropertyUsedByQuarkusBom(pomDocument, propertyName))
{
/**
* Upgrades a property value if it represents a plugin version below the
minimum.
+ * First checks the current POM's properties, then searches other POMs in
the project
+ * (e.g., parent POMs) if the property is not found locally.
*/
private boolean upgradePropertyVersion(
Document pomDocument,
+ Map<Path, Document> pomMap,
String propertyName,
PluginUpgradeInfo upgrade,
String sectionName,
UpgradeContext context) {
- Editor editor = new Editor(pomDocument);
- Element root = editor.root();
+ // First, try the current POM's properties
+ if (upgradePropertyInDocument(pomDocument, propertyName, upgrade,
sectionName, context)) {
+ return true;
+ }
+
+ // Property not found or not upgradable in current POM — search other
POMs in the project
+ for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
+ Document otherDoc = entry.getValue();
+ if (otherDoc == pomDocument) {
+ continue; // Skip the current POM, already checked
+ }
+ if (upgradePropertyInDocument(otherDoc, propertyName, upgrade,
sectionName, context)) {
+ return true;
+ }
+ }
+
+ // Property not found anywhere in the project
+ context.warning("Property " + propertyName + " not found in any
project POM properties");
Review Comment:
Fixed in 63c472cc51. Added an existence check after
`upgradePropertyInDocument` returns false — if the property is present in the
current POM's `<properties>` (or any sibling POM's), we return false
immediately without the cross-POM search or the spurious warning. Also added a
test for the exact scenario (property already at 3.5.0 + submodule reference →
no warning).
##########
impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategyTest.java:
##########
@@ -712,6 +712,106 @@ void shouldNotUpgradeWhenPropertyNotFound() throws
Exception {
assertTrue(result.success(), "Plugin upgrade should succeed");
// Note: POM might still be modified due to plugin management
additions
}
+
+ @Test
+ @DisplayName("should upgrade plugin with property version defined in
parent POM")
+ void shouldUpgradePluginWithPropertyVersionInParentPom() throws
Exception {
+ // Simulates hbase pattern: root POM defines
<exec.maven.version>3.1.0</exec.maven.version>
+ // and submodule uses <version>${exec.maven.version}</version>
+ String parentPomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0";>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>org.example</groupId>
+ <artifactId>parent</artifactId>
+ <version>1.0.0</version>
+ <packaging>pom</packaging>
+ <properties>
+ <exec.maven.version>3.1.0</exec.maven.version>
+ </properties>
+ <modules>
+ <module>assembly</module>
+ </modules>
+ </project>
+ """;
+
+ String submodulePomXml = """
+ <?xml version="1.0" encoding="UTF-8"?>
+ <project xmlns="http://maven.apache.org/POM/4.0.0";>
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.example</groupId>
+ <artifactId>parent</artifactId>
+ <version>1.0.0</version>
+ </parent>
+ <artifactId>assembly</artifactId>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>exec-maven-plugin</artifactId>
+ <version>${exec.maven.version}</version>
+ </plugin>
+ </plugins>
+ </build>
+ </project>
+ """;
+
+ Path tempDir = Files.createTempDirectory("mvnup-test-");
+ try {
+ Files.createDirectories(tempDir.resolve(".mvn"));
+ Path parentPomPath = tempDir.resolve("pom.xml");
+ Files.writeString(parentPomPath, parentPomXml);
+ Path assemblyDir = tempDir.resolve("assembly");
+ Files.createDirectories(assemblyDir);
+ Path submodulePomPath = assemblyDir.resolve("pom.xml");
+ Files.writeString(submodulePomPath, submodulePomXml);
Review Comment:
Fixed in 63c472cc51. Removed the dead
`Files.createDirectories`/`Files.writeString` calls — the tests now create a
temp dir only for path structure (so `relativize` works in
`createTempProjectStructure`), but skip writing the POM files since `doApply`
handles that internally. Applied the same cleanup to
`shouldUpgradeExecPluginInSubmodule` for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
