From ccb01f7dff8d216379d02471926a5810564b947e Mon Sep 17 00:00:00 2001
From: Tamas Cservenak <tamas@cservenak.net>
Date: Fri, 7 Jun 2024 15:26:30 +0200
Subject: [PATCH 1/2] [MNG-8141] Another attempt

WIP
---
 .../model/building/DefaultModelBuilder.java   | 12 ++++----
 .../building/ModelBuildingException.java      |  4 +--
 .../DefaultArtifactDescriptorReader.java      | 28 ++++++++++++++++++-
 3 files changed, 36 insertions(+), 8 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..99c50a9e37cd 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
@@ -431,7 +431,7 @@ private interface InterpolateString {
 
     private Map<String, Activation> getInterpolatedActivations(
             Model rawModel, DefaultProfileActivationContext context, DefaultModelProblemCollector problems) {
-        Map<String, Activation> interpolatedActivations = getProfileActivations(rawModel, true);
+        Map<String, Activation> interpolatedActivations = getProfileActivations(rawModel, true, problems);
 
         if (interpolatedActivations.isEmpty()) {
             return Collections.emptyMap();
@@ -753,7 +753,7 @@ private void assembleInheritance(
         }
     }
 
-    private Map<String, Activation> getProfileActivations(Model model, boolean clone) {
+    private Map<String, Activation> getProfileActivations(Model model, boolean clone, ModelProblemCollector problems) {
         Map<String, Activation> activations = new HashMap<>();
         for (Profile profile : model.getProfiles()) {
             Activation activation = profile.getActivation();
@@ -766,9 +766,11 @@ private Map<String, Activation> getProfileActivations(Model model, boolean clone
                 activation = activation.clone();
             }
 
-            activations.put(profile.getId(), activation);
+            if (activations.put(profile.getId(), activation) != null) {
+                problems.add(new ModelProblemCollectorRequest(Severity.WARNING, ModelProblem.Version.BASE)
+                        .setMessage("Duplicate activation for profile " + profile.getId()));
+            }
         }
-
         return activations;
     }
 
@@ -787,7 +789,7 @@ private void injectProfileActivations(Model model, Map<String, Activation> activ
 
     private Model interpolateModel(Model model, ModelBuildingRequest request, ModelProblemCollector problems) {
         // save profile activations before interpolation, since they are evaluated with limited scope
-        Map<String, Activation> originalActivations = getProfileActivations(model, true);
+        Map<String, Activation> originalActivations = getProfileActivations(model, true, problems);
 
         Model interpolatedModel =
                 modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems);
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<ModelProblem> 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<ModelProblem> 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..7626ae8553fa 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,27 @@ 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<ModelProblem> problems = modelResult.getProblems();
+                        logger.warn(
+                                "{} {} encountered while building the effective model for {}",
+                                problems.size(),
+                                (problems.size() == 1) ? " problem was " : " problems were ",
+                                modelResult.getModelIds().isEmpty()
+                                        ? "n/a"
+                                        : modelResult.getModelIds().get(0));
+                    }
+                }
+                model = modelResult.getEffectiveModel();
             } catch (ModelBuildingException e) {
                 for (ModelProblem problem : e.getProblems()) {
                     if (problem.getException() instanceof UnresolvableModelException) {

From 3f1e391acc5df0ad9ae9d42c1d676e42eab1e5cc Mon Sep 17 00:00:00 2001
From: Tamas Cservenak <tamas@cservenak.net>
Date: Fri, 7 Jun 2024 15:48:24 +0200
Subject: [PATCH 2/2] Use artifact in message

That way is clear which artifact POM are wrong.
---
 .../internal/DefaultArtifactDescriptorReader.java           | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

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 7626ae8553fa..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
@@ -301,10 +301,8 @@ private Model loadPom(
                         logger.warn(
                                 "{} {} encountered while building the effective model for {}",
                                 problems.size(),
-                                (problems.size() == 1) ? " problem was " : " problems were ",
-                                modelResult.getModelIds().isEmpty()
-                                        ? "n/a"
-                                        : modelResult.getModelIds().get(0));
+                                (problems.size() == 1) ? "problem was" : "problems were",
+                                request.getArtifact());
                     }
                 }
                 model = modelResult.getEffectiveModel();