From 1d0ca63fbd5f3828ea017c32d2de868eaa190d72 Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Tue, 13 Feb 2024 16:51:42 +0100 Subject: [PATCH 1/3] [MNG-8050] emit warn in case of repo id clashes between settings and POM --- maven-model-builder/pom.xml | 1 + .../model/building/DefaultModelBuilder.java | 1 + .../validation/DefaultModelValidator.java | 33 +++++++++++++++++++ .../model/validation/ModelValidator.java | 21 ++++++++++++ 4 files changed, 56 insertions(+) diff --git a/maven-model-builder/pom.xml b/maven-model-builder/pom.xml index f98c7e019fc..be93a2a485e 100644 --- a/maven-model-builder/pom.xml +++ b/maven-model-builder/pom.xml @@ -179,6 +179,7 @@ under the License. org.apache.maven.model.superpom.SuperPomProvider#getSuperModel(java.lang.String):METHOD_RETURN_TYPE_CHANGED org.apache.maven.model.validation.DefaultModelValidator#validateDependencyVersion(org.apache.maven.model.building.ModelProblemCollector,org.apache.maven.model.Dependency,java.lang.String):METHOD_REMOVED org.apache.maven.model.validation.ModelValidator#validateFileModel(org.apache.maven.model.Model,org.apache.maven.model.building.ModelBuildingRequest,org.apache.maven.model.building.ModelProblemCollector):METHOD_NEW_DEFAULT + org.apache.maven.model.validation.ModelValidator#validateExternalProfiles(java.util.List,org.apache.maven.model.Model,org.apache.maven.model.building.ModelBuildingRequest,org.apache.maven.model.building.ModelProblemCollector):METHOD_NEW_DEFAULT diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java index 70455795134..73d5f3b14a5 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java @@ -754,6 +754,7 @@ private void activateFileModel( profileInjector.injectProfile(inputModel, activeProfile, request, problems); } + modelValidator.validateExternalProfiles(activeExternalProfiles, inputModel, request, problems); for (Profile activeProfile : activeExternalProfiles) { profileInjector.injectProfile(inputModel, activeProfile, request, problems); } diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 246e2f8b131..c4652424e92 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -41,7 +41,10 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +<<<<<<< Upstream, based on master import java.util.stream.StreamSupport; +======= +>>>>>>> 4f3fe69 [MNG-8050] emit warn in case of repo id clashes between settings and POM import org.apache.maven.api.model.Activation; import org.apache.maven.api.model.ActivationFile; @@ -788,6 +791,36 @@ public void validateEffectiveModel(Model ma, ModelBuildingRequest request, Model } } + @Override + public void validateExternalProfiles( + List activeExternalProfiles, + Model ma, + ModelBuildingRequest request, + ModelProblemCollector problems) { + org.apache.maven.api.model.Model m = ma.getDelegate(); + // check for id clashes in repositories + for (Profile profile : activeExternalProfiles.stream() + .map(org.apache.maven.model.Profile::getDelegate) + .collect(Collectors.toList())) { + for (Repository repository : profile.getRepositories()) { + Optional clashingPomRepository = m.getRepositories().stream() + .filter(r -> r.getId().equals(repository.getId())) + .findFirst(); + if (clashingPomRepository.isPresent()) { + addViolation( + problems, + Severity.WARNING, + Version.V40, // ? + "pom repository", + "?", + "is overwritten by the repository with same id from external profile with id " + + profile.getId(), + clashingPomRepository.get()); + } + } + } + } + private void validate20RawDependencies( ModelProblemCollector problems, List dependencies, diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java index 2817d95fc66..bb87370e0ef 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/ModelValidator.java @@ -18,7 +18,10 @@ */ package org.apache.maven.model.validation; +import java.util.List; + import org.apache.maven.model.Model; +import org.apache.maven.model.Profile; import org.apache.maven.model.building.ModelBuildingRequest; import org.apache.maven.model.building.ModelProblemCollector; @@ -49,6 +52,24 @@ default void validateFileModel(Model model, ModelBuildingRequest request, ModelP */ void validateRawModel(Model model, ModelBuildingRequest request, ModelProblemCollector problems); + /** + * Checks the specified (raw) model for clashes with the passed active external profiles. The raw model is the + * file model + buildpom filter transformation and has not been subjected to inheritance, interpolation or profile/default injection. + * + * @param activeExternalProfiles the active profiles coming from external sources (settings.xml), must not be {@code null} + * @param model The model to validate, must not be {@code null}. + * @param request The model building request that holds further settings, must not be {@code null}. + * @param problems The container used to collect problems that were encountered, must not be {@code null}. + * @since 4.0.0 + */ + default void validateExternalProfiles( + List activeExternalProfiles, + Model model, + ModelBuildingRequest request, + ModelProblemCollector problems) { + // do nothing + } + /** * Checks the specified (effective) model for missing or invalid values. The effective model is fully assembled and * has undergone inheritance, interpolation and other model operations. From f32e0cbc9c36776bbbf02a71064f6281f4b64dcb Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Tue, 13 Feb 2024 19:53:09 +0100 Subject: [PATCH 2/3] improve message, validate pluginRepositories as well --- .../apache/maven/model/validation/DefaultModelValidator.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index c4652424e92..25838c5787e 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -41,10 +41,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; -<<<<<<< Upstream, based on master import java.util.stream.StreamSupport; -======= ->>>>>>> 4f3fe69 [MNG-8050] emit warn in case of repo id clashes between settings and POM import org.apache.maven.api.model.Activation; import org.apache.maven.api.model.ActivationFile; From 23cf492ef959330e21933197a21cb10009cd5e5d Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Mon, 4 Mar 2024 11:13:33 +0100 Subject: [PATCH 3/3] only emit warnings when urls differ --- .../validation/DefaultModelValidator.java | 49 +++++++++++++------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 25838c5787e..b51bbc22c73 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -24,6 +24,7 @@ import java.io.File; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Deque; import java.util.HashMap; @@ -799,21 +800,39 @@ public void validateExternalProfiles( for (Profile profile : activeExternalProfiles.stream() .map(org.apache.maven.model.Profile::getDelegate) .collect(Collectors.toList())) { - for (Repository repository : profile.getRepositories()) { - Optional clashingPomRepository = m.getRepositories().stream() - .filter(r -> r.getId().equals(repository.getId())) - .findFirst(); - if (clashingPomRepository.isPresent()) { - addViolation( - problems, - Severity.WARNING, - Version.V40, // ? - "pom repository", - "?", - "is overwritten by the repository with same id from external profile with id " - + profile.getId(), - clashingPomRepository.get()); - } + String externalRepositoriesSource = "external profile with id '" + profile.getId() + "' in settings.xml"; + validateUniqueRepositoryIds( + false, m.getRepositories(), profile.getRepositories(), externalRepositoriesSource, problems); + validateUniqueRepositoryIds( + true, + m.getPluginRepositories(), + profile.getPluginRepositories(), + externalRepositoriesSource, + problems); + } + } + + private void validateUniqueRepositoryIds( + boolean isPluginRepository, + Collection pomRepositories, + Collection externalRepositories, + String externalRepositoriesSource, + ModelProblemCollector problems) { + for (Repository externalRepository : externalRepositories) { + Optional clashingPomRepository = pomRepositories.stream() + .filter(r -> Objects.equals(r.getId(), externalRepository.getId())) + .filter(r -> !Objects.equals(r.getUrl(), externalRepository.getUrl())) + .findFirst(); + if (clashingPomRepository.isPresent()) { + addViolation( + problems, + Severity.WARNING, + Version.BASE, + isPluginRepository ? "pluginRepositories.repository" : "repositories.repository", + clashingPomRepository.get().getId(), + "is overwritten by the repository with same id but having a different url from " + + externalRepositoriesSource, + clashingPomRepository.get()); } } }