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 e8e61495bfaf..11d7276f8f72 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 @@ -33,7 +33,6 @@ import org.apache.maven.api.di.Singleton; import org.apache.maven.cling.invoker.mvnup.UpgradeContext; import org.jdom2.Attribute; -import org.jdom2.Comment; import org.jdom2.Content; import org.jdom2.Document; import org.jdom2.Element; @@ -498,25 +497,12 @@ private boolean fixRepositoryExpressions(Element repositoriesElement, Namespace Element urlElement = repository.getChild("url", namespace); if (urlElement != null) { String url = urlElement.getTextTrim(); - if (url.contains("${") - && !url.contains("${project.basedir}") - && !url.contains("${project.rootDirectory}")) { + if (url.contains("${")) { + // Allow repository URL interpolation; do not disable. + // Keep a gentle warning to help users notice unresolved placeholders at build time. String repositoryId = getChildText(repository, "id", namespace); - context.warning("Found unsupported expression in " + elementType + " URL (id: " + repositoryId + context.info("Detected interpolated expression in " + elementType + " URL (id: " + repositoryId + "): " + url); - context.warning( - "Maven 4 only supports ${project.basedir} and ${project.rootDirectory} expressions in repository URLs"); - - // Comment out the problematic repository - Comment comment = - new Comment(" Repository disabled due to unsupported expression in URL: " + url + " "); - Element parent = repository.getParentElement(); - parent.addContent(parent.indexOf(repository), comment); - removeElementWithFormatting(repository); - - context.detail("Fixed: " + "Commented out " + elementType + " with unsupported URL expression (id: " - + repositoryId + ")"); - fixed = true; } } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java index 9e2a776d7a7b..26f0b4fe98f1 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java @@ -41,6 +41,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiFunction; import java.util.function.Supplier; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -63,12 +64,15 @@ import org.apache.maven.api.model.Activation; import org.apache.maven.api.model.Dependency; import org.apache.maven.api.model.DependencyManagement; +import org.apache.maven.api.model.DeploymentRepository; +import org.apache.maven.api.model.DistributionManagement; import org.apache.maven.api.model.Exclusion; import org.apache.maven.api.model.InputLocation; import org.apache.maven.api.model.Mixin; import org.apache.maven.api.model.Model; import org.apache.maven.api.model.Parent; import org.apache.maven.api.model.Profile; +import org.apache.maven.api.model.Repository; import org.apache.maven.api.services.BuilderProblem; import org.apache.maven.api.services.BuilderProblem.Severity; import org.apache.maven.api.services.Interpolator; @@ -1441,6 +1445,29 @@ Model doReadFileModel() throws ModelBuilderException { model.getParent().getVersion())) : null) .build(); + // Interpolate repository URLs + if (model.getProjectDirectory() != null) { + String basedir = model.getProjectDirectory().toString(); + String basedirUri = model.getProjectDirectory().toUri().toString(); + properties.put("basedir", basedir); + properties.put("project.basedir", basedir); + properties.put("project.basedir.uri", basedirUri); + } + try { + String root = request.getSession().getRootDirectory().toString(); + String rootUri = + request.getSession().getRootDirectory().toUri().toString(); + properties.put("project.rootDirectory", root); + properties.put("project.rootDirectory.uri", rootUri); + } catch (IllegalStateException e) { + } + UnaryOperator callback = properties::get; + model = model.with() + .repositories(interpolateRepository(model.getRepositories(), callback)) + .pluginRepositories(interpolateRepository(model.getPluginRepositories(), callback)) + .profiles(map(model.getProfiles(), this::interpolateRepository, callback)) + .distributionManagement(interpolateRepository(model.getDistributionManagement(), callback)) + .build(); // Override model properties with user properties Map newProps = merge(model.getProperties(), session.getUserProperties()); if (newProps != null) { @@ -1471,6 +1498,41 @@ Model doReadFileModel() throws ModelBuilderException { return model; } + private DistributionManagement interpolateRepository( + DistributionManagement distributionManagement, UnaryOperator callback) { + return distributionManagement == null + ? null + : distributionManagement + .with() + .repository((DeploymentRepository) + interpolateRepository(distributionManagement.getRepository(), callback)) + .snapshotRepository((DeploymentRepository) + interpolateRepository(distributionManagement.getSnapshotRepository(), callback)) + .build(); + } + + private Profile interpolateRepository(Profile profile, UnaryOperator callback) { + return profile == null + ? null + : profile.with() + .repositories(interpolateRepository(profile.getRepositories(), callback)) + .pluginRepositories(interpolateRepository(profile.getPluginRepositories(), callback)) + .build(); + } + + private List interpolateRepository(List repositories, UnaryOperator callback) { + return map(repositories, this::interpolateRepository, callback); + } + + private Repository interpolateRepository(Repository repository, UnaryOperator callback) { + return repository == null + ? null + : repository + .with() + .url(interpolator.interpolate(repository.getUrl(), callback)) + .build(); + } + /** * Merges a list of model profiles with user-defined properties. * For each property defined in both the model and user properties, the user property value @@ -2340,4 +2402,21 @@ private Object getOuterRequest(Request req) { } return req; } + + private static List map(List resources, BiFunction mapper, A argument) { + List newResources = null; + if (resources != null) { + for (int i = 0; i < resources.size(); i++) { + T resource = resources.get(i); + T newResource = mapper.apply(resource, argument); + if (newResource != resource) { + if (newResources == null) { + newResources = new ArrayList<>(resources); + } + newResources.set(i, newResource); + } + } + } + return newResources; + } } 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 cd290dd16e24..8fcaf377568d 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 @@ -542,16 +542,6 @@ && equals(parent.getArtifactId(), model.getArtifactId())) { validationLevel); } - validateRawRepositories( - problems, model.getRepositories(), "repositories.repository.", EMPTY, validationLevel); - - validateRawRepositories( - problems, - model.getPluginRepositories(), - "pluginRepositories.pluginRepository.", - EMPTY, - validationLevel); - Build build = model.getBuild(); if (build != null) { validate20RawPlugins(problems, build.getPlugins(), "build.plugins.plugin.", EMPTY, validationLevel); @@ -605,16 +595,6 @@ && equals(parent.getArtifactId(), model.getArtifactId())) { validationLevel); } - validateRawRepositories( - problems, profile.getRepositories(), prefix, "repositories.repository.", validationLevel); - - validateRawRepositories( - problems, - profile.getPluginRepositories(), - prefix, - "pluginRepositories.pluginRepository.", - validationLevel); - BuildBase buildBase = profile.getBuild(); if (buildBase != null) { validate20RawPlugins(problems, buildBase.getPlugins(), prefix, "plugins.plugin.", validationLevel); @@ -685,6 +665,44 @@ && equals(parent.getArtifactId(), model.getArtifactId())) { parent); } } + + if (validationLevel > VALIDATION_LEVEL_MINIMAL) { + validateRawRepositories( + problems, model.getRepositories(), "repositories.repository.", EMPTY, validationLevel); + + validateRawRepositories( + problems, + model.getPluginRepositories(), + "pluginRepositories.pluginRepository.", + EMPTY, + validationLevel); + + for (Profile profile : model.getProfiles()) { + String prefix = "profiles.profile[" + profile.getId() + "]."; + + validateRawRepositories( + problems, profile.getRepositories(), prefix, "repositories.repository.", validationLevel); + + validateRawRepositories( + problems, + profile.getPluginRepositories(), + prefix, + "pluginRepositories.pluginRepository.", + validationLevel); + } + + DistributionManagement distMgmt = model.getDistributionManagement(); + if (distMgmt != null) { + validateRawRepository( + problems, distMgmt.getRepository(), "distributionManagement.repository.", "", true); + validateRawRepository( + problems, + distMgmt.getSnapshotRepository(), + "distributionManagement.snapshotRepository.", + "", + true); + } + } } private void validate30RawProfileActivation(ModelProblemCollector problems, Activation activation, String prefix) { @@ -1536,40 +1554,7 @@ private void validateRawRepositories( Map index = new HashMap<>(); for (Repository repository : repositories) { - validateStringNotEmpty( - prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository); - - if (validateStringNotEmpty( - prefix, - prefix2, - "[" + repository.getId() + "].url", - problems, - Severity.ERROR, - Version.V20, - repository.getUrl(), - null, - repository)) { - // only allow ${basedir} and ${project.basedir} - Matcher matcher = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl()); - while (matcher.find()) { - String expr = matcher.group(1); - if (!("basedir".equals(expr) - || "project.basedir".equals(expr) - || expr.startsWith("project.basedir.") - || "project.rootDirectory".equals(expr) - || expr.startsWith("project.rootDirectory."))) { - addViolation( - problems, - Severity.ERROR, - Version.V40, - prefix + prefix2 + "[" + repository.getId() + "].url", - null, - "contains an unsupported expression (only expressions starting with 'project.basedir' or 'project.rootDirectory' are supported).", - repository); - break; - } - } - } + validateRawRepository(problems, repository, prefix, prefix2, false); String key = repository.getId(); @@ -1593,6 +1578,44 @@ private void validateRawRepositories( } } + private void validateRawRepository( + ModelProblemCollector problems, + Repository repository, + String prefix, + String prefix2, + boolean allowEmptyUrl) { + if (repository == null) { + return; + } + validateStringNotEmpty( + prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository); + + if (!allowEmptyUrl + && validateStringNotEmpty( + prefix, + prefix2, + "[" + repository.getId() + "].url", + problems, + Severity.ERROR, + Version.V20, + repository.getUrl(), + null, + repository)) { + // Check for uninterpolated expressions - these should have been interpolated by now + Matcher matcher = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl()); + if (matcher.find()) { + addViolation( + problems, + Severity.ERROR, + Version.V40, + prefix + prefix2 + "[" + repository.getId() + "].url", + null, + "contains an uninterpolated expression.", + repository); + } + } + } + private void validate20EffectiveRepository( ModelProblemCollector problems, Repository repository, String prefix, int validationLevel) { if (repository != null) { 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 0403519dbab3..2fd9de0ae4fe 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 @@ -365,7 +365,7 @@ void testEmptyPluginVersion() throws Exception { @Test void testMissingRepositoryId() throws Exception { SimpleProblemCollector result = - validateFile("missing-repository-id-pom.xml", ModelValidator.VALIDATION_LEVEL_STRICT); + validateRaw("missing-repository-id-pom.xml", ModelValidator.VALIDATION_LEVEL_STRICT); assertViolations(result, 0, 4, 0); @@ -888,16 +888,23 @@ void testParentVersionRELEASE() throws Exception { @Test void repositoryWithExpression() throws Exception { SimpleProblemCollector result = validateFile("raw-model/repository-with-expression.xml"); - assertViolations(result, 0, 1, 0); - assertEquals( - "'repositories.repository.[repo].url' contains an unsupported expression (only expressions starting with 'project.basedir' or 'project.rootDirectory' are supported).", - result.getErrors().get(0)); + // Interpolation in repository URLs is allowed; unresolved placeholders will fail later during resolution + assertViolations(result, 0, 0, 0); } @Test void repositoryWithBasedirExpression() throws Exception { SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml"); - assertViolations(result, 0, 0, 0); + // 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); + } + + @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); } @Test diff --git a/impl/maven-impl/src/test/resources/poms/validation/raw-model/repository-with-unsupported-expression.xml b/impl/maven-impl/src/test/resources/poms/validation/raw-model/repository-with-unsupported-expression.xml new file mode 100644 index 000000000000..ed61d566aab0 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/raw-model/repository-with-unsupported-expression.xml @@ -0,0 +1,41 @@ + + + + + + 4.1.0 + + org.apache.maven.its.mng0000 + test + 1.0-SNAPSHOT + pom + + Maven Integration Test :: Test + Test unsupported repository URL expressions that should cause validation errors. + + + + repo-unsupported + ${project.baseUri}/sdk/maven/repo + + + + 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 new file mode 100644 index 000000000000..b3a29292399a --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoDmUnresolvedTest.java @@ -0,0 +1,48 @@ +/* + * 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.it; + +import java.io.File; + +import org.junit.jupiter.api.Test; + +/** + * IT to assert unresolved placeholders cause failure when used. + */ +class MavenITgh11140RepoDmUnresolvedTest extends AbstractMavenIntegrationTestCase { + + MavenITgh11140RepoDmUnresolvedTest() { + super("(4.0.0-rc-3,)"); + } + + @Test + void testFailsOnUnresolvedPlaceholders() 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.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 new file mode 100644 index 000000000000..d354b33f2ec8 --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11140RepoInterpolationTest.java @@ -0,0 +1,90 @@ +/* + * 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.it; + +import java.io.File; +import java.nio.file.Path; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * ITs for repository/distributionManagement URL interpolation. + */ +class MavenITgh11140RepoInterpolationTest extends AbstractMavenIntegrationTestCase { + + MavenITgh11140RepoInterpolationTest() { + super("(4.0.0-rc-3,)"); + } + + @Test + void testInterpolationFromEnvAndProps() throws Exception { + File testDir = extractResources("/gh-11140-repo-interpolation"); + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + + // Provide env vars consumed by POM via ${env.*} + Path base = testDir.toPath().toAbsolutePath(); + String baseUri = getBaseUri(base); + verifier.setEnvironmentVariable("IT_REPO_BASE", baseUri); + verifier.setEnvironmentVariable("IT_DM_BASE", baseUri); + + // Use a cheap goal that prints effective POM + verifier.addCliArgument("help:effective-pom"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + List lines = verifier.loadLogLines(); + // Expect resolved file:// URLs, not placeholders + assertTrue(lines.stream().anyMatch(s -> s.contains("envRepo")), "envRepo present"); + assertTrue(lines.stream().anyMatch(s -> s.contains("" + baseUri + "/repo")), "envRepo url resolved"); + assertTrue(lines.stream().anyMatch(s -> s.contains("propRepo")), "propRepo present"); + assertTrue( + lines.stream().anyMatch(s -> s.contains("" + baseUri + "/custom")), + "propRepo url resolved via property"); + assertTrue(lines.stream().anyMatch(s -> s.contains("distRepo")), "distRepo present"); + assertTrue( + lines.stream().anyMatch(s -> s.contains("" + baseUri + "/dist")), "distRepo url resolved"); + } + + private static String getBaseUri(Path base) { + String baseUri = base.toUri().toString(); + if (baseUri.endsWith("/")) { + baseUri = baseUri.substring(0, baseUri.length() - 1); + } + return baseUri; + } + + @Test + void testUnresolvedPlaceholderFailsResolution() 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.verifyTextInLog("contains an uninterpolated expression"); + } +} diff --git a/its/core-it-suite/src/test/resources/gh-11140-repo-dm-unresolved/pom.xml b/its/core-it-suite/src/test/resources/gh-11140-repo-dm-unresolved/pom.xml new file mode 100644 index 000000000000..106bb79dc367 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-11140-repo-dm-unresolved/pom.xml @@ -0,0 +1,42 @@ + + + + org.apache.maven.its.repointerp + repo-dm-unresolved + 1.0 + pom + + Maven Integration Test :: Unresolved placeholders must fail + Verify that unresolved placeholders in repository/distributionManagement cause failure when used. + + + + badDist + ${env.MISSING_VAR}/dist + + + + + + badRepo + ${env.MISSING_VAR}/repo + + + diff --git a/its/core-it-suite/src/test/resources/gh-11140-repo-interpolation/pom.xml b/its/core-it-suite/src/test/resources/gh-11140-repo-interpolation/pom.xml new file mode 100644 index 000000000000..5f07980e74b5 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-11140-repo-interpolation/pom.xml @@ -0,0 +1,51 @@ + + + + org.apache.maven.its.repointerp + repo-interpolation + 1.0 + pom + + Maven Integration Test :: Repository and DistributionManagement URL interpolation + Verify that repository and distributionManagement URLs are interpolated from env and project properties. + + + + distRepo + ${env.IT_DM_BASE}/dist + + + + + + ${env.IT_REPO_BASE}/custom + + + + + envRepo + ${env.IT_REPO_BASE}/repo + + + propRepo + ${customRepoUrl} + + +