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/06/04 09:39:11 UTC
(maven) branch maven-4.0.x updated: Filter project repos with uninterpolated property expressions (#12204) (#12229)
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
The following commit(s) were added to refs/heads/maven-4.0.x by this push:
new 3c62e2e759 Filter project repos with uninterpolated property expressions (#12204) (#12229)
3c62e2e759 is described below
commit 3c62e2e759ef9caa8d08775086439e8e5c245eba
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Thu Jun 4 11:39:05 2026 +0200
Filter project repos with uninterpolated property expressions (#12204) (#12229)
* mvnup: comment out repositories with undefined property expressions
Projects like uima-uimaj, opennlp-sandbox, and seatunnel-shade define
repositories with ${eclipseP2RepoId} in the repo ID. This property is
never defined — it was used as a literal string in Maven 3. Maven 4
rejects it with IllegalArgumentException at runtime.
Extend the mvnup compatibility fix strategy to detect and comment out
repositories and plugin repositories whose id or url contain undefined
property expressions, following the same pattern used for dependencies
(#12080). Handles both root-level and profile-level repositories.
* Filter project repositories with uninterpolated property expressions
When a project POM defines repositories with ${...} expressions that
cannot be resolved (e.g. ${eclipseP2RepoId}), Maven 4 previously
failed with InternalErrorException. This change downgrades the model
validation from ERROR to WARNING and filters such repositories before
they reach the resolver, logging a warning instead of failing the build.
---------
Co-authored-by: Claude Opus 4.6 <no...@anthropic.com>
---
.../mvnup/goals/CompatibilityFixStrategy.java | 84 ++++++++++
.../mvnup/goals/CompatibilityFixStrategyTest.java | 175 +++++++++++++++++++++
.../project/DefaultProjectBuildingHelper.java | 11 ++
.../maven/impl/model/DefaultModelValidator.java | 8 +-
.../impl/model/DefaultModelValidatorTest.java | 26 +--
.../it/MavenITgh11140RepoDmUnresolvedTest.java | 13 +-
.../it/MavenITgh11140RepoInterpolationTest.java | 11 +-
7 files changed, 297 insertions(+), 31 deletions(-)
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 343caf147b..6b9b0c8e39 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
@@ -149,6 +149,7 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
hasIssues |= fixUnsupportedRepositoryExpressions(pomDocument, context);
hasIssues |= fixIncorrectParentRelativePaths(pomDocument, pomPath, pomMap, context);
hasIssues |= fixUndefinedPropertyExpressions(pomDocument, allDefinedProperties, context);
+ hasIssues |= fixUndefinedPropertyExpressionsInRepositories(pomDocument, allDefinedProperties, context);
if (hasIssues) {
context.success("Maven 4 compatibility issues fixed");
@@ -429,6 +430,89 @@ private boolean fixUndefinedPropertyExpressions(
.reduce(false, Boolean::logicalOr);
}
+ /**
+ * Fixes repositories with undefined property expressions by commenting them out.
+ */
+ private boolean fixUndefinedPropertyExpressionsInRepositories(
+ Document pomDocument, Set<String> allDefinedProperties, UpgradeContext context) {
+ Element root = pomDocument.root();
+
+ Stream<RepositoryContainer> repositoryContainers = Stream.concat(
+ Stream.of(
+ new RepositoryContainer(
+ root.childElement(REPOSITORIES).orElse(null), REPOSITORY, REPOSITORIES),
+ new RepositoryContainer(
+ root.childElement(PLUGIN_REPOSITORIES).orElse(null),
+ PLUGIN_REPOSITORY,
+ PLUGIN_REPOSITORIES))
+ .filter(c -> c.element != null),
+ root.childElement(PROFILES).stream()
+ .flatMap(profiles -> profiles.childElements(PROFILE))
+ .flatMap(profile -> Stream.of(
+ new RepositoryContainer(
+ profile.childElement(REPOSITORIES)
+ .orElse(null),
+ REPOSITORY,
+ "profile repositories"),
+ new RepositoryContainer(
+ profile.childElement(PLUGIN_REPOSITORIES)
+ .orElse(null),
+ PLUGIN_REPOSITORY,
+ "profile pluginRepositories"))
+ .filter(c -> c.element != null)));
+
+ return repositoryContainers
+ .map(c -> fixUndefinedPropertyExpressionsInRepositorySection(
+ c.element, c.elementType, allDefinedProperties, pomDocument, context, c.sectionName))
+ .reduce(false, Boolean::logicalOr);
+ }
+
+ private record RepositoryContainer(Element element, String elementType, String sectionName) {}
+
+ private boolean fixUndefinedPropertyExpressionsInRepositorySection(
+ Element repositoriesElement,
+ String elementType,
+ Set<String> allDefinedProperties,
+ Document pomDocument,
+ UpgradeContext context,
+ String sectionName) {
+ boolean fixed = false;
+ List<Element> repositories =
+ repositoriesElement.childElements(elementType).toList();
+ Editor editor = new Editor(pomDocument);
+
+ for (Element repository : repositories) {
+ Set<String> undefinedProps = findUndefinedPropertiesInRepository(repository, allDefinedProperties);
+ if (!undefinedProps.isEmpty()) {
+ String propLabel = undefinedProps.size() > 1 ? "properties" : "property";
+ String propsStr = "'" + String.join("', '", undefinedProps) + "'";
+
+ Comment comment = editor.commentOutElement(repository);
+ String elementXml = comment.content().trim();
+ comment.content(
+ " mvnup: commented out - undefined " + propLabel + " " + propsStr + "\n" + elementXml + " ");
+
+ context.detail("Fixed: Commented out " + elementType + " with undefined " + propLabel + " " + propsStr
+ + " in " + sectionName);
+ fixed = true;
+ }
+ }
+
+ return fixed;
+ }
+
+ private Set<String> findUndefinedPropertiesInRepository(Element repository, Set<String> allDefinedProperties) {
+ Set<String> undefinedProperties = new HashSet<>();
+
+ String id = repository.childText("id");
+ String url = repository.childText("url");
+
+ collectUndefinedExpressions(id, allDefinedProperties, undefinedProperties);
+ collectUndefinedExpressions(url, allDefinedProperties, undefinedProperties);
+
+ return undefinedProperties;
+ }
+
/**
* Fixes undefined property expressions in a specific dependencies section.
*/
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 051a33fd34..3b8e7fc9a0 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
@@ -1457,6 +1457,181 @@ void shouldCommentOutWhenPropertyTrulyUndefinedInEffectiveModel() throws Excepti
}
}
+ @Nested
+ @DisplayName("Undefined Property Expression in Repositories Fixes")
+ class UndefinedPropertyExpressionInRepositoriesTests {
+
+ @Test
+ @DisplayName("should comment out repository with undefined property in id")
+ void shouldCommentOutRepositoryWithUndefinedPropertyInId() 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>
+ <repositories>
+ <repository>
+ <id>${eclipseP2RepoId}</id>
+ <url>https://repo.example.com</url>
+ </repository>
+ </repositories>
+ </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 repository");
+
+ String xml = DomUtils.toXml(document);
+ assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
+ assertTrue(xml.contains("eclipseP2RepoId"), "Should mention the undefined property");
+
+ Element root = document.root();
+ Element repos = DomUtils.findChildElement(root, "repositories");
+ assertEquals(0, repos.childElements("repository").count(), "Should have no repository elements");
+ }
+
+ @Test
+ @DisplayName("should comment out repository with undefined property in url")
+ void shouldCommentOutRepositoryWithUndefinedPropertyInUrl() 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>
+ <repositories>
+ <repository>
+ <id>my-repo</id>
+ <url>${undefinedBaseUrl}/releases</url>
+ </repository>
+ </repositories>
+ </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 repository");
+
+ String xml = DomUtils.toXml(document);
+ assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
+ assertTrue(xml.contains("undefinedBaseUrl"), "Should mention the undefined property");
+ }
+
+ @Test
+ @DisplayName("should not comment out repository with defined property")
+ void shouldNotCommentOutRepositoryWithDefinedProperty() 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>
+ <repoId>my-custom-repo</repoId>
+ </properties>
+ <repositories>
+ <repository>
+ <id>${repoId}</id>
+ <url>https://repo.example.com</url>
+ </repository>
+ </repositories>
+ </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 repos = DomUtils.findChildElement(root, "repositories");
+ assertEquals(1, repos.childElements("repository").count(), "Repository should still be present");
+ }
+
+ @Test
+ @DisplayName("should comment out plugin repository with undefined property")
+ void shouldCommentOutPluginRepositoryWithUndefinedProperty() 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>
+ <pluginRepositories>
+ <pluginRepository>
+ <id>${eclipseP2RepoId}</id>
+ <url>https://plugins.example.com</url>
+ </pluginRepository>
+ </pluginRepositories>
+ </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 plugin repository");
+
+ String xml = DomUtils.toXml(document);
+ assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
+
+ Element root = document.root();
+ Element repos = DomUtils.findChildElement(root, "pluginRepositories");
+ assertEquals(
+ 0, repos.childElements("pluginRepository").count(), "Should have no pluginRepository elements");
+ }
+
+ @Test
+ @DisplayName("should not comment out repository with well-known property")
+ void shouldNotCommentOutRepositoryWithWellKnownProperty() 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>
+ <repositories>
+ <repository>
+ <id>local-repo</id>
+ <url>file://${project.basedir}/repo</url>
+ </repository>
+ </repositories>
+ </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 repos = DomUtils.findChildElement(root, "repositories");
+ assertEquals(1, repos.childElements("repository").count(), "Repository should still be present");
+ }
+ }
+
@Nested
@DisplayName("Strategy Description")
class StrategyDescriptionTests {
diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java
index f8f77c7cea..6bde396a97 100644
--- a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java
+++ b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java
@@ -92,6 +92,13 @@ public List<ArtifactRepository> createArtifactRepositories(
List<ArtifactRepository> internalRepositories = new ArrayList<>();
for (Repository repository : pomRepositories) {
+ if (containsExpression(repository.getId()) || containsExpression(repository.getUrl())) {
+ logger.warn(
+ "Skipping repository '{}' (url: '{}') containing an uninterpolated property expression",
+ repository.getId(),
+ repository.getUrl());
+ continue;
+ }
internalRepositories.add(MavenRepositorySystem.buildArtifactRepository(repository));
}
@@ -267,6 +274,10 @@ public void selectProjectRealm(MavenProject project) {
Thread.currentThread().setContextClassLoader(projectRealm);
}
+ private static boolean containsExpression(String value) {
+ return value != null && value.contains("${");
+ }
+
private List<org.eclipse.aether.artifact.Artifact> toAetherArtifacts(final List<Artifact> pluginArtifacts) {
return new ArrayList<>(RepositoryUtils.toArtifacts(pluginArtifacts));
}
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 44b3353336..f17504b7d0 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
@@ -1535,11 +1535,11 @@ private void validateRawRepository(
if (matcher.find()) {
addViolation(
problems,
- Severity.ERROR,
+ Severity.WARNING,
Version.V40,
prefix + prefix2 + "[" + repository.getId() + "].id",
null,
- "contains an uninterpolated expression.",
+ "contains an uninterpolated expression; the repository will be skipped.",
repository);
}
}
@@ -1561,11 +1561,11 @@ && validateStringNotEmpty(
if (matcher.find()) {
addViolation(
problems,
- Severity.ERROR,
+ Severity.WARNING,
Version.V40,
prefix + prefix2 + "[" + repository.getId() + "].url",
null,
- "contains an uninterpolated expression.",
+ "contains an uninterpolated expression; the repository will be skipped.",
repository);
}
}
diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java
index 36ba2745a0..c65391872c 100644
--- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java
+++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java
@@ -907,31 +907,33 @@ void repositoryWithBasedirExpression() throws Exception {
SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml");
// This test runs on raw model without interpolation, so all expressions appear uninterpolated
// In the real flow, supported expressions would be interpolated before validation
- assertViolations(result, 0, 3, 0);
+ assertViolations(result, 0, 0, 3);
}
@Test
void repositoryWithUnsupportedExpression() throws Exception {
SimpleProblemCollector result = validateRaw("raw-model/repository-with-unsupported-expression.xml");
- // Unsupported expressions should cause validation errors
- assertViolations(result, 0, 1, 0);
+ // Unsupported expressions should cause validation warnings (repos will be skipped at build time)
+ assertViolations(result, 0, 0, 1);
}
@Test
void repositoryWithUninterpolatedId() throws Exception {
SimpleProblemCollector result = validateRaw("raw-model/repository-with-uninterpolated-id.xml");
- // Uninterpolated expressions in repository IDs should cause validation errors
+ // Uninterpolated expressions in repository IDs should cause validation warnings
+ // (repos will be skipped at build time)
// distributionManagement repositories skip expression check since parent properties
// may not be available at file model validation stage
- assertViolations(result, 0, 2, 0);
+ assertViolations(result, 0, 0, 2);
- // Check that repository ID validation errors are present for repositories and pluginRepositories
- assertTrue(result.getErrors().stream()
- .anyMatch(error -> error.contains("repositories.repository.[${repository.id}].id")
- && error.contains("contains an uninterpolated expression")));
- assertTrue(result.getErrors().stream()
- .anyMatch(error -> error.contains("pluginRepositories.pluginRepository.[${plugin.repository.id}].id")
- && error.contains("contains an uninterpolated expression")));
+ // Check that repository ID validation warnings are present for repositories and pluginRepositories
+ assertTrue(result.getWarnings().stream()
+ .anyMatch(warning -> warning.contains("repositories.repository.[${repository.id}].id")
+ && warning.contains("contains an uninterpolated expression")));
+ assertTrue(result.getWarnings().stream()
+ .anyMatch(
+ warning -> warning.contains("pluginRepositories.pluginRepository.[${plugin.repository.id}].id")
+ && warning.contains("contains an uninterpolated expression")));
}
@Test
diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoDmUnresolvedTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoDmUnresolvedTest.java
index b3a2929239..6716658281 100644
--- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoDmUnresolvedTest.java
+++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoDmUnresolvedTest.java
@@ -32,17 +32,14 @@ class MavenITgh11140RepoDmUnresolvedTest extends AbstractMavenIntegrationTestCas
}
@Test
- void testFailsOnUnresolvedPlaceholders() throws Exception {
+ void testWarnsOnUnresolvedPlaceholders() throws Exception {
File testDir = extractResources("/gh-11140-repo-dm-unresolved");
Verifier verifier = newVerifier(testDir.getAbsolutePath());
- try {
- verifier.addCliArgument("validate");
- verifier.execute();
- } catch (VerificationException expected) {
- // Expected to fail due to unresolved placeholders during model validation
- }
- // We expect error mentioning uninterpolated expression
+ verifier.addCliArgument("validate");
+ verifier.execute();
+ // Build should succeed, but warn about uninterpolated expressions (repos are skipped)
+ verifier.verifyErrorFreeLog();
verifier.verifyTextInLog("contains an uninterpolated expression");
}
}
diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoInterpolationTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoInterpolationTest.java
index d354b33f2e..30b34510a3 100644
--- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoInterpolationTest.java
+++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoInterpolationTest.java
@@ -73,18 +73,15 @@ private static String getBaseUri(Path base) {
}
@Test
- void testUnresolvedPlaceholderFailsResolution() throws Exception {
+ void testUnresolvedPlaceholderWarnsAndSkipsRepository() throws Exception {
File testDir = extractResources("/gh-11140-repo-interpolation");
Verifier verifier = newVerifier(testDir.getAbsolutePath());
// Do NOT set env vars, so placeholders stay
verifier.addCliArgument("validate");
- try {
- verifier.execute();
- } catch (VerificationException expected) {
- // Expected to fail due to unresolved placeholders during model validation
- }
- // We expect error mentioning uninterpolated expression
+ verifier.execute();
+ // Build should succeed, but warn about uninterpolated expressions
+ verifier.verifyErrorFreeLog();
verifier.verifyTextInLog("contains an uninterpolated expression");
}
}