This is an automated email from the ASF dual-hosted git repository.

cstamas pushed a commit to branch maven-3.9.x
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/maven-3.9.x by this push:
     new 7fcd8c5d15 [MNG-8141] Model builder should report problems it finds 
during build (#1556)
7fcd8c5d15 is described below

commit 7fcd8c5d154bbf7163350d4d3fbfd03fbe17cf93
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Thu Jun 6 21:21:17 2024 +0200

    [MNG-8141] Model builder should report problems it finds during build 
(#1556)
    
    And not rely that model was validated, which is not true in some cases. 
Model builder can still easily detect issues with models while building them.
    
    Provides "escape hatch" for projects stuck on invalid models in form of 
user property that can be enabled with 
`-Dmaven.modelBuilder.failOnInvalidModel=false`, this reverts to _old_ 
behaviour of maven, and the JavaFX reproducer goes back to error "unable to 
resolve" errors with uninterpolated `${javafx.platform}` property as classifier.
    
    ---
    
    https://issues.apache.org/jira/browse/MNG-8141
---
 .../maven/model/building/DefaultModelBuilder.java  | 47 ++++++++++++++++---
 .../model/building/DefaultModelBuilderTest.java    | 36 +++++++++++++++
 .../test/resources/poms/building/badprofiles.xml   | 54 ++++++++++++++++++++++
 3 files changed, 131 insertions(+), 6 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 3043a76a0e..04a6a9e622 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
@@ -97,6 +97,30 @@ import static 
org.apache.maven.model.building.Result.newResult;
 @Named
 @Singleton
 public class DefaultModelBuilder implements ModelBuilder {
+    /**
+     * Key for "fail on invalid model" property.
+     * <p>
+     * Visible for testing.
+     */
+    static final String FAIL_ON_INVALID_MODEL = 
"maven.modelBuilder.failOnInvalidModel";
+
+    /**
+     * Checks user and system properties (in this order) for value of {@link 
#FAIL_ON_INVALID_MODEL} property key, if
+     * set and returns it. If not set, defaults to {@code true}.
+     * <p>
+     * This is only meant to provide "escape hatch" for those builds, that are 
for some reason stuck with invalid models.
+     */
+    private static boolean isFailOnInvalidModel(ModelBuildingRequest request) {
+        String val = 
request.getUserProperties().getProperty(FAIL_ON_INVALID_MODEL);
+        if (val == null) {
+            val = 
request.getSystemProperties().getProperty(FAIL_ON_INVALID_MODEL);
+        }
+        if (val != null) {
+            return Boolean.parseBoolean(val);
+        }
+        return true;
+    }
+
     @Inject
     private ModelProcessor modelProcessor;
 
@@ -253,6 +277,7 @@ public class DefaultModelBuilder implements ModelBuilder {
     protected ModelBuildingResult build(ModelBuildingRequest request, 
Collection<String> importIds)
             throws ModelBuildingException {
         // phase 1
+        boolean failOnInvalidModel = isFailOnInvalidModel(request);
         DefaultModelBuildingResult result = new DefaultModelBuildingResult();
 
         DefaultModelProblemCollector problems = new 
DefaultModelProblemCollector(result);
@@ -306,7 +331,7 @@ public class DefaultModelBuilder implements ModelBuilder {
             
profileActivationContext.setProjectProperties(tmpModel.getProperties());
 
             Map<String, Activation> interpolatedActivations =
-                    getInterpolatedActivations(rawModel, 
profileActivationContext, problems);
+                    getInterpolatedActivations(rawModel, 
profileActivationContext, failOnInvalidModel, problems);
             injectProfileActivations(tmpModel, interpolatedActivations);
 
             List<Profile> activePomProfiles =
@@ -430,8 +455,12 @@ public class DefaultModelBuilder implements ModelBuilder {
     }
 
     private Map<String, Activation> getInterpolatedActivations(
-            Model rawModel, DefaultProfileActivationContext context, 
DefaultModelProblemCollector problems) {
-        Map<String, Activation> interpolatedActivations = 
getProfileActivations(rawModel, true);
+            Model rawModel,
+            DefaultProfileActivationContext context,
+            boolean failOnInvalidModel,
+            DefaultModelProblemCollector problems) {
+        Map<String, Activation> interpolatedActivations =
+                getProfileActivations(rawModel, true, failOnInvalidModel, 
problems);
 
         if (interpolatedActivations.isEmpty()) {
             return Collections.emptyMap();
@@ -753,7 +782,8 @@ public class DefaultModelBuilder implements ModelBuilder {
         }
     }
 
-    private Map<String, Activation> getProfileActivations(Model model, boolean 
clone) {
+    private Map<String, Activation> getProfileActivations(
+            Model model, boolean clone, boolean failOnInvalidModel, 
ModelProblemCollector problems) {
         Map<String, Activation> activations = new HashMap<>();
         for (Profile profile : model.getProfiles()) {
             Activation activation = profile.getActivation();
@@ -766,7 +796,11 @@ public class DefaultModelBuilder implements ModelBuilder {
                 activation = activation.clone();
             }
 
-            activations.put(profile.getId(), activation);
+            if (activations.put(profile.getId(), activation) != null) {
+                problems.add(new ModelProblemCollectorRequest(
+                                failOnInvalidModel ? Severity.FATAL : 
Severity.WARNING, ModelProblem.Version.BASE)
+                        .setMessage("Duplicate activation for profile " + 
profile.getId()));
+            }
         }
 
         return activations;
@@ -787,7 +821,8 @@ public class DefaultModelBuilder implements ModelBuilder {
 
     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);
+        // at this stage we already failed if wanted to
+        Map<String, Activation> originalActivations = 
getProfileActivations(model, true, false, problems);
 
         Model interpolatedModel =
                 modelInterpolator.interpolateModel(model, 
model.getProjectDirectory(), request, problems);
diff --git 
a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java
 
b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java
index 59fb447500..13a8ac0e47 100644
--- 
a/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java
+++ 
b/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java
@@ -18,6 +18,8 @@
  */
 package org.apache.maven.model.building;
 
+import java.io.File;
+
 import org.apache.maven.model.Dependency;
 import org.apache.maven.model.Parent;
 import org.apache.maven.model.Repository;
@@ -27,6 +29,8 @@ import 
org.apache.maven.model.resolution.UnresolvableModelException;
 import org.junit.Test;
 
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * @author Guillaume Nodet
@@ -87,6 +91,38 @@ public class DefaultModelBuilderTest {
         builder.build(request);
     }
 
+    @Test
+    public void testBadProfiles() {
+        ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
+        assertNotNull(builder);
+
+        DefaultModelBuildingRequest request = new 
DefaultModelBuildingRequest();
+        
request.setValidationLevel(ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL);
+        request.setModelSource(new FileModelSource(new 
File("src/test/resources/poms/building/badprofiles.xml")));
+        request.setModelResolver(new BaseModelResolver());
+
+        try {
+            builder.build(request); // throw, making "pom not available"
+            fail();
+        } catch (ModelBuildingException e) {
+            assertTrue(e.getMessage().contains("Duplicate activation for 
profile badprofile"));
+        }
+    }
+
+    @Test
+    public void testBadProfilesCheckDisabled() throws Exception {
+        ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
+        assertNotNull(builder);
+
+        DefaultModelBuildingRequest request = new 
DefaultModelBuildingRequest();
+        
request.getUserProperties().setProperty(DefaultModelBuilder.FAIL_ON_INVALID_MODEL,
 "false");
+        
request.setValidationLevel(ModelBuildingRequest.VALIDATION_LEVEL_MINIMAL);
+        request.setModelSource(new FileModelSource(new 
File("src/test/resources/poms/building/badprofiles.xml")));
+        request.setModelResolver(new BaseModelResolver());
+
+        builder.build(request); // does not throw, old behaviour (but result 
may be fully off)
+    }
+
     static class CycleInImportsResolver extends BaseModelResolver {
         @Override
         public ModelSource resolveModel(Dependency dependency) throws 
UnresolvableModelException {
diff --git 
a/maven-model-builder/src/test/resources/poms/building/badprofiles.xml 
b/maven-model-builder/src/test/resources/poms/building/badprofiles.xml
new file mode 100644
index 0000000000..e098a70fb9
--- /dev/null
+++ b/maven-model-builder/src/test/resources/poms/building/badprofiles.xml
@@ -0,0 +1,54 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+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.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0";
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
https://maven.apache.org/xsd/maven-4.0.0.xsd";>
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>test</groupId>
+  <artifactId>test</artifactId>
+  <version>0.1-SNAPSHOT</version>
+  <packaging>pom</packaging>
+
+  <profiles>
+    <profile>
+      <id>badprofile</id>
+      <activation>
+        <activeByDefault>true</activeByDefault>
+      </activation>
+      <properties>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+      </properties>
+    </profile>
+    <profile>
+      <id>badprofile</id>
+      <activation>
+        <file>
+          <exists>simple.xml</exists>
+        </file>
+      </activation>
+      <properties>
+        <profile.file>activated</profile.file>
+      </properties>
+    </profile>
+  </profiles>
+</project>

Reply via email to