Skip to content

Commit df007ff

Browse files
gnodetclaude
gnodet
and
claude
authored
Filter project repos with uninterpolated property expressions (#12204)
* 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. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a52f476 commit df007ff

7 files changed

Lines changed: 297 additions & 31 deletions

File tree

‎impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ public UpgradeResult doApply(UpgradeContext context, Map<Path, Document> pomMap)
144144
hasIssues |= fixUnsupportedRepositoryExpressions(pomDocument, context);
145145
hasIssues |= fixIncorrectParentRelativePaths(pomDocument, pomPath, pomMap, context);
146146
hasIssues |= fixUndefinedPropertyExpressions(pomDocument, allDefinedProperties, context);
147+
hasIssues |= fixUndefinedPropertyExpressionsInRepositories(pomDocument, allDefinedProperties, context);
147148

148149
if (hasIssues) {
149150
context.success("Maven 4 compatibility issues fixed");
@@ -412,6 +413,89 @@ private boolean fixUndefinedPropertyExpressions(
412413
.reduce(false, Boolean::logicalOr);
413414
}
414415

416+
/**
417+
* Fixes repositories with undefined property expressions by commenting them out.
418+
*/
419+
private boolean fixUndefinedPropertyExpressionsInRepositories(
420+
Document pomDocument, Set<String> allDefinedProperties, UpgradeContext context) {
421+
Element root = pomDocument.root();
422+
423+
Stream<RepositoryContainer> repositoryContainers = Stream.concat(
424+
Stream.of(
425+
new RepositoryContainer(
426+
root.childElement(REPOSITORIES).orElse(null), REPOSITORY, REPOSITORIES),
427+
new RepositoryContainer(
428+
root.childElement(PLUGIN_REPOSITORIES).orElse(null),
429+
PLUGIN_REPOSITORY,
430+
PLUGIN_REPOSITORIES))
431+
.filter(c -> c.element != null),
432+
root.childElement(PROFILES).stream()
433+
.flatMap(profiles -> profiles.childElements(PROFILE))
434+
.flatMap(profile -> Stream.of(
435+
new RepositoryContainer(
436+
profile.childElement(REPOSITORIES)
437+
.orElse(null),
438+
REPOSITORY,
439+
"profile repositories"),
440+
new RepositoryContainer(
441+
profile.childElement(PLUGIN_REPOSITORIES)
442+
.orElse(null),
443+
PLUGIN_REPOSITORY,
444+
"profile pluginRepositories"))
445+
.filter(c -> c.element != null)));
446+
447+
return repositoryContainers
448+
.map(c -> fixUndefinedPropertyExpressionsInRepositorySection(
449+
c.element, c.elementType, allDefinedProperties, pomDocument, context, c.sectionName))
450+
.reduce(false, Boolean::logicalOr);
451+
}
452+
453+
private record RepositoryContainer(Element element, String elementType, String sectionName) {}
454+
455+
private boolean fixUndefinedPropertyExpressionsInRepositorySection(
456+
Element repositoriesElement,
457+
String elementType,
458+
Set<String> allDefinedProperties,
459+
Document pomDocument,
460+
UpgradeContext context,
461+
String sectionName) {
462+
boolean fixed = false;
463+
List<Element> repositories =
464+
repositoriesElement.childElements(elementType).toList();
465+
Editor editor = new Editor(pomDocument);
466+
467+
for (Element repository : repositories) {
468+
Set<String> undefinedProps = findUndefinedPropertiesInRepository(repository, allDefinedProperties);
469+
if (!undefinedProps.isEmpty()) {
470+
String propLabel = undefinedProps.size() > 1 ? "properties" : "property";
471+
String propsStr = "'" + String.join("', '", undefinedProps) + "'";
472+
473+
Comment comment = editor.commentOutElement(repository);
474+
String elementXml = comment.content().trim();
475+
comment.content(
476+
" mvnup: commented out - undefined " + propLabel + " " + propsStr + "\n" + elementXml + " ");
477+
478+
context.detail("Fixed: Commented out " + elementType + " with undefined " + propLabel + " " + propsStr
479+
+ " in " + sectionName);
480+
fixed = true;
481+
}
482+
}
483+
484+
return fixed;
485+
}
486+
487+
private Set<String> findUndefinedPropertiesInRepository(Element repository, Set<String> allDefinedProperties) {
488+
Set<String> undefinedProperties = new HashSet<>();
489+
490+
String id = repository.childText("id");
491+
String url = repository.childText("url");
492+
493+
collectUndefinedExpressions(id, allDefinedProperties, undefinedProperties);
494+
collectUndefinedExpressions(url, allDefinedProperties, undefinedProperties);
495+
496+
return undefinedProperties;
497+
}
498+
415499
/**
416500
* Fixes undefined property expressions in a specific dependencies section.
417501
*/

‎impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,181 @@ void shouldRecognizePropertyFromGrandparent() throws Exception {
921921
}
922922
}
923923

924+
@Nested
925+
@DisplayName("Undefined Property Expression in Repositories Fixes")
926+
class UndefinedPropertyExpressionInRepositoriesTests {
927+
928+
@Test
929+
@DisplayName("should comment out repository with undefined property in id")
930+
void shouldCommentOutRepositoryWithUndefinedPropertyInId() throws Exception {
931+
String pomXml = """
932+
<?xml version="1.0" encoding="UTF-8"?>
933+
<project xmlns="http://maven.apache.org/POM/4.0.0">
934+
<modelVersion>4.0.0</modelVersion>
935+
<groupId>test</groupId>
936+
<artifactId>test</artifactId>
937+
<version>1.0.0</version>
938+
<repositories>
939+
<repository>
940+
<id>${eclipseP2RepoId}</id>
941+
<url>https://repo.example.com</url>
942+
</repository>
943+
</repositories>
944+
</project>
945+
""";
946+
947+
Document document = Document.of(pomXml);
948+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
949+
950+
UpgradeContext context = createMockContext();
951+
UpgradeResult result = strategy.doApply(context, pomMap);
952+
953+
assertTrue(result.success(), "Compatibility fix should succeed");
954+
assertTrue(result.modifiedCount() > 0, "Should have commented out repository");
955+
956+
String xml = DomUtils.toXml(document);
957+
assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
958+
assertTrue(xml.contains("eclipseP2RepoId"), "Should mention the undefined property");
959+
960+
Element root = document.root();
961+
Element repos = DomUtils.findChildElement(root, "repositories");
962+
assertEquals(0, repos.childElements("repository").count(), "Should have no repository elements");
963+
}
964+
965+
@Test
966+
@DisplayName("should comment out repository with undefined property in url")
967+
void shouldCommentOutRepositoryWithUndefinedPropertyInUrl() throws Exception {
968+
String pomXml = """
969+
<?xml version="1.0" encoding="UTF-8"?>
970+
<project xmlns="http://maven.apache.org/POM/4.0.0">
971+
<modelVersion>4.0.0</modelVersion>
972+
<groupId>test</groupId>
973+
<artifactId>test</artifactId>
974+
<version>1.0.0</version>
975+
<repositories>
976+
<repository>
977+
<id>my-repo</id>
978+
<url>${undefinedBaseUrl}/releases</url>
979+
</repository>
980+
</repositories>
981+
</project>
982+
""";
983+
984+
Document document = Document.of(pomXml);
985+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
986+
987+
UpgradeContext context = createMockContext();
988+
UpgradeResult result = strategy.doApply(context, pomMap);
989+
990+
assertTrue(result.success(), "Compatibility fix should succeed");
991+
assertTrue(result.modifiedCount() > 0, "Should have commented out repository");
992+
993+
String xml = DomUtils.toXml(document);
994+
assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
995+
assertTrue(xml.contains("undefinedBaseUrl"), "Should mention the undefined property");
996+
}
997+
998+
@Test
999+
@DisplayName("should not comment out repository with defined property")
1000+
void shouldNotCommentOutRepositoryWithDefinedProperty() throws Exception {
1001+
String pomXml = """
1002+
<?xml version="1.0" encoding="UTF-8"?>
1003+
<project xmlns="http://maven.apache.org/POM/4.0.0">
1004+
<modelVersion>4.0.0</modelVersion>
1005+
<groupId>test</groupId>
1006+
<artifactId>test</artifactId>
1007+
<version>1.0.0</version>
1008+
<properties>
1009+
<repoId>my-custom-repo</repoId>
1010+
</properties>
1011+
<repositories>
1012+
<repository>
1013+
<id>${repoId}</id>
1014+
<url>https://repo.example.com</url>
1015+
</repository>
1016+
</repositories>
1017+
</project>
1018+
""";
1019+
1020+
Document document = Document.of(pomXml);
1021+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
1022+
1023+
UpgradeContext context = createMockContext();
1024+
strategy.doApply(context, pomMap);
1025+
1026+
Element root = document.root();
1027+
Element repos = DomUtils.findChildElement(root, "repositories");
1028+
assertEquals(1, repos.childElements("repository").count(), "Repository should still be present");
1029+
}
1030+
1031+
@Test
1032+
@DisplayName("should comment out plugin repository with undefined property")
1033+
void shouldCommentOutPluginRepositoryWithUndefinedProperty() throws Exception {
1034+
String pomXml = """
1035+
<?xml version="1.0" encoding="UTF-8"?>
1036+
<project xmlns="http://maven.apache.org/POM/4.0.0">
1037+
<modelVersion>4.0.0</modelVersion>
1038+
<groupId>test</groupId>
1039+
<artifactId>test</artifactId>
1040+
<version>1.0.0</version>
1041+
<pluginRepositories>
1042+
<pluginRepository>
1043+
<id>${eclipseP2RepoId}</id>
1044+
<url>https://plugins.example.com</url>
1045+
</pluginRepository>
1046+
</pluginRepositories>
1047+
</project>
1048+
""";
1049+
1050+
Document document = Document.of(pomXml);
1051+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
1052+
1053+
UpgradeContext context = createMockContext();
1054+
UpgradeResult result = strategy.doApply(context, pomMap);
1055+
1056+
assertTrue(result.success(), "Compatibility fix should succeed");
1057+
assertTrue(result.modifiedCount() > 0, "Should have commented out plugin repository");
1058+
1059+
String xml = DomUtils.toXml(document);
1060+
assertTrue(xml.contains("mvnup: commented out"), "Should contain comment-out marker");
1061+
1062+
Element root = document.root();
1063+
Element repos = DomUtils.findChildElement(root, "pluginRepositories");
1064+
assertEquals(
1065+
0, repos.childElements("pluginRepository").count(), "Should have no pluginRepository elements");
1066+
}
1067+
1068+
@Test
1069+
@DisplayName("should not comment out repository with well-known property")
1070+
void shouldNotCommentOutRepositoryWithWellKnownProperty() throws Exception {
1071+
String pomXml = """
1072+
<?xml version="1.0" encoding="UTF-8"?>
1073+
<project xmlns="http://maven.apache.org/POM/4.0.0">
1074+
<modelVersion>4.0.0</modelVersion>
1075+
<groupId>test</groupId>
1076+
<artifactId>test</artifactId>
1077+
<version>1.0.0</version>
1078+
<repositories>
1079+
<repository>
1080+
<id>local-repo</id>
1081+
<url>file://${project.basedir}/repo</url>
1082+
</repository>
1083+
</repositories>
1084+
</project>
1085+
""";
1086+
1087+
Document document = Document.of(pomXml);
1088+
Map<Path, Document> pomMap = Map.of(Paths.get("pom.xml"), document);
1089+
1090+
UpgradeContext context = createMockContext();
1091+
strategy.doApply(context, pomMap);
1092+
1093+
Element root = document.root();
1094+
Element repos = DomUtils.findChildElement(root, "repositories");
1095+
assertEquals(1, repos.childElements("repository").count(), "Repository should still be present");
1096+
}
1097+
}
1098+
9241099
@Nested
9251100
@DisplayName("Strategy Description")
9261101
class StrategyDescriptionTests {

‎impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,13 @@ public List<ArtifactRepository> createArtifactRepositories(
9292
List<ArtifactRepository> internalRepositories = new ArrayList<>();
9393

9494
for (Repository repository : pomRepositories) {
95+
if (containsExpression(repository.getId()) || containsExpression(repository.getUrl())) {
96+
logger.warn(
97+
"Skipping repository '{}' (url: '{}') containing an uninterpolated property expression",
98+
repository.getId(),
99+
repository.getUrl());
100+
continue;
101+
}
95102
internalRepositories.add(MavenRepositorySystem.buildArtifactRepository(repository));
96103
}
97104

@@ -267,6 +274,10 @@ public void selectProjectRealm(MavenProject project) {
267274
Thread.currentThread().setContextClassLoader(projectRealm);
268275
}
269276

277+
private static boolean containsExpression(String value) {
278+
return value != null && value.contains("${");
279+
}
280+
270281
private List<org.eclipse.aether.artifact.Artifact> toAetherArtifacts(final List<Artifact> pluginArtifacts) {
271282
return new ArrayList<>(RepositoryUtils.toArtifacts(pluginArtifacts));
272283
}

‎impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1624,11 +1624,11 @@ private void validateRawRepository(
16241624
if (matcher.find()) {
16251625
addViolation(
16261626
problems,
1627-
Severity.ERROR,
1627+
Severity.WARNING,
16281628
Version.V40,
16291629
prefix + prefix2 + "[" + repository.getId() + "].id",
16301630
null,
1631-
"contains an uninterpolated expression.",
1631+
"contains an uninterpolated expression; the repository will be skipped.",
16321632
repository);
16331633
}
16341634
}
@@ -1650,11 +1650,11 @@ && validateStringNotEmpty(
16501650
if (matcher.find()) {
16511651
addViolation(
16521652
problems,
1653-
Severity.ERROR,
1653+
Severity.WARNING,
16541654
Version.V40,
16551655
prefix + prefix2 + "[" + repository.getId() + "].url",
16561656
null,
1657-
"contains an uninterpolated expression.",
1657+
"contains an uninterpolated expression; the repository will be skipped.",
16581658
repository);
16591659
}
16601660
}

‎impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -915,31 +915,33 @@ void repositoryWithBasedirExpression() throws Exception {
915915
SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml");
916916
// This test runs on raw model without interpolation, so all expressions appear uninterpolated
917917
// In the real flow, supported expressions would be interpolated before validation
918-
assertViolations(result, 0, 3, 0);
918+
assertViolations(result, 0, 0, 3);
919919
}
920920

921921
@Test
922922
void repositoryWithUnsupportedExpression() throws Exception {
923923
SimpleProblemCollector result = validateRaw("raw-model/repository-with-unsupported-expression.xml");
924-
// Unsupported expressions should cause validation errors
925-
assertViolations(result, 0, 1, 0);
924+
// Unsupported expressions should cause validation warnings (repos will be skipped at build time)
925+
assertViolations(result, 0, 0, 1);
926926
}
927927

928928
@Test
929929
void repositoryWithUninterpolatedId() throws Exception {
930930
SimpleProblemCollector result = validateRaw("raw-model/repository-with-uninterpolated-id.xml");
931-
// Uninterpolated expressions in repository IDs should cause validation errors
931+
// Uninterpolated expressions in repository IDs should cause validation warnings
932+
// (repos will be skipped at build time)
932933
// distributionManagement repositories skip expression check since parent properties
933934
// may not be available at file model validation stage
934-
assertViolations(result, 0, 2, 0);
935+
assertViolations(result, 0, 0, 2);
935936

936-
// Check that repository ID validation errors are present for repositories and pluginRepositories
937-
assertTrue(result.getErrors().stream()
938-
.anyMatch(error -> error.contains("repositories.repository.[${repository.id}].id")
939-
&& error.contains("contains an uninterpolated expression")));
940-
assertTrue(result.getErrors().stream()
941-
.anyMatch(error -> error.contains("pluginRepositories.pluginRepository.[${plugin.repository.id}].id")
942-
&& error.contains("contains an uninterpolated expression")));
937+
// Check that repository ID validation warnings are present for repositories and pluginRepositories
938+
assertTrue(result.getWarnings().stream()
939+
.anyMatch(warning -> warning.contains("repositories.repository.[${repository.id}].id")
940+
&& warning.contains("contains an uninterpolated expression")));
941+
assertTrue(result.getWarnings().stream()
942+
.anyMatch(
943+
warning -> warning.contains("pluginRepositories.pluginRepository.[${plugin.repository.id}].id")
944+
&& warning.contains("contains an uninterpolated expression")));
943945
}
944946

945947
@Test

0 commit comments

Comments
 (0)