You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by st...@apache.org on 2019/06/03 20:34:21 UTC

[maven] branch master updated: [MNG-6667] Enhance the error reporting when trying to build a modelVersion that the current Maven doesn't understand

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

stephenc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/master by this push:
     new 01405a2  [MNG-6667] Enhance the error reporting when trying to build a modelVersion that the current Maven doesn't understand
01405a2 is described below

commit 01405a2d60ec44f9c11843a8f904822a9ed280fc
Author: Stephen Connolly <st...@apache.org>
AuthorDate: Mon Jun 3 21:33:44 2019 +0100

    [MNG-6667] Enhance the error reporting when trying to build a modelVersion that the current Maven doesn't understand
---
 .../project/DefaultMavenProjectBuilderTest.java    |  68 +++++++++++++-
 .../projects/future-model-version-pom.xml          |  25 ++++++
 .../projects/future-schema-model-version-pom.xml   |  24 +++++
 .../resources/projects/past-model-version-pom.xml  |  26 ++++++
 .../model/validation/DefaultModelValidator.java    | 100 ++++++++++++++++++---
 .../validation/DefaultModelValidatorTest.java      |   4 +-
 6 files changed, 229 insertions(+), 18 deletions(-)

diff --git a/maven-core/src/test/java/org/apache/maven/project/DefaultMavenProjectBuilderTest.java b/maven-core/src/test/java/org/apache/maven/project/DefaultMavenProjectBuilderTest.java
index 73629bc..f82800b 100644
--- a/maven-core/src/test/java/org/apache/maven/project/DefaultMavenProjectBuilderTest.java
+++ b/maven-core/src/test/java/org/apache/maven/project/DefaultMavenProjectBuilderTest.java
@@ -19,15 +19,15 @@ package org.apache.maven.project;
  * under the License.
  */
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.repository.ArtifactRepository;
 import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout;
 import org.codehaus.plexus.util.FileUtils;
 
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
 public class DefaultMavenProjectBuilderTest
     extends AbstractMavenProjectTestCase
 {
@@ -112,6 +112,66 @@ public class DefaultMavenProjectBuilderTest
         assertEquals( "first", project.getBuildPlugins().get( 0 ).getExecutions().get( 0 ).getId() );
     }
 
+    public void testFutureModelVersion()
+        throws Exception
+    {
+        File f1 = getTestFile( "src/test/resources/projects/future-model-version-pom.xml" );
+
+        try
+        {
+            getProject( f1 );
+            fail( "Expected to fail for future versions" );
+        }
+        catch ( ProjectBuildingException e )
+        {
+            assertContains( "Building this project requires a newer version of Maven", e.getMessage() );
+        }
+    }
+
+    public void testPastModelVersion()
+        throws Exception
+    {
+        // a Maven 1.x pom will not even
+        // update the resource if we stop supporting modelVersion 4.0.0
+        File f1 = getTestFile( "src/test/resources/projects/past-model-version-pom.xml" );
+
+        try
+        {
+            getProject( f1 );
+            fail( "Expected to fail for past versions" );
+        }
+        catch ( ProjectBuildingException e )
+        {
+            assertContains( "Building this project requires an older version of Maven", e.getMessage() );
+        }
+    }
+
+    public void testFutureSchemaModelVersion()
+        throws Exception
+    {
+        File f1 = getTestFile( "src/test/resources/projects/future-schema-model-version-pom.xml" );
+
+        try
+        {
+            getProject( f1 );
+            fail( "Expected to fail for future versions" );
+        }
+        catch ( ProjectBuildingException e )
+        {
+            assertContains( "Building this project requires a newer version of Maven", e.getMessage() );
+        }
+    }
+
+    private void assertContains( String expected, String actual )
+    {
+        if ( actual == null || !actual.contains( expected ) )
+        {
+            fail( "Expected: a string containing " + expected + "\nActual: " + ( actual == null
+                ? "null"
+                : "'" + actual + "'" ) );
+        }
+    }
+
     public void testBuildStubModelForMissingRemotePom()
         throws Exception
     {
diff --git a/maven-core/src/test/resources/projects/future-model-version-pom.xml b/maven-core/src/test/resources/projects/future-model-version-pom.xml
new file mode 100644
index 0000000..1a73a44
--- /dev/null
+++ b/maven-core/src/test/resources/projects/future-model-version-pom.xml
@@ -0,0 +1,25 @@
+<!--
+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>
+    <modelVersion>4.0.1</modelVersion>
+    <groupId>tests.project</groupId>
+    <artifactId>future-model-version</artifactId>
+    <version>1</version>
+</project>
diff --git a/maven-core/src/test/resources/projects/future-schema-model-version-pom.xml b/maven-core/src/test/resources/projects/future-schema-model-version-pom.xml
new file mode 100644
index 0000000..f234aab
--- /dev/null
+++ b/maven-core/src/test/resources/projects/future-schema-model-version-pom.xml
@@ -0,0 +1,24 @@
+<!--
+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>
+    <modelVersion>4.999.999</modelVersion>
+    <!-- this should be a fake "future" version that only has modelVersion understood by us -->
+    <coordinates>tests.project:future-model-version</coordinates>
+</project>
diff --git a/maven-core/src/test/resources/projects/past-model-version-pom.xml b/maven-core/src/test/resources/projects/past-model-version-pom.xml
new file mode 100644
index 0000000..cdc7f85
--- /dev/null
+++ b/maven-core/src/test/resources/projects/past-model-version-pom.xml
@@ -0,0 +1,26 @@
+<!--
+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>
+    <!-- once we move past supporting modelVersion 4.0.0 we should update this version -->
+    <modelVersion>3.9.9</modelVersion>
+    <groupId>tests.project</groupId>
+    <artifactId>past-model-version</artifactId>
+    <version>1</version>
+</project>
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 749dd2f..722c8c2 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
@@ -19,16 +19,6 @@ package org.apache.maven.model.validation;
  * under the License.
  */
 
-import java.io.File;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 import org.apache.maven.model.Activation;
 import org.apache.maven.model.ActivationFile;
 import org.apache.maven.model.Build;
@@ -58,6 +48,16 @@ import org.apache.maven.model.interpolation.AbstractStringBasedModelInterpolator
 import org.codehaus.plexus.component.annotations.Component;
 import org.codehaus.plexus.util.StringUtils;
 
+import java.io.File;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 /**
  * @author <a href="mailto:trygvis@inamo.no">Trygve Laugst&oslash;l</a>
  */
@@ -122,8 +122,7 @@ public class DefaultModelValidator
             // models without a version starting with 3.4.
             validateStringNotEmpty( "modelVersion", problems, Severity.ERROR, Version.V20, m.getModelVersion(), m );
 
-            validateEnum( "modelVersion", problems, Severity.ERROR, Version.V20, m.getModelVersion(), null, m,
-                          "4.0.0" );
+            validateModelVersion( problems, m.getModelVersion(), m, "4.0.0" );
 
             validateStringNoExpression( "groupId", problems, Severity.WARNING, Version.V20, m.getGroupId(), m );
             if ( parent == null )
@@ -1042,6 +1041,83 @@ public class DefaultModelValidator
     }
 
     @SuppressWarnings( "checkstyle:parameternumber" )
+    private boolean validateModelVersion( ModelProblemCollector problems, String string, InputLocationTracker tracker,
+                                          String... validVersions )
+    {
+        if ( string == null || string.length() <= 0 )
+        {
+            return true;
+        }
+
+        List<String> values = Arrays.asList( validVersions );
+
+        if ( values.contains( string ) )
+        {
+            return true;
+        }
+
+        boolean newerThanAll = true;
+        boolean olderThanAll = true;
+        for ( String validValue : validVersions )
+        {
+            final int comparison = compareModelVersions( validValue, string );
+            newerThanAll = newerThanAll && comparison < 0;
+            olderThanAll = olderThanAll && comparison > 0;
+        }
+
+        if ( newerThanAll )
+        {
+            addViolation( problems, Severity.FATAL, Version.V20, "modelVersion", null,
+                          "of '" + string + "' is newer than the versions supported by this version of Maven: " + values
+                              + ". Building this project requires a newer version of Maven.", tracker );
+
+        }
+        else if ( olderThanAll )
+        {
+            // note this will not be hit for Maven 1.x project.xml as it is an incompatible schema
+            addViolation( problems, Severity.FATAL, Version.V20, "modelVersion", null,
+                          "of '" + string + "' is older than the versions supported by this version of Maven: " + values
+                              + ". Building this project requires an older version of Maven.", tracker );
+
+        }
+        else
+        {
+            addViolation( problems, Severity.ERROR, Version.V20, "modelVersion", null,
+                          "must be one of " + values + " but is '" + string + "'.", tracker );
+        }
+
+        return false;
+    }
+
+    /**
+     * Compares two model versions.
+     *
+     * @param first the first version.
+     * @param second the second version.
+     * @return negative if the first version is newer than the second version, zero if they are the same or positive if
+     * the second version is the newer.
+     */
+    private static int compareModelVersions( String first, String second )
+    {
+        // we use a dedicated comparator because we control our model version scheme.
+        String[] firstSegments = StringUtils.split( first, "." );
+        String[] secondSegments = StringUtils.split( second, "." );
+        for ( int i = 0; i < Math.min( firstSegments.length, secondSegments.length ); i++ )
+        {
+            int result = Long.valueOf( firstSegments[i] ).compareTo( Long.valueOf( secondSegments[i] ) );
+            if ( result != 0 )
+            {
+                return result;
+            }
+        }
+        if ( firstSegments.length == secondSegments.length )
+        {
+            return 0;
+        }
+        return firstSegments.length > secondSegments.length ? -1 : 1;
+    }
+
+    @SuppressWarnings( "checkstyle:parameternumber" )
     private boolean validateBannedCharacters( String fieldName, ModelProblemCollector problems, Severity severity,
                                               Version version, String string, String sourceHint,
                                               InputLocationTracker tracker, String banned )
diff --git a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
index a9d4c00..9d5f172 100644
--- a/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
+++ b/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java
@@ -129,9 +129,9 @@ public class DefaultModelValidatorTest
         SimpleProblemCollector result =
             validateRaw( "bad-modelVersion.xml", ModelBuildingRequest.VALIDATION_LEVEL_STRICT );
 
-        assertViolations( result, 0, 1, 0 );
+        assertViolations( result, 1, 0, 0 );
 
-        assertTrue( result.getErrors().get( 0 ).contains( "modelVersion" ) );
+        assertTrue( result.getFatals().get( 0 ).contains( "modelVersion" ) );
     }
 
     public void testMissingArtifactId()