You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Christian Schulte <cs...@schulte.it> on 2016/08/13 22:01:29 UTC

Re: maven git commit: [MNG-5971] Imported dependencies should be available to inheritance processing

Let's just ask for another preliminary snapshot testing with this commit
applied, please. Especially ask for users making use of the 'import'
scope heavily to test the snapshot. If curious, just download the
'bom-cloud.zip' attachment from MNG-5971 and run 'mvn verify' with a
recent snapshot. The warnings Maven then prints are real POM issues.
Let's see how many projects are affected by this. It does not change
behaviour. It just prints a warning.

Regards,
-- 
Christian

Am 08/13/16 um 23:47 schrieb schulte@apache.org:
> Repository: maven
> Updated Branches:
>   refs/heads/master 814b51661 -> 059b9ab30
> 
> 
> [MNG-5971] Imported dependencies should be available to inheritance processing
> 
> o Updated to warn about conflicting imports in Maven 3.4 and to add the next
>   validation level for Maven 3.5 to the API. This will warn about projects
>   relying on dependency management import conflict resolution based on the
>   order of XML element declarations. Those conflicts can be resolved by adding
>   the conflicting dependencies directly to the dependency management. As of
>   Maven 3.4, inheritance logic can be used to override things as needed. In the
>   release notes, we may need to give an example on how to update a project to be
>   compatible with all Maven versions >= 2.0.9. For example: The advertised BOM
>   users are asked to use may need to be updated to be composed from multiple
>   parent POMs internally. Transparent to the users and compatible with all
>   Maven versions since 2.0.9.
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/maven/repo
> Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/059b9ab3
> Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/059b9ab3
> Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/059b9ab3
> 
> Branch: refs/heads/master
> Commit: 059b9ab3028360b011c1539caa4c40d033b9143f
> Parents: 814b516
> Author: Christian Schulte <sc...@apache.org>
> Authored: Sat Aug 13 22:58:43 2016 +0200
> Committer: Christian Schulte <sc...@apache.org>
> Committed: Sat Aug 13 23:24:20 2016 +0200
> 
> ----------------------------------------------------------------------
>  .../model/building/ModelBuildingRequest.java    |  15 +-
>  .../maven/model/building/ModelProblem.java      |   8 +-
>  .../DefaultDependencyManagementImporter.java    | 191 ++++++++++++++++++-
>  3 files changed, 199 insertions(+), 15 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/maven/blob/059b9ab3/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java
> ----------------------------------------------------------------------
> diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java
> index ac0f8b8..bc6d85d 100644
> --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java
> +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelBuildingRequest.java
> @@ -23,7 +23,6 @@ import java.io.File;
>  import java.util.Date;
>  import java.util.List;
>  import java.util.Properties;
> -
>  import org.apache.maven.model.Model;
>  import org.apache.maven.model.Profile;
>  import org.apache.maven.model.resolution.ModelResolver;
> @@ -55,19 +54,25 @@ public interface ModelBuildingRequest
>      int VALIDATION_LEVEL_MAVEN_3_0 = 30;
>  
>      /**
> -     * Denotes validation as performed by Maven 3.1. This validation level is meant for new projects. As of Maven 3.4,
> -     * this is the default validation level.
> +     * Denotes validation as performed by Maven 3.1. As of Maven 3.4, this is the default validation level.
>       */
>      int VALIDATION_LEVEL_MAVEN_3_1 = 31;
>  
>      /**
> +     * Denotes validation as performed by Maven 3.5. As of Maven 3.5, this is the default validation level.
> +     *
> +     * @since 3.4
> +     */
> +    int VALIDATION_LEVEL_MAVEN_3_5 = 35;
> +
> +    /**
>       * Denotes strict validation as recommended by the current Maven version.
>       */
>      int VALIDATION_LEVEL_STRICT = VALIDATION_LEVEL_MAVEN_3_1;
>  
>      /**
>       * Gets the raw model to build. If not set, model source will be used to load raw model.
> -     * 
> +     *
>       * @return The raw model to build or {@code null} if not set.
>       */
>      Model getRawModel();
> @@ -334,4 +339,4 @@ public interface ModelBuildingRequest
>  
>      ModelBuildingRequest setWorkspaceModelResolver( WorkspaceModelResolver workspaceResolver );
>  
> -}
> \ No newline at end of file
> +}
> 
> http://git-wip-us.apache.org/repos/asf/maven/blob/059b9ab3/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelProblem.java
> ----------------------------------------------------------------------
> diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelProblem.java b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelProblem.java
> index 43272ba..fa12a94 100644
> --- a/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelProblem.java
> +++ b/maven-model-builder/src/main/java/org/apache/maven/model/building/ModelProblem.java
> @@ -43,11 +43,17 @@ public interface ModelProblem
>  
>      enum Version
>      {
> +
>          //based on ModeBuildingResult.validationLevel
>          BASE,
>          V20,
>          V30,
> -        V31
> +        V31,
> +        /**
> +         * @since 3.4
> +         */
> +        V35
> +
>      }
>  
>      /**
> 
> http://git-wip-us.apache.org/repos/asf/maven/blob/059b9ab3/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java
> ----------------------------------------------------------------------
> diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java
> index 6c361d3..4072c24 100644
> --- a/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java
> +++ b/maven-model-builder/src/main/java/org/apache/maven/model/composition/DefaultDependencyManagementImporter.java
> @@ -19,15 +19,19 @@ package org.apache.maven.model.composition;
>   * under the License.
>   */
>  
> +import java.util.ArrayList;
> +import java.util.Iterator;
>  import java.util.LinkedHashMap;
>  import java.util.List;
>  import java.util.Map;
> -
>  import org.apache.maven.model.Dependency;
>  import org.apache.maven.model.DependencyManagement;
> +import org.apache.maven.model.Exclusion;
>  import org.apache.maven.model.Model;
>  import org.apache.maven.model.building.ModelBuildingRequest;
> +import org.apache.maven.model.building.ModelProblem;
>  import org.apache.maven.model.building.ModelProblemCollector;
> +import org.apache.maven.model.building.ModelProblemCollectorRequest;
>  import org.codehaus.plexus.component.annotations.Component;
>  
>  /**
> @@ -47,28 +51,197 @@ public class DefaultDependencyManagementImporter
>          if ( sources != null && !sources.isEmpty() )
>          {
>              final Map<String, Dependency> targetDependencies = new LinkedHashMap<>();
> +            final DependencyManagement targetDependencyManagement = target.getDependencyManagement() != null
> +                                                                        ? target.getDependencyManagement()
> +                                                                        : new DependencyManagement();
> +
> +            target.setDependencyManagement( targetDependencyManagement );
>  
> -            if ( target.getDependencyManagement() != null )
> +            for ( final Dependency targetDependency : targetDependencyManagement.getDependencies() )
>              {
> -                for ( final Dependency targetDependency : target.getDependencyManagement().getDependencies() )
> -                {
> -                    targetDependencies.put( targetDependency.getManagementKey(), targetDependency );
> -                }
> +                targetDependencies.put( targetDependency.getManagementKey(), targetDependency );
>              }
>  
> +            final Map<String, List<Dependency>> sourceDependencies = new LinkedHashMap<>();
> +
>              for ( final DependencyManagement source : sources )
>              {
>                  for ( final Dependency sourceDependency : source.getDependencies() )
>                  {
>                      if ( !targetDependencies.containsKey( sourceDependency.getManagementKey() ) )
>                      {
> -                        targetDependencies.put( sourceDependency.getManagementKey(), sourceDependency );
> +                        List<Dependency> conflictCanditates =
> +                            sourceDependencies.get( sourceDependency.getManagementKey() );
> +
> +                        if ( conflictCanditates == null )
> +                        {
> +                            conflictCanditates = new ArrayList<>( source.getDependencies().size() );
> +                            sourceDependencies.put( sourceDependency.getManagementKey(), conflictCanditates );
> +                        }
> +
> +                        conflictCanditates.add( sourceDependency );
>                      }
>                  }
>              }
>  
> -            target.setDependencyManagement( new DependencyManagement() );
> -            target.getDependencyManagement().getDependencies().addAll( targetDependencies.values() );
> +            for ( final List<Dependency> conflictCanditates : sourceDependencies.values() )
> +            {
> +                final List<Dependency> conflictingDependencies = removeRedundantDependencies( conflictCanditates );
> +
> +                // First declaration wins. This is what makes the conflict resolution indeterministic because this
> +                // solely relies on the order of declaration. There is no such thing as the "first" declaration.
> +                targetDependencyManagement.getDependencies().add( conflictingDependencies.get( 0 ) );
> +
> +                // As of Maven 3.4, we print a warning about such conflicting imports.
> +                // As of Maven 3.5, those warnings will become errors.
> +                if ( conflictingDependencies.size() > 1 )
> +                {
> +                    final StringBuilder conflictsBuilder = new StringBuilder( conflictingDependencies.size() * 128 );
> +
> +                    for ( final Dependency dependency : conflictingDependencies )
> +                    {
> +                        conflictsBuilder.append( ", '" ).append( dependency.getLocation( "" ) ).append( '\'' );
> +                    }
> +
> +                    problems.add( new ModelProblemCollectorRequest(
> +                        effectiveSeverity( request.getValidationLevel(),
> +                                           ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_5 ),
> +                        ModelProblem.Version.V35 ).
> +                        setMessage( String.format(
> +                                "Multiple conflicting imports of dependency '%1$s' into model '%2$s'%3$s(%4$s). "
> +                                    + "To resolve this conflict, either declare the dependency directly "
> +                                    + "in the dependency management of model '%2$s' to override what gets imported "
> +                                    + "or, as of Maven 3.4, rearrange the causing imports in the inheritance hierarchy "
> +                                    + "to apply standard override logic based on artifact coordinates. "
> +                                    + "Without resolving this conflict, your build relies on indeterministic "
> +                                    + "behaviour.",
> +                                conflictingDependencies.get( 0 ).getManagementKey(), target.getId(),
> +                                target.getPomFile() != null
> +                                    ? " @ '" + target.getPomFile().getAbsolutePath() + "' "
> +                                    : " ", conflictsBuilder.substring( 2 ) ) ) );
> +
> +                }
> +            }
> +        }
> +    }
> +
> +    private static List<Dependency> removeRedundantDependencies( final List<Dependency> candidateDependencies )
> +    {
> +        final List<Dependency> resultDependencies = new ArrayList<>( candidateDependencies.size() );
> +
> +        while ( !candidateDependencies.isEmpty() )
> +        {
> +            final Dependency resultDependency = candidateDependencies.remove( 0 );
> +            resultDependencies.add( resultDependency );
> +
> +            // Removes redundant dependencies.
> +            for ( final Iterator<Dependency> it = candidateDependencies.iterator(); it.hasNext(); )
> +            {
> +                final Dependency candidateDependency = it.next();
> +                boolean redundant = true;
> +
> +                redundancy_check:
> +                {
> +                    if ( !( resultDependency.getOptional() != null
> +                            ? resultDependency.getOptional().equals( candidateDependency.getOptional() )
> +                            : candidateDependency.getOptional() == null ) )
> +                    {
> +                        redundant = false;
> +                        break redundancy_check;
> +                    }
> +
> +                    if ( !( effectiveScope( resultDependency ).equals( effectiveScope( candidateDependency ) ) ) )
> +                    {
> +                        redundant = false;
> +                        break redundancy_check;
> +                    }
> +
> +                    if ( !( resultDependency.getSystemPath() != null
> +                            ? resultDependency.getSystemPath().equals( candidateDependency.getSystemPath() )
> +                            : candidateDependency.getSystemPath() == null ) )
> +                    {
> +                        redundant = false;
> +                        break redundancy_check;
> +                    }
> +
> +                    if ( !( resultDependency.getVersion() != null
> +                            ? resultDependency.getVersion().equals( candidateDependency.getVersion() )
> +                            : candidateDependency.getVersion() == null ) )
> +                    {
> +                        redundant = false;
> +                        break redundancy_check;
> +                    }
> +
> +                    for ( int i = 0, s0 = resultDependency.getExclusions().size(); i < s0; i++ )
> +                    {
> +                        final Exclusion resultExclusion = resultDependency.getExclusions().get( i );
> +
> +                        if ( !containsExclusion( candidateDependency.getExclusions(), resultExclusion ) )
> +                        {
> +                            redundant = false;
> +                            break redundancy_check;
> +                        }
> +                    }
> +
> +                    for ( int i = 0, s0 = candidateDependency.getExclusions().size(); i < s0; i++ )
> +                    {
> +                        final Exclusion candidateExclusion = candidateDependency.getExclusions().get( i );
> +
> +                        if ( !containsExclusion( resultDependency.getExclusions(), candidateExclusion ) )
> +                        {
> +                            redundant = false;
> +                            break redundancy_check;
> +                        }
> +                    }
> +                }
> +
> +                if ( redundant )
> +                {
> +                    it.remove();
> +                }
> +            }
> +        }
> +
> +        return resultDependencies;
> +    }
> +
> +    private static boolean containsExclusion( final List<Exclusion> exclusions, final Exclusion exclusion )
> +    {
> +        for ( int i = 0, s0 = exclusions.size(); i < s0; i++ )
> +        {
> +            final Exclusion current = exclusions.get( i );
> +
> +            if ( ( exclusion.getArtifactId() != null
> +                   ? exclusion.getArtifactId().equals( current.getArtifactId() )
> +                   : current.getArtifactId() == null )
> +                     && ( exclusion.getGroupId() != null
> +                          ? exclusion.getGroupId().equals( current.getGroupId() )
> +                          : current.getGroupId() == null ) )
> +            {
> +                return true;
> +            }
> +        }
> +
> +        return false;
> +    }
> +
> +    private static String effectiveScope( final Dependency dependency )
> +    {
> +        return dependency.getScope() == null
> +                   ? "compile"
> +                   : dependency.getScope();
> +
> +    }
> +
> +    private static ModelProblem.Severity effectiveSeverity( final int validationLevel, final int errorThreshold )
> +    {
> +        if ( validationLevel < errorThreshold )
> +        {
> +            return ModelProblem.Severity.WARNING;
> +        }
> +        else
> +        {
> +            return ModelProblem.Severity.ERROR;
>          }
>      }
>  
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org