From ebfeb5d3d7413317c1e6ce0996c117f1c7972f15 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 7 Jun 2024 18:25:30 +0200 Subject: [PATCH 1/3] [MNG-8147] Restore profile ID invariance --- https://issues.apache.org/jira/browse/MNG-8147 --- .../model/building/DefaultModelBuilder.java | 63 ++++++------------- 1 file changed, 20 insertions(+), 43 deletions(-) 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 3043a76a0ea6..931e8c5bd379 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 @@ -35,9 +35,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Properties; -import java.util.Set; import java.util.function.Consumer; -import java.util.stream.Collectors; import org.apache.maven.artifact.versioning.DefaultArtifactVersion; import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException; @@ -305,18 +303,17 @@ protected ModelBuildingResult build(ModelBuildingRequest request, Collection interpolatedActivations = - getInterpolatedActivations(rawModel, profileActivationContext, problems); - injectProfileActivations(tmpModel, interpolatedActivations); + List interpolatedProfiles = getInterpolatedProfiles(rawModel, profileActivationContext, problems); + tmpModel.setProfiles(interpolatedProfiles); List activePomProfiles = profileSelector.getActiveProfiles(tmpModel.getProfiles(), profileActivationContext, problems); - Set activeProfileIds = - activePomProfiles.stream().map(Profile::getId).collect(Collectors.toSet()); - currentData.setActiveProfiles(rawModel.getProfiles().stream() - .filter(p -> activeProfileIds.contains(p.getId())) - .collect(Collectors.toList())); + List rawProfiles = new ArrayList<>(); + for (Profile activePomProfile : activePomProfiles) { + rawProfiles.add(rawModel.getProfiles().get(interpolatedProfiles.indexOf(activePomProfile))); + } + currentData.setActiveProfiles(rawProfiles); // profile injection for (Profile activeProfile : activePomProfiles) { @@ -429,12 +426,12 @@ private interface InterpolateString { String apply(String s) throws InterpolationException; } - private Map getInterpolatedActivations( + private List getInterpolatedProfiles( Model rawModel, DefaultProfileActivationContext context, DefaultModelProblemCollector problems) { - Map interpolatedActivations = getProfileActivations(rawModel, true); + List interpolatedActivations = getProfiles(rawModel, true); if (interpolatedActivations.isEmpty()) { - return Collections.emptyMap(); + return Collections.emptyList(); } RegexBasedInterpolator interpolator = new RegexBasedInterpolator(); @@ -466,8 +463,9 @@ void performFor(String value, String locationKey, Consumer mutator) { } } } - for (Activation activation : interpolatedActivations.values()) { - Optional a = Optional.of(activation); + for (Profile profile : interpolatedActivations) { + Activation activation = profile.getActivation(); + Optional a = Optional.ofNullable(activation); a.map(Activation::getFile).ifPresent(fa -> { Interpolation nt = new Interpolation(fa, s -> profileActivationFilePathInterpolator.interpolate(s, context)); @@ -753,41 +751,20 @@ private void assembleInheritance( } } - private Map getProfileActivations(Model model, boolean clone) { - Map activations = new HashMap<>(); + private List getProfiles(Model model, boolean clone) { + ArrayList profiles = new ArrayList<>(); for (Profile profile : model.getProfiles()) { - Activation activation = profile.getActivation(); - - if (activation == null) { - continue; - } - if (clone) { - activation = activation.clone(); - } - - activations.put(profile.getId(), activation); - } - - return activations; - } - - private void injectProfileActivations(Model model, Map activations) { - for (Profile profile : model.getProfiles()) { - Activation activation = profile.getActivation(); - - if (activation == null) { - continue; + profile = profile.clone(); } - - // restore activation - profile.setActivation(activations.get(profile.getId())); + profiles.add(profile); } + return profiles; } private Model interpolateModel(Model model, ModelBuildingRequest request, ModelProblemCollector problems) { // save profile activations before interpolation, since they are evaluated with limited scope - Map originalActivations = getProfileActivations(model, true); + List originalActivations = getProfiles(model, true); Model interpolatedModel = modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems); @@ -815,7 +792,7 @@ private Model interpolateModel(Model model, ModelBuildingRequest request, ModelP interpolatedModel.setPomFile(model.getPomFile()); // restore profiles with file activation to their value before full interpolation - injectProfileActivations(model, originalActivations); + model.setProfiles(originalActivations); return interpolatedModel; } From 4415dd964e75e1b20a14a8be56636f68b3e2fce9 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 7 Jun 2024 18:43:58 +0200 Subject: [PATCH 2/3] Compose in bits needed for MNG-8141 --- .../model/building/DefaultModelBuilder.java | 6 +++++ .../building/ModelBuildingException.java | 4 +-- .../DefaultArtifactDescriptorReader.java | 26 ++++++++++++++++++- 3 files changed, 33 insertions(+), 3 deletions(-) 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 931e8c5bd379..26ace56496c0 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 @@ -28,6 +28,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; @@ -463,7 +464,12 @@ void performFor(String value, String locationKey, Consumer mutator) { } } } + HashSet profileIds = new HashSet<>(); for (Profile profile : interpolatedActivations) { + if (!profileIds.add(profile.getId())) { + problems.add(new ModelProblemCollectorRequest(Severity.WARNING, ModelProblem.Version.BASE) + .setMessage("Duplicate activation for profile " + profile.getId())); + } Activation activation = profile.getActivation(); Optional a = Optional.ofNullable(activation); a.map(Activation::getFile).ifPresent(fa -> { diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java index b367ffee9363..233fcfb6e4ec 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java @@ -122,7 +122,7 @@ public List getProblems() { return Collections.unmodifiableList(result.getProblems()); } - private static String toMessage(ModelBuildingResult result) { + public static String toMessage(ModelBuildingResult result) { if (result != null && !result.getModelIds().isEmpty()) { return toMessage(result.getModelIds().get(0), result.getProblems()); } @@ -137,7 +137,7 @@ private static String toMessage(String modelId, List problems) { writer.print(problems.size()); writer.print((problems.size() == 1) ? " problem was " : " problems were "); writer.print("encountered while building the effective model"); - if (modelId != null && modelId.length() > 0) { + if (modelId != null && !modelId.isEmpty()) { writer.print(" for "); writer.print(modelId); } diff --git a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java index bbbc463e86c7..68f7f484beba 100644 --- a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java +++ b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java @@ -23,6 +23,7 @@ import javax.inject.Singleton; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Properties; @@ -37,6 +38,7 @@ import org.apache.maven.model.building.ModelBuilder; import org.apache.maven.model.building.ModelBuildingException; import org.apache.maven.model.building.ModelBuildingRequest; +import org.apache.maven.model.building.ModelBuildingResult; import org.apache.maven.model.building.ModelProblem; import org.apache.maven.model.resolution.UnresolvableModelException; import org.eclipse.aether.RepositoryEvent; @@ -67,6 +69,8 @@ import org.eclipse.aether.spi.locator.Service; import org.eclipse.aether.spi.locator.ServiceLocator; import org.eclipse.aether.transfer.ArtifactNotFoundException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * @author Benjamin Bentmann @@ -91,6 +95,8 @@ public class DefaultArtifactDescriptorReader implements ArtifactDescriptorReader private final ArtifactDescriptorReaderDelegate artifactDescriptorReaderDelegate = new ArtifactDescriptorReaderDelegate(); + private final Logger logger = LoggerFactory.getLogger(getClass()); + @Deprecated public DefaultArtifactDescriptorReader() { // enable no-arg constructor @@ -281,7 +287,25 @@ private Model loadPom( modelRequest.setModelSource(new FileModelSource(pomArtifact.getFile())); } - model = modelBuilder.build(modelRequest).getEffectiveModel(); + ModelBuildingResult modelResult = modelBuilder.build(modelRequest); + // ModelBuildingEx is thrown only on FATAL and ERROR severities, but we still can have WARNs + // that may lead to unexpected build failure, log them + if (!modelResult.getProblems().isEmpty()) { + if (logger.isDebugEnabled()) { + String message = ModelBuildingException.toMessage(modelResult); + if (message != null) { + logger.warn(message); + } + } else { + List problems = modelResult.getProblems(); + logger.warn( + "{} {} encountered while building the effective model for {}", + problems.size(), + (problems.size() == 1) ? "problem was" : "problems were", + request.getArtifact()); + } + } + model = modelResult.getEffectiveModel(); } catch (ModelBuildingException e) { for (ModelProblem problem : e.getProblems()) { if (problem.getException() instanceof UnresolvableModelException) { From e48469c366f0503fa51c8067363ae8a7e5dba44a Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 7 Jun 2024 18:58:40 +0200 Subject: [PATCH 3/3] Rollback unneeded change, improve logging --- .../building/ModelBuildingException.java | 4 ++-- .../DefaultArtifactDescriptorReader.java | 22 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java index 233fcfb6e4ec..b367ffee9363 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingException.java @@ -122,7 +122,7 @@ public List getProblems() { return Collections.unmodifiableList(result.getProblems()); } - public static String toMessage(ModelBuildingResult result) { + private static String toMessage(ModelBuildingResult result) { if (result != null && !result.getModelIds().isEmpty()) { return toMessage(result.getModelIds().get(0), result.getProblems()); } @@ -137,7 +137,7 @@ private static String toMessage(String modelId, List problems) { writer.print(problems.size()); writer.print((problems.size() == 1) ? " problem was " : " problems were "); writer.print("encountered while building the effective model"); - if (modelId != null && !modelId.isEmpty()) { + if (modelId != null && modelId.length() > 0) { writer.print(" for "); writer.print(modelId); } diff --git a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java index 68f7f484beba..4108d642f98f 100644 --- a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java +++ b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java @@ -40,6 +40,7 @@ import org.apache.maven.model.building.ModelBuildingRequest; import org.apache.maven.model.building.ModelBuildingResult; import org.apache.maven.model.building.ModelProblem; +import org.apache.maven.model.building.ModelProblemUtils; import org.apache.maven.model.resolution.UnresolvableModelException; import org.eclipse.aether.RepositoryEvent; import org.eclipse.aether.RepositoryEvent.EventType; @@ -291,18 +292,19 @@ private Model loadPom( // ModelBuildingEx is thrown only on FATAL and ERROR severities, but we still can have WARNs // that may lead to unexpected build failure, log them if (!modelResult.getProblems().isEmpty()) { + List problems = modelResult.getProblems(); + logger.warn( + "{} {} encountered while building the effective model for {}", + problems.size(), + (problems.size() == 1) ? "problem was" : "problems were", + request.getArtifact()); if (logger.isDebugEnabled()) { - String message = ModelBuildingException.toMessage(modelResult); - if (message != null) { - logger.warn(message); + for (ModelProblem problem : problems) { + logger.warn( + "{} @ {}", + problem.getMessage(), + ModelProblemUtils.formatLocation(problem, null)); } - } else { - List problems = modelResult.getProblems(); - logger.warn( - "{} {} encountered while building the effective model for {}", - problems.size(), - (problems.size() == 1) ? "problem was" : "problems were", - request.getArtifact()); } } model = modelResult.getEffectiveModel();