This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch maven-4.0.x in repository https://gitbox.apache.org/repos/asf/maven.git
commit a0ed03ca9ffac39e86f204cfa5035eb6fe0cb924 Author: Guillaume Nodet <[email protected]> AuthorDate: Tue May 26 16:13:16 2026 +0200 Fix #12080: mvnup - comment out dependencies with undefined property expressions Comment out dependencies whose version uses a property expression that is not defined in any POM in the reactor. Scans <properties> sections including those in profiles. Well-known properties (project.*, env.*, maven.*, etc.) are excluded from the check. Also fixes domtrip API migration (child→childElement, children→childElements) needed for 4.0.x branch compatibility. Co-Authored-By: Claude Opus 4.6 <[email protected]> --- .github/workflows/maven.yml | 12 +- .../repository/metadata/ArtifactMetadata.java | 2 +- .../repository/metadata/ArtifactMetadataTest.java | 68 ---- .../apache/maven/cling/invoker/LookupInvoker.java | 10 +- .../mvnup/goals/CompatibilityFixStrategy.java | 155 ++++++- .../mvnup/goals/CompatibilityFixStrategyTest.java | 443 +++++++++++++++++++++ .../maven/impl/model/DefaultModelValidator.java | 2 +- its/pom.xml | 2 +- pom.xml | 9 +- 9 files changed, 610 insertions(+), 93 deletions(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index fb4275028e..555df17614 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -32,8 +32,7 @@ concurrency: permissions: {} env: - # Keep in sync with mimir testing version in pom.xml - MIMIR_VERSION: 0.11.4 + MIMIR_VERSION: 0.11.2 MIMIR_BASEDIR: ~/.mimir MIMIR_LOCAL: ~/.mimir/local MAVEN_OPTS: -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=./target/java_heapdump.hprof @@ -54,15 +53,6 @@ jobs: with: persist-credentials: false - - name: Verify Mimir version alignment - shell: bash - run: | - POM_VERSION=$(grep -A2 'eu.maveniverse.maven.mimir' pom.xml | grep '<version>' | sed 's/.*<version>\(.*\)<\/version>.*/\1/') - if [ "$POM_VERSION" != "${{ env.MIMIR_VERSION }}" ]; then - echo "::error::MIMIR_VERSION (${{ env.MIMIR_VERSION }}) does not match pom.xml ($POM_VERSION) — update MIMIR_VERSION in this workflow" - exit 1 - fi - - name: Prepare Mimir for Maven 3.x shell: bash run: | diff --git a/compat/maven-compat/src/main/java/org/apache/maven/repository/metadata/ArtifactMetadata.java b/compat/maven-compat/src/main/java/org/apache/maven/repository/metadata/ArtifactMetadata.java index c690cebf9a..a808e2be8c 100644 --- a/compat/maven-compat/src/main/java/org/apache/maven/repository/metadata/ArtifactMetadata.java +++ b/compat/maven-compat/src/main/java/org/apache/maven/repository/metadata/ArtifactMetadata.java @@ -301,7 +301,7 @@ public void setError(String error) { } public boolean isError() { - return error != null; + return error == null; } public String getDependencyConflictId() { diff --git a/compat/maven-compat/src/test/java/org/apache/maven/repository/metadata/ArtifactMetadataTest.java b/compat/maven-compat/src/test/java/org/apache/maven/repository/metadata/ArtifactMetadataTest.java deleted file mode 100644 index c319730133..0000000000 --- a/compat/maven-compat/src/test/java/org/apache/maven/repository/metadata/ArtifactMetadataTest.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * 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.repository.metadata; - -import org.apache.maven.artifact.ArtifactScopeEnum; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -/** - * Regression test for {@link ArtifactMetadata#isError()}. - * Verifies that isError() returns true when error is set, false when error is null. - */ -@Deprecated -class ArtifactMetadataTest { - - @Test - void isErrorReturnsFalseWhenErrorIsNull() { - ArtifactMetadata metadata = new ArtifactMetadata("g:a:1.0"); - assertFalse(metadata.isError()); - } - - @Test - void isErrorReturnsTrueWhenErrorIsSet() { - ArtifactMetadata metadata = new ArtifactMetadata("g:a:1.0"); - metadata.setError("Something went wrong"); - assertTrue(metadata.isError()); - } - - @Test - void isErrorReturnsFalseAfterErrorIsCleared() { - ArtifactMetadata metadata = new ArtifactMetadata("g:a:1.0"); - metadata.setError("Something went wrong"); - metadata.setError(null); - assertFalse(metadata.isError()); - } - - @Test - void isErrorReturnsTrueWhenConstructedWithError() { - ArtifactMetadata metadata = new ArtifactMetadata( - "g", "a", "1.0", "jar", ArtifactScopeEnum.DEFAULT_SCOPE, null, null, null, false, "Resolution failed"); - assertTrue(metadata.isError()); - } - - @Test - void isErrorReturnsFalseWhenConstructedWithoutError() { - ArtifactMetadata metadata = new ArtifactMetadata( - "g", "a", "1.0", "jar", ArtifactScopeEnum.DEFAULT_SCOPE, null, null, null, true, null); - assertFalse(metadata.isError()); - } -} diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java index d29e09a5fc..5ec158321d 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java @@ -311,6 +311,11 @@ protected BuildEventListener doDetermineBuildEventListener(C context) { protected final void createTerminal(C context) { if (context.terminal == null) { + // Create the build log appender; also sets MavenSimpleLogger sink + ProjectBuildLogAppender projectBuildLogAppender = + new ProjectBuildLogAppender(determineBuildEventListener(context)); + context.closeables.add(projectBuildLogAppender); + MessageUtils.systemInstall( builder -> doCreateTerminal(context, builder), terminal -> doConfigureWithTerminal(context, terminal)); @@ -318,11 +323,6 @@ protected final void createTerminal(C context) { context.terminal = MessageUtils.getTerminal(); context.closeables.add(MessageUtils::systemUninstall); MessageUtils.registerShutdownHook(); // safety belt - - // Create the build log appender; also sets MavenSimpleLogger sink - ProjectBuildLogAppender projectBuildLogAppender = - new ProjectBuildLogAppender(determineBuildEventListener(context)); - context.closeables.add(projectBuildLogAppender); } else { doConfigureWithTerminal(context, context.terminal); } diff --git a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java index eaaf888e48..f18b5161d7 100644 --- a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java +++ b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java @@ -25,9 +25,13 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Stream; +import eu.maveniverse.domtrip.Comment; import eu.maveniverse.domtrip.Document; +import eu.maveniverse.domtrip.Editor; import eu.maveniverse.domtrip.Element; import eu.maveniverse.domtrip.maven.Coordinates; import eu.maveniverse.domtrip.maven.MavenPomElements; @@ -54,6 +58,7 @@ import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PLUGIN_REPOSITORY; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILE; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROFILES; +import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PROPERTIES; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.RELATIVE_PATH; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.REPOSITORIES; import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.REPOSITORY; @@ -70,6 +75,8 @@ @Priority(20) public class CompatibilityFixStrategy extends AbstractUpgradeStrategy { + private static final Pattern EXPRESSION_PATTERN = Pattern.compile("\\$\\{([^}]+)}"); + @Override public boolean isApplicable(UpgradeContext context) { UpgradeOptions options = getOptions(context); @@ -117,6 +124,8 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap) Set<Path> modifiedPoms = new HashSet<>(); Set<Path> errorPoms = new HashSet<>(); + Set<String> allDefinedProperties = collectAllDefinedProperties(pomMap); + for (Map.Entry<Path, Document> entry : pomMap.entrySet()) { Path pomPath = entry.getKey(); Document pomDocument = entry.getValue(); @@ -128,13 +137,13 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap) try { boolean hasIssues = false; - // Apply all compatibility fixes hasIssues |= fixUnsupportedCombineChildrenAttributes(pomDocument, context); hasIssues |= fixUnsupportedCombineSelfAttributes(pomDocument, context); hasIssues |= fixDuplicateDependencies(pomDocument, context); hasIssues |= fixDuplicatePlugins(pomDocument, context); hasIssues |= fixUnsupportedRepositoryExpressions(pomDocument, context); hasIssues |= fixIncorrectParentRelativePaths(pomDocument, pomPath, pomMap, context); + hasIssues |= fixUndefinedPropertyExpressions(pomDocument, allDefinedProperties, context); if (hasIssues) { context.success("Maven 4 compatibility issues fixed"); @@ -345,6 +354,150 @@ private boolean fixIncorrectParentRelativePaths( return false; } + private Set<String> collectAllDefinedProperties(Map<Path, Document> pomMap) { + Set<String> properties = new HashSet<>(); + for (Map.Entry<Path, Document> entry : pomMap.entrySet()) { + collectPropertiesFromDom(entry.getValue(), properties); + } + return properties; + } + + private void collectPropertiesFromDom(Document document, Set<String> properties) { + Element root = document.root(); + + root.childElement(PROPERTIES) + .ifPresent(propsElement -> propsElement.childElements().forEach(child -> properties.add(child.name()))); + + root.childElement(PROFILES).ifPresent(profiles -> profiles.childElements(PROFILE) + .forEach(profile -> profile.childElement(PROPERTIES) + .ifPresent(propsElement -> + propsElement.childElements().forEach(child -> properties.add(child.name()))))); + } + + /** + * Fixes dependencies with undefined property expressions by commenting them out. + */ + private boolean fixUndefinedPropertyExpressions( + Document pomDocument, Set<String> allDefinedProperties, UpgradeContext context) { + Element root = pomDocument.root(); + + Stream<DependencyContainer> dependencyContainers = Stream.concat( + Stream.of( + new DependencyContainer( + root.childElement(DEPENDENCIES).orElse(null), DEPENDENCIES), + new DependencyContainer( + root.childElement(DEPENDENCY_MANAGEMENT) + .flatMap(dm -> dm.childElement(DEPENDENCIES)) + .orElse(null), + DEPENDENCY_MANAGEMENT)) + .filter(container -> container.element != null), + root.childElement(PROFILES).stream() + .flatMap(profiles -> profiles.childElements(PROFILE)) + .flatMap(profile -> Stream.of( + new DependencyContainer( + profile.childElement(DEPENDENCIES) + .orElse(null), + "profile dependencies"), + new DependencyContainer( + profile.childElement(DEPENDENCY_MANAGEMENT) + .flatMap(dm -> dm.childElement(DEPENDENCIES)) + .orElse(null), + "profile dependencyManagement")) + .filter(container -> container.element != null))); + + return dependencyContainers + .map(container -> fixUndefinedPropertyExpressionsInSection( + container.element, allDefinedProperties, pomDocument, context, container.sectionName)) + .reduce(false, Boolean::logicalOr); + } + + /** + * Fixes undefined property expressions in a specific dependencies section. + */ + private boolean fixUndefinedPropertyExpressionsInSection( + Element dependenciesElement, + Set<String> allDefinedProperties, + Document pomDocument, + UpgradeContext context, + String sectionName) { + boolean fixed = false; + List<Element> dependencies = + dependenciesElement.childElements(DEPENDENCY).toList(); + Editor editor = new Editor(pomDocument); + + for (Element dependency : dependencies) { + Set<String> undefinedProps = findUndefinedProperties(dependency, allDefinedProperties); + if (!undefinedProps.isEmpty()) { + String propLabel = undefinedProps.size() > 1 ? "properties" : "property"; + String propsStr = "'" + String.join("', '", undefinedProps) + "'"; + + Comment comment = editor.commentOutElement(dependency); + String elementXml = comment.content().trim(); + comment.content( + " mvnup: commented out - undefined " + propLabel + " " + propsStr + "\n" + elementXml + " "); + + context.detail("Fixed: Commented out dependency with undefined " + propLabel + " " + propsStr + " in " + + sectionName); + fixed = true; + } + } + + return fixed; + } + + /** + * Finds undefined property expressions in a dependency's coordinate fields. + */ + private Set<String> findUndefinedProperties(Element dependency, Set<String> allDefinedProperties) { + Set<String> undefinedProperties = new HashSet<>(); + + String groupId = dependency.childText(MavenPomElements.Elements.GROUP_ID); + String artifactId = dependency.childText(MavenPomElements.Elements.ARTIFACT_ID); + String version = dependency.childText(MavenPomElements.Elements.VERSION); + + collectUndefinedExpressions(groupId, allDefinedProperties, undefinedProperties); + collectUndefinedExpressions(artifactId, allDefinedProperties, undefinedProperties); + collectUndefinedExpressions(version, allDefinedProperties, undefinedProperties); + + return undefinedProperties; + } + + private void collectUndefinedExpressions(String value, Set<String> allDefinedProperties, Set<String> result) { + if (value == null) { + return; + } + Matcher matcher = EXPRESSION_PATTERN.matcher(value); + while (matcher.find()) { + String propertyName = matcher.group(1); + if (!isWellKnownProperty(propertyName) && !allDefinedProperties.contains(propertyName)) { + result.add(propertyName); + } + } + } + + private static boolean isWellKnownProperty(String propertyName) { + if (propertyName.startsWith("project.") + || propertyName.startsWith("pom.") + || propertyName.startsWith("env.") + || propertyName.startsWith("settings.") + || propertyName.startsWith("maven.")) { + return true; + } + if (propertyName.startsWith("java.") + || propertyName.startsWith("os.") + || propertyName.startsWith("user.") + || propertyName.startsWith("file.") + || propertyName.startsWith("line.") + || propertyName.startsWith("path.") + || propertyName.startsWith("sun.")) { + return true; + } + return "basedir".equals(propertyName) + || "revision".equals(propertyName) + || "sha1".equals(propertyName) + || "changelist".equals(propertyName); + } + /** * Recursively finds all elements with a specific attribute value. */ diff --git a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java index 6144f4ca32..00dacbb1c1 100644 --- a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java +++ b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java @@ -18,6 +18,7 @@ */ package org.apache.maven.cling.invoker.mvnup.goals; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Map; @@ -32,6 +33,7 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -58,6 +60,10 @@ private UpgradeContext createMockContext() { return TestUtils.createMockContext(); } + private UpgradeContext createMockContext(Path workingDirectory) { + return TestUtils.createMockContext(workingDirectory); + } + private UpgradeContext createMockContext(UpgradeOptions options) { return TestUtils.createMockContext(options); } @@ -478,6 +484,443 @@ void shouldNotModifyUrlsWithoutDeprecatedExpressions() throws Exception { } } + @Nested + @DisplayName("Undefined Property Expression Fixes") + class UndefinedPropertyExpressionFixesTests { + + @Test + @DisplayName("should comment out dependency with undefined property expression") + void shouldCommentOutDependencyWithUndefinedProperty() throws Exception { + String pomXml = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0";> + <modelVersion>4.0.0</modelVersion> + <groupId>test</groupId> + <artifactId>test</artifactId> + <version>1.0.0</version> + <dependencyManagement> + <dependencies> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <version>${guava-version}</version> + </dependency> + </dependencies> + </dependencyManagement> + </project> + """; + + Document document = Document.of(pomXml); + Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + UpgradeResult result = strategy.doApply(context, pomMap); + + assertTrue(result.success(), "Compatibility fix should succeed"); + assertTrue(result.modifiedCount() > 0, "Should have commented out dependency"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker"); + assertTrue(xml.contains("guava-version"), "Should mention the undefined property"); + + Element root = document.root(); + Element depMgmt = DomUtils.findChildElement(root, "dependencyManagement"); + Element deps = DomUtils.findChildElement(depMgmt, "dependencies"); + assertEquals(0, deps.childElements("dependency").count(), "Should have no dependency elements"); + } + + @Test + @DisplayName("should not comment out dependency with defined property") + void shouldNotCommentOutDependencyWithDefinedProperty() throws Exception { + String pomXml = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0";> + <modelVersion>4.0.0</modelVersion> + <groupId>test</groupId> + <artifactId>test</artifactId> + <version>1.0.0</version> + <properties> + <guava-version>30.0-jre</guava-version> + </properties> + <dependencyManagement> + <dependencies> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <version>${guava-version}</version> + </dependency> + </dependencies> + </dependencyManagement> + </project> + """; + + Document document = Document.of(pomXml); + Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + strategy.doApply(context, pomMap); + + Element root = document.root(); + Element depMgmt = DomUtils.findChildElement(root, "dependencyManagement"); + Element deps = DomUtils.findChildElement(depMgmt, "dependencies"); + assertEquals(1, deps.childElements("dependency").count(), "Dependency should still be present"); + } + + @Test + @DisplayName("should not comment out dependency with well-known built-in property") + void shouldNotCommentOutDependencyWithBuiltinProperty() throws Exception { + String pomXml = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0";> + <modelVersion>4.0.0</modelVersion> + <groupId>test</groupId> + <artifactId>test</artifactId> + <version>1.0.0</version> + <dependencies> + <dependency> + <groupId>test</groupId> + <artifactId>test-dep</artifactId> + <version>${project.version}</version> + </dependency> + </dependencies> + </project> + """; + + Document document = Document.of(pomXml); + Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document); + + UpgradeContext context = createMockContext(); + strategy.doApply(context, pomMap); + + Element root = document.root(); + Element deps = DomUtils.findChildElement(root, "dependencies"); + assertEquals( + 1, + deps.childElements("dependency").count(), + "Dependency with built-in property should still be present"); + } + + @Test + @DisplayName("should recognize property defined in another module POM") + void shouldRecognizePropertyFromOtherPom() throws Exception { + String parentPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0";> + <modelVersion>4.0.0</modelVersion> + <groupId>test</groupId> + <artifactId>parent</artifactId> + <version>1.0.0</version> + <properties> + <guava-version>30.0-jre</guava-version> + </properties> + </project> + """; + + String childPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>test</groupId> + <artifactId>parent</artifactId> + <version>1.0.0</version> + </parent> + <artifactId>child</artifactId> + <dependencies> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <version>${guava-version}</version> + </dependency> + </dependencies> + </project> + """; + + Document parentDoc = Document.of(parentPom); + Document childDoc = Document.of(childPom); + Map<Path, Document> pomMap = Map.of( + Paths.get("pom.xml"), parentDoc, + Paths.get("child/pom.xml"), childDoc); + + UpgradeContext context = createMockContext(); + strategy.doApply(context, pomMap); + + Element root = childDoc.root(); + Element deps = DomUtils.findChildElement(root, "dependencies"); + assertEquals( + 1, + deps.childElements("dependency").count(), + "Dependency should not be commented out when property is defined in another POM"); + } + } + + @Nested + @DisplayName("Undefined Property Expression Fixes with Effective Model") + class UndefinedPropertyEffectiveModelTests { + + @TempDir + Path tempDir; + + @Test + @DisplayName("should recognize property inherited from external parent via relativePath") + void shouldRecognizePropertyFromExternalParent() throws Exception { + String parentPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <groupId>com.example</groupId> + <artifactId>external-parent</artifactId> + <version>1.0.0</version> + <packaging>pom</packaging> + <properties> + <guava.version>32.1.3-jre</guava.version> + </properties> + </project> + """; + + String childPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>com.example</groupId> + <artifactId>external-parent</artifactId> + <version>1.0.0</version> + <relativePath>../pom.xml</relativePath> + </parent> + <artifactId>child</artifactId> + <dependencies> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <version>${guava.version}</version> + </dependency> + </dependencies> + </project> + """; + + Path parentPomPath = tempDir.resolve("pom.xml"); + Path childDir = Files.createDirectories(tempDir.resolve("child")); + Path childPomPath = childDir.resolve("pom.xml"); + + Files.writeString(parentPomPath, parentPom); + Files.writeString(childPomPath, childPom); + + Document parentDoc = Document.of(parentPom); + Document childDoc = Document.of(childPom); + Map<Path, Document> pomMap = Map.of( + parentPomPath, parentDoc, + childPomPath, childDoc); + + UpgradeContext context = createMockContext(tempDir); + strategy.doApply(context, pomMap); + + Element root = childDoc.root(); + Element deps = DomUtils.findChildElement(root, "dependencies"); + assertEquals( + 1, + deps.childElements("dependency").count(), + "Dependency should not be commented out when property is inherited from external parent"); + } + + @Test + @DisplayName("should comment out dependency when property is not in parent either") + void shouldCommentOutWhenPropertyNotInParent() throws Exception { + String parentPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <groupId>com.example</groupId> + <artifactId>external-parent</artifactId> + <version>1.0.0</version> + <packaging>pom</packaging> + </project> + """; + + String childPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>com.example</groupId> + <artifactId>external-parent</artifactId> + <version>1.0.0</version> + <relativePath>../pom.xml</relativePath> + </parent> + <artifactId>child</artifactId> + <dependencies> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <version>${undefined.prop}</version> + </dependency> + </dependencies> + </project> + """; + + Path parentPomPath = tempDir.resolve("pom.xml"); + Path childDir = Files.createDirectories(tempDir.resolve("child")); + Path childPomPath = childDir.resolve("pom.xml"); + + Files.writeString(parentPomPath, parentPom); + Files.writeString(childPomPath, childPom); + + Document childDoc = Document.of(childPom); + Map<Path, Document> pomMap = Map.of(childPomPath, childDoc); + + UpgradeContext context = createMockContext(tempDir); + strategy.doApply(context, pomMap); + + String xml = DomUtils.toXml(childDoc); + assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker"); + } + + @Test + @DisplayName("should handle partial undefined - only comment out dependency with undefined property") + void shouldHandlePartialUndefined() throws Exception { + String pomXml = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <groupId>com.example</groupId> + <artifactId>test</artifactId> + <version>1.0.0</version> + <properties> + <commons.version>3.14.0</commons.version> + </properties> + <dependencies> + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-lang3</artifactId> + <version>${commons.version}</version> + </dependency> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <version>${undefined.version}</version> + </dependency> + </dependencies> + </project> + """; + + Path pomPath = tempDir.resolve("pom.xml"); + Files.writeString(pomPath, pomXml); + + Document document = Document.of(pomXml); + Map<Path, Document> pomMap = Map.of(pomPath, document); + + UpgradeContext context = createMockContext(tempDir); + strategy.doApply(context, pomMap); + + Element root = document.root(); + Element deps = DomUtils.findChildElement(root, "dependencies"); + assertEquals(1, deps.childElements("dependency").count(), "Only defined-property dependency should remain"); + + String xml = DomUtils.toXml(document); + assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker"); + assertTrue(xml.contains("'undefined.version'"), "Should mention the undefined property in comment"); + assertFalse(xml.contains("'commons.version'"), "Should not flag the defined property as undefined"); + } + + @Test + @DisplayName("should recognize property from grandparent POM") + void shouldRecognizePropertyFromGrandparent() throws Exception { + String grandparentPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <groupId>com.example</groupId> + <artifactId>grandparent</artifactId> + <version>1.0.0</version> + <packaging>pom</packaging> + <properties> + <guava.version>32.1.3-jre</guava.version> + </properties> + </project> + """; + + String parentPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>com.example</groupId> + <artifactId>grandparent</artifactId> + <version>1.0.0</version> + <relativePath>../pom.xml</relativePath> + </parent> + <artifactId>parent</artifactId> + <packaging>pom</packaging> + </project> + """; + + String childPom = """ + <?xml version="1.0" encoding="UTF-8"?> + <project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>com.example</groupId> + <artifactId>parent</artifactId> + <version>1.0.0</version> + <relativePath>../parent/pom.xml</relativePath> + </parent> + <artifactId>child</artifactId> + <dependencies> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + <version>${guava.version}</version> + </dependency> + </dependencies> + </project> + """; + + Path grandparentPath = tempDir.resolve("pom.xml"); + Path parentDir = Files.createDirectories(tempDir.resolve("parent")); + Path parentPath = parentDir.resolve("pom.xml"); + Path childDir = Files.createDirectories(tempDir.resolve("child")); + Path childPomPath = childDir.resolve("pom.xml"); + + Files.writeString(grandparentPath, grandparentPom); + Files.writeString(parentPath, parentPom); + Files.writeString(childPomPath, childPom); + + Document grandparentDoc = Document.of(grandparentPom); + Document parentDoc = Document.of(parentPom); + Document childDoc = Document.of(childPom); + Map<Path, Document> pomMap = Map.of( + grandparentPath, grandparentDoc, + parentPath, parentDoc, + childPomPath, childDoc); + + UpgradeContext context = createMockContext(tempDir); + strategy.doApply(context, pomMap); + + Element root = childDoc.root(); + Element deps = DomUtils.findChildElement(root, "dependencies"); + assertEquals( + 1, + deps.childElements("dependency").count(), + "Dependency should not be commented out when property is inherited from grandparent"); + } + } + @Nested @DisplayName("Strategy Description") class StrategyDescriptionTests { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java index 47cd0efefe..44b3353336 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java @@ -1705,7 +1705,7 @@ private boolean validateProfileId( String id, @Nullable SourceHint sourceHint, InputLocationTracker tracker) { - if (id != null && validProfileIds.contains(id)) { + if (validProfileIds.contains(id)) { return true; } if (!validateStringNotEmpty(prefix, fieldName, problems, severity, version, id, sourceHint, tracker)) { diff --git a/its/pom.xml b/its/pom.xml index 1782b0a50a..afa5799493 100644 --- a/its/pom.xml +++ b/its/pom.xml @@ -83,7 +83,7 @@ under the License. <maven.compiler.release>17</maven.compiler.release> <wagonVersion>3.5.3</wagonVersion> - <asmVersion>9.10.1</asmVersion> + <asmVersion>9.10</asmVersion> <plexusUtilsVersion>4.0.2</plexusUtilsVersion> <plexusXmlVersion>4.1.1</plexusXmlVersion> diff --git a/pom.xml b/pom.xml index 658ac2b105..dd41346d22 100644 --- a/pom.xml +++ b/pom.xml @@ -143,7 +143,7 @@ under the License. <project.build.outputTimestamp>2025-11-07T22:54:23Z</project.build.outputTimestamp> <!-- various versions --> <assertjVersion>3.27.7</assertjVersion> - <asmVersion>9.10.1</asmVersion> + <asmVersion>9.10</asmVersion> <byteBuddyVersion>1.18.8</byteBuddyVersion> <classWorldsVersion>2.12.0</classWorldsVersion> <commonsCliVersion>1.11.0</commonsCliVersion> @@ -154,7 +154,7 @@ under the License. <hamcrestVersion>3.0</hamcrestVersion> <jakartaInjectApiVersion>2.0.1</jakartaInjectApiVersion> <javaxAnnotationApiVersion>1.3.2</javaxAnnotationApiVersion> - <jlineVersion>4.1.2</jlineVersion> + <jlineVersion>4.1.0</jlineVersion> <jmhVersion>1.37</jmhVersion> <junitVersion>5.13.4</junitVersion> <jxpathVersion>1.4.0</jxpathVersion> @@ -697,11 +697,10 @@ under the License. <artifactId>jmh-generator-annprocess</artifactId> <version>${jmhVersion}</version> </dependency> - <!-- Keep version in sync with MIMIR_VERSION in .github/workflows/maven.yml --> <dependency> <groupId>eu.maveniverse.maven.mimir</groupId> <artifactId>testing</artifactId> - <version>0.11.4</version> + <version>0.11.2</version> </dependency> </dependencies> <!--bootstrap-start-comment--> @@ -751,7 +750,7 @@ under the License. <plugin> <groupId>com.github.siom79.japicmp</groupId> <artifactId>japicmp-maven-plugin</artifactId> - <version>0.26.0</version> + <version>0.25.7</version> <executions> <execution> <goals>
