[maven] branch MNG-6135 created (now 30d88e5)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[maven] branch MNG-6135 created (now 30d88e5)

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

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


      at 30d88e5  [MNG-6135] Maven plugins and core extensions are not dependencies, they should be resolved the same way as projects.

This branch includes the following new commits:

     new 30d88e5  [MNG-6135] Maven plugins and core extensions are not dependencies, they should be resolved the same way as projects.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Reply | Threaded
Open this post in threaded view
|

[maven] 01/01: [MNG-6135] Maven plugins and core extensions are not dependencies, they should be resolved the same way as projects.

michaelo
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 <[hidden email]>
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 )