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

michaelo pushed a commit to branch MNG-6135
in repository https://gitbox.apache.org/repos/asf/maven.git

commit 30d88e5d8e085b794d06dbeb3a7fd383923908d4
Author: Christian Schulte <[email protected]>
AuthorDate: Sun Oct 22 06:27:47 2017 +0200

    [MNG-6135] Maven plugins and core extensions are not dependencies, they 
should be resolved the same way as projects.
---
 .../java/org/apache/maven/RepositoryUtils.java     |  12 +-
 .../DefaultPluginDependenciesResolver.java         | 250 ++++++++++++++++-----
 .../maven/plugin/internal/WagonExcluder.java       |  14 +-
 3 files changed, 211 insertions(+), 65 deletions(-)

diff --git a/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java 
b/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java
index c1e21c4..4723eb1 100644
--- a/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java
+++ b/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java
@@ -119,12 +119,16 @@ public class RepositoryUtils
 
             List<String> nodeTrail = new ArrayList<>( trail.size() + 1 );
             nodeTrail.addAll( trail );
-            nodeTrail.add( artifact.getId() );
 
-            if ( filter == null || filter.accept( node, 
Collections.<DependencyNode>emptyList() ) )
+            if ( artifact != null )
             {
-                artifact.setDependencyTrail( nodeTrail );
-                artifacts.add( artifact );
+                nodeTrail.add( artifact.getId() );
+
+                if ( filter == null || filter.accept( node, 
Collections.<DependencyNode>emptyList() ) )
+                {
+                    artifact.setDependencyTrail( nodeTrail );
+                    artifacts.add( artifact );
+                }
             }
 
             toArtifacts( artifacts, node.getChildren(), nodeTrail, filter );
diff --git 
a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
 
b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
index 709bd72..4473270 100644
--- 
a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
+++ 
b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
@@ -30,6 +30,7 @@ import javax.inject.Named;
 import javax.inject.Singleton;
 
 import org.apache.maven.RepositoryUtils;
+import org.apache.maven.artifact.versioning.ComparableVersion;
 import org.apache.maven.model.Dependency;
 import org.apache.maven.model.Plugin;
 import org.apache.maven.plugin.PluginResolutionException;
@@ -42,6 +43,7 @@ import org.eclipse.aether.RequestTrace;
 import org.eclipse.aether.artifact.Artifact;
 import org.eclipse.aether.artifact.DefaultArtifact;
 import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionContext;
 import org.eclipse.aether.collection.DependencyCollectionException;
 import org.eclipse.aether.collection.DependencySelector;
 import org.eclipse.aether.graph.DependencyFilter;
@@ -58,8 +60,14 @@ import 
org.eclipse.aether.resolution.DependencyResolutionException;
 import org.eclipse.aether.util.artifact.JavaScopes;
 import org.eclipse.aether.util.filter.AndDependencyFilter;
 import org.eclipse.aether.util.filter.ScopeDependencyFilter;
+import org.eclipse.aether.util.graph.manager.ClassicDependencyManager;
 import org.eclipse.aether.util.graph.manager.DependencyManagerUtils;
+import org.eclipse.aether.util.graph.manager.TransitiveDependencyManager;
 import org.eclipse.aether.util.graph.selector.AndDependencySelector;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
+import org.eclipse.aether.util.graph.selector.OptionalDependencySelector;
+import 
org.eclipse.aether.util.graph.transformer.ChainedDependencyGraphTransformer;
+import org.eclipse.aether.collection.DependencyGraphTransformer;
 import org.eclipse.aether.util.repository.SimpleArtifactDescriptorPolicy;
 
 /**
@@ -78,6 +86,10 @@ public class DefaultPluginDependenciesResolver
 
     private static final String REPOSITORY_CONTEXT = "plugin";
 
+    private static final String DEFAULT_PREREQUISITES = "2";
+
+    private static final ComparableVersion DEFAULT_RESULTION_PREREQUISITES = 
new ComparableVersion( "3" );
+
     @Inject
     private Logger logger;
 
@@ -90,50 +102,21 @@ public class DefaultPluginDependenciesResolver
                                     session.getArtifactTypeRegistry().get( 
"maven-plugin" ) );
     }
 
+    @Override
     public Artifact resolve( Plugin plugin, List<RemoteRepository> 
repositories, RepositorySystemSession session )
         throws PluginResolutionException
     {
-        RequestTrace trace = RequestTrace.newChild( null, plugin );
-
-        Artifact pluginArtifact = toArtifact( plugin, session );
-
         try
         {
-            DefaultRepositorySystemSession pluginSession = new 
DefaultRepositorySystemSession( session );
-            pluginSession.setArtifactDescriptorPolicy( new 
SimpleArtifactDescriptorPolicy( true, false ) );
-
-            ArtifactDescriptorRequest request =
-                new ArtifactDescriptorRequest( pluginArtifact, repositories, 
REPOSITORY_CONTEXT );
-            request.setTrace( trace );
-            ArtifactDescriptorResult result = 
repoSystem.readArtifactDescriptor( pluginSession, request );
-
-            pluginArtifact = result.getArtifact();
-
-            String requiredMavenVersion = (String) result.getProperties().get( 
"prerequisites.maven" );
-            if ( requiredMavenVersion != null )
-            {
-                Map<String, String> props = new LinkedHashMap<>( 
pluginArtifact.getProperties() );
-                props.put( "requiredMavenVersion", requiredMavenVersion );
-                pluginArtifact = pluginArtifact.setProperties( props );
-            }
+            final Artifact pluginArtifact = this.createPluginArtifact( plugin, 
session, repositories );
+            final ArtifactRequest request = new ArtifactRequest( 
pluginArtifact, repositories, REPOSITORY_CONTEXT );
+            request.setTrace( RequestTrace.newChild( null, plugin ) );
+            return this.repoSystem.resolveArtifact( session, request 
).getArtifact();
         }
-        catch ( ArtifactDescriptorException e )
+        catch ( ArtifactDescriptorException | ArtifactResolutionException e )
         {
             throw new PluginResolutionException( plugin, e );
         }
-
-        try
-        {
-            ArtifactRequest request = new ArtifactRequest( pluginArtifact, 
repositories, REPOSITORY_CONTEXT );
-            request.setTrace( trace );
-            pluginArtifact = repoSystem.resolveArtifact( session, request 
).getArtifact();
-        }
-        catch ( ArtifactResolutionException e )
-        {
-            throw new PluginResolutionException( plugin, e );
-        }
-
-        return pluginArtifact;
     }
 
     /**
@@ -155,52 +138,167 @@ public class DefaultPluginDependenciesResolver
                                 session );
     }
 
-    private DependencyNode resolveInternal( Plugin plugin, Artifact 
pluginArtifact, DependencyFilter dependencyFilter,
+    private DependencyNode resolveInternal( Plugin plugin, Artifact artifact, 
DependencyFilter dependencyFilter,
                                             List<RemoteRepository> 
repositories, RepositorySystemSession session )
         throws PluginResolutionException
     {
-        RequestTrace trace = RequestTrace.newChild( null, plugin );
-
-        if ( pluginArtifact == null )
+        // This dependency selector matches the resolver's implementation 
before MRESOLVER-8 got fixed. It is
+        // used for plugin's with prerequisites < 3.6 to mimic incorrect but 
backwards compatible behaviour.
+        class ClassicScopeDependencySelector implements DependencySelector
         {
-            pluginArtifact = toArtifact( plugin, session );
+
+            private final boolean transitive;
+
+            ClassicScopeDependencySelector()
+            {
+                this( false );
+            }
+
+            private ClassicScopeDependencySelector( final boolean transitive )
+            {
+                super();
+                this.transitive = transitive;
+            }
+
+            @Override
+            public boolean selectDependency( final 
org.eclipse.aether.graph.Dependency dependency )
+            {
+                return !this.transitive
+                           || !( "test".equals( dependency.getScope() )
+                                 || "provided".equals( dependency.getScope() ) 
);
+
+            }
+
+            @Override
+            public DependencySelector deriveChildSelector( final 
DependencyCollectionContext context )
+            {
+                ClassicScopeDependencySelector child = this;
+
+                if ( context.getDependency() != null && !child.transitive )
+                {
+                    child = new ClassicScopeDependencySelector( true );
+                }
+                if ( context.getDependency() == null && child.transitive )
+                {
+                    child = new ClassicScopeDependencySelector( false );
+                }
+
+                return child;
+            }
+
+            @Override
+            public boolean equals( Object obj )
+            {
+                boolean equal = obj instanceof ClassicScopeDependencySelector;
+
+                if ( equal )
+                {
+                    final ClassicScopeDependencySelector that = 
(ClassicScopeDependencySelector) obj;
+                    equal = this.transitive == that.transitive;
+                }
+
+                return equal;
+            }
+
+            @Override
+            public int hashCode()
+            {
+                int hash = 17;
+                hash = hash * 31 + ( ( (Boolean) this.transitive ).hashCode() 
);
+                return hash;
+            }
+
         }
 
-        DependencyFilter collectionFilter = new ScopeDependencyFilter( 
"provided", "test" );
-        DependencyFilter resolutionFilter = AndDependencyFilter.newInstance( 
collectionFilter, dependencyFilter );
+        final RepositorySystemSession verboseSession;
+        if ( this.logger.isDebugEnabled()
+                 && session.getConfigProperties().get( 
DependencyManagerUtils.CONFIG_PROP_VERBOSE ) == null )
+        {
+            final DefaultRepositorySystemSession defaultSession = new 
DefaultRepositorySystemSession( session );
+            defaultSession.setConfigProperty( 
DependencyManagerUtils.CONFIG_PROP_VERBOSE, Boolean.TRUE );
+            verboseSession = defaultSession;
+        }
+        else
+        {
+            verboseSession = session;
+        }
 
-        DependencyNode node;
+        final RequestTrace trace = RequestTrace.newChild( null, plugin );
+        final DependencyFilter collectionFilter = new ScopeDependencyFilter( 
"provided", "test" );
+        final DependencyFilter resolutionFilter = 
AndDependencyFilter.newInstance( collectionFilter, dependencyFilter );
 
         try
         {
-            DependencySelector selector =
-                AndDependencySelector.newInstance( 
session.getDependencySelector(), new WagonExcluder() );
+            final Artifact pluginArtifact = artifact != null
+                                                ? this.createPluginArtifact( 
artifact, verboseSession, repositories )
+                                                : this.createPluginArtifact( 
plugin, verboseSession, repositories );
+
+            final ComparableVersion prerequisites =
+                new ComparableVersion( pluginArtifact.getProperty( 
"requiredMavenVersion", DEFAULT_PREREQUISITES ) );
 
-            DefaultRepositorySystemSession pluginSession = new 
DefaultRepositorySystemSession( session );
-            pluginSession.setDependencySelector( selector );
-            pluginSession.setDependencyGraphTransformer( 
session.getDependencyGraphTransformer() );
+            final boolean classicResolution = prerequisites.compareTo( 
DEFAULT_RESULTION_PREREQUISITES ) < 0;
+
+            if ( this.logger.isDebugEnabled() )
+            {
+                if ( classicResolution )
+                {
+                    this.logger.debug( String.format(
+                        "Constructing classic plugin classpath '%s' for 
prerequisites '%s'.",
+                        pluginArtifact, prerequisites ) );
+
+                }
+                else
+                {
+                    this.logger.debug( String.format(
+                        "Constructing default plugin classpath '%s' for 
prerequisites '%s'.",
+                        pluginArtifact, prerequisites ) );
+
+                }
+            }
+
+            final DependencySelector pluginDependencySelector =
+                classicResolution
+                    ? new AndDependencySelector( new 
ClassicScopeDependencySelector(), // incorrect - see MRESOLVER-8
+                                                 new 
OptionalDependencySelector(),
+                                                 new 
ExclusionDependencySelector(),
+                                                 new WagonExcluder() )
+                    : AndDependencySelector.newInstance( 
verboseSession.getDependencySelector(), new WagonExcluder() );
+
+            final DependencyGraphTransformer pluginDependencyGraphTransformer =
+                ChainedDependencyGraphTransformer.newInstance( 
verboseSession.getDependencyGraphTransformer(), null );
+
+            DefaultRepositorySystemSession pluginSession = new 
DefaultRepositorySystemSession( verboseSession );
+            pluginSession.setDependencySelector( pluginDependencySelector );
+            pluginSession.setDependencyGraphTransformer( 
pluginDependencyGraphTransformer );
+            pluginSession.setDependencyManager( classicResolution
+                                                    ? new 
ClassicDependencyManager()
+                                                    : new 
TransitiveDependencyManager() );
 
             CollectRequest request = new CollectRequest();
             request.setRequestContext( REPOSITORY_CONTEXT );
             request.setRepositories( repositories );
-            request.setRoot( new org.eclipse.aether.graph.Dependency( 
pluginArtifact, null ) );
+
             for ( Dependency dependency : plugin.getDependencies() )
             {
                 org.eclipse.aether.graph.Dependency pluginDep =
-                    RepositoryUtils.toDependency( dependency, 
session.getArtifactTypeRegistry() );
+                    RepositoryUtils.toDependency( dependency, 
verboseSession.getArtifactTypeRegistry() );
+
                 if ( !JavaScopes.SYSTEM.equals( pluginDep.getScope() ) )
                 {
                     pluginDep = pluginDep.setScope( JavaScopes.RUNTIME );
                 }
+
                 request.addDependency( pluginDep );
             }
 
+            request.setRoot( new org.eclipse.aether.graph.Dependency( 
pluginArtifact, null ) );
+
             DependencyRequest depRequest = new DependencyRequest( request, 
resolutionFilter );
             depRequest.setTrace( trace );
 
             request.setTrace( RequestTrace.newChild( trace, depRequest ) );
 
-            node = repoSystem.collectDependencies( pluginSession, request 
).getRoot();
+            final DependencyNode node = repoSystem.collectDependencies( 
pluginSession, request ).getRoot();
 
             if ( logger.isDebugEnabled() )
             {
@@ -208,9 +306,10 @@ public class DefaultPluginDependenciesResolver
             }
 
             depRequest.setRoot( node );
-            repoSystem.resolveDependencies( session, depRequest );
+            repoSystem.resolveDependencies( verboseSession, depRequest );
+            return node;
         }
-        catch ( DependencyCollectionException e )
+        catch ( ArtifactDescriptorException | DependencyCollectionException e )
         {
             throw new PluginResolutionException( plugin, e );
         }
@@ -218,8 +317,44 @@ public class DefaultPluginDependenciesResolver
         {
             throw new PluginResolutionException( plugin, e.getCause() );
         }
+    }
+
+    private Artifact createPluginArtifact( final Plugin plugin,
+                                           final RepositorySystemSession 
session,
+                                           final List<RemoteRepository> 
repositories )
+        throws ArtifactDescriptorException
+    {
+        return this.createPluginArtifact( toArtifact( plugin, session ), 
session, repositories );
+    }
+
+    private Artifact createPluginArtifact( final Artifact artifact,
+                                           final RepositorySystemSession 
session,
+                                           final List<RemoteRepository> 
repositories )
+        throws ArtifactDescriptorException
+    {
+        Artifact pluginArtifact = artifact;
+        final DefaultRepositorySystemSession pluginSession = new 
DefaultRepositorySystemSession( session );
+        pluginSession.setArtifactDescriptorPolicy( new 
SimpleArtifactDescriptorPolicy( true, false ) );
+
+        final ArtifactDescriptorRequest request =
+            new ArtifactDescriptorRequest( pluginArtifact, repositories, 
REPOSITORY_CONTEXT );
+
+        request.setTrace( RequestTrace.newChild( null, artifact ) );
+
+        final ArtifactDescriptorResult result = 
this.repoSystem.readArtifactDescriptor( pluginSession, request );
+
+        pluginArtifact = result.getArtifact();
+
+        final String requiredMavenVersion = (String) 
result.getProperties().get( "prerequisites.maven" );
+
+        if ( requiredMavenVersion != null )
+        {
+            final Map<String, String> props = new LinkedHashMap<>( 
pluginArtifact.getProperties() );
+            props.put( "requiredMavenVersion", requiredMavenVersion );
+            pluginArtifact = pluginArtifact.setProperties( props );
+        }
 
-        return node;
+        return pluginArtifact;
     }
 
     // Keep this class in sync with 
org.apache.maven.project.DefaultProjectDependenciesResolver.GraphLogger
@@ -229,6 +364,12 @@ public class DefaultPluginDependenciesResolver
 
         private String indent = "";
 
+        GraphLogger()
+        {
+            super();
+        }
+
+        @Override
         public boolean visitEnter( DependencyNode node )
         {
             StringBuilder buffer = new StringBuilder( 128 );
@@ -304,6 +445,7 @@ public class DefaultPluginDependenciesResolver
             return true;
         }
 
+        @Override
         public boolean visitLeave( DependencyNode node )
         {
             indent = indent.substring( 0, indent.length() - 3 );
diff --git 
a/maven-core/src/main/java/org/apache/maven/plugin/internal/WagonExcluder.java 
b/maven-core/src/main/java/org/apache/maven/plugin/internal/WagonExcluder.java
index d374cab..e003ede 100644
--- 
a/maven-core/src/main/java/org/apache/maven/plugin/internal/WagonExcluder.java
+++ 
b/maven-core/src/main/java/org/apache/maven/plugin/internal/WagonExcluder.java
@@ -51,19 +51,19 @@ class WagonExcluder
 
     public boolean selectDependency( Dependency dependency )
     {
-        return !coreArtifact || !isWagonProvider( dependency.getArtifact() );
+        return !( coreArtifact && isWagonProvider( dependency.getArtifact() ) 
);
     }
 
     public DependencySelector deriveChildSelector( DependencyCollectionContext 
context )
     {
-        if ( coreArtifact || !isLegacyCoreArtifact( 
context.getDependency().getArtifact() ) )
-        {
-            return this;
-        }
-        else
+        WagonExcluder child = this;
+
+        if ( isLegacyCoreArtifact( context.getArtifact() ) && 
!this.coreArtifact )
         {
-            return new WagonExcluder( true );
+            child = new WagonExcluder( true );
         }
+
+        return child;
     }
 
     private boolean isLegacyCoreArtifact( Artifact artifact )

Reply via email to