[MNG-6135] Maven plugins and core extensions are not dependencies, they should be resolved the same way as projects.
o Updated to use the '<prerequisites>' element to decide the style of resolution as discussed on dev@. Plugins with prerequisites >= 3.6 get the correct resolution, all other plugins will be resolved with direct 'test' and 'provided' dependencies ignored but direct 'optional' dependencies. o During working on MNG-6135, it turned out that plugin dependency management also is applied to the dependencies declared for a plugin using the 'plugins/plugin/dependencies/dependency' element. Dependencies declared that way must not be overridden by any management. I added management for 'plexus-utils' in the core IT suite. This immediately made the ITs for MNG-4276 and MNG-4274 fail, because 'plexus-utils' got subject to management although a different version has been declared in the plugin declaration. Project: http://git-wip-us.apache.org/repos/asf/maven/repo Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/f1a1fc6d Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/f1a1fc6d Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/f1a1fc6d Branch: refs/heads/DEPMGMT Commit: f1a1fc6dc27191882656057c5737c9fb81bbab21 Parents: 9d4f54a Author: Christian Schulte <schu...@apache.org> Authored: Sat Dec 17 03:07:04 2016 +0100 Committer: Christian Schulte <schu...@apache.org> Committed: Sun Feb 5 00:08:37 2017 +0100 ---------------------------------------------------------------------- .../java/org/apache/maven/RepositoryUtils.java | 12 +- .../DefaultPluginDependenciesResolver.java | 370 ++++++++++++++++--- .../plugin/internal/PlexusUtilsInjector.java | 30 +- .../maven/plugin/internal/WagonExcluder.java | 14 +- 4 files changed, 344 insertions(+), 82 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven/blob/f1a1fc6d/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java ---------------------------------------------------------------------- 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 52442b7..08749ea 100644 --- a/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java +++ b/maven-core/src/main/java/org/apache/maven/RepositoryUtils.java @@ -114,12 +114,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 ); http://git-wip-us.apache.org/repos/asf/maven/blob/f1a1fc6d/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java ---------------------------------------------------------------------- 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 b79b15f..80fd80d 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 @@ -20,10 +20,11 @@ package org.apache.maven.plugin.internal; */ import java.util.LinkedHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; - 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; @@ -37,8 +38,11 @@ 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.DependencyGraphTransformer; +import org.eclipse.aether.collection.DependencyManagement; +import org.eclipse.aether.collection.DependencyManager; import org.eclipse.aether.collection.DependencySelector; import org.eclipse.aether.graph.DependencyFilter; import org.eclipse.aether.graph.DependencyNode; @@ -54,8 +58,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.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.util.graph.visitor.FilteringDependencyVisitor; +import org.eclipse.aether.util.graph.visitor.PreorderNodeListGenerator; import org.eclipse.aether.util.repository.SimpleArtifactDescriptorPolicy; /** @@ -73,6 +83,10 @@ public class DefaultPluginDependenciesResolver private static final String REPOSITORY_CONTEXT = "plugin"; + private static final String DEFAULT_PREREQUISITES = "2.0"; + + private static final ComparableVersion DEFAULT_RESULTION_PREREQUISITES = new ComparableVersion( "3.6" ); + @Requirement private Logger logger; @@ -85,50 +99,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 ); - } - } - catch ( ArtifactDescriptorException e ) - { - throw new PluginResolutionException( plugin, e ); - } - try { - ArtifactRequest request = new ArtifactRequest( pluginArtifact, repositories, REPOSITORY_CONTEXT ); - request.setTrace( trace ); - pluginArtifact = repoSystem.resolveArtifact( session, request ).getArtifact(); + 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 ( ArtifactResolutionException e ) + catch ( ArtifactDescriptorException | ArtifactResolutionException e ) { throw new PluginResolutionException( plugin, e ); } - - return pluginArtifact; } /** @@ -150,56 +135,237 @@ public class DefaultPluginDependenciesResolver session ); } - private DependencyNode resolveInternal( Plugin plugin, Artifact pluginArtifact, DependencyFilter dependencyFilter, - DependencyGraphTransformer transformer, - List<RemoteRepository> repositories, RepositorySystemSession session ) + private DependencyNode resolveInternal( final Plugin plugin, final Artifact artifact, + final DependencyFilter dependencyFilter, + final DependencyGraphTransformer transformer, + final List<RemoteRepository> repositories, + final RepositorySystemSession session ) throws PluginResolutionException { - RequestTrace trace = RequestTrace.newChild( null, plugin ); - - if ( pluginArtifact == null ) + // This dependency manager delegates to the session's dependency manager but supports excluding plugin + // dependency overrides from the plugins/plugin/dependencies POM element so that what is declared there will + // not get changed due to any management performed. + class PluginDependencyManager implements DependencyManager { - pluginArtifact = toArtifact( plugin, session ); + + private final int depth; + + private final DependencyManager delegate; + + private final List<Artifact> exclusions; + + PluginDependencyManager( final DependencyManager delegate ) + { + this( 0, delegate, new LinkedList<Artifact>() ); + } + + private PluginDependencyManager( final int depth, final DependencyManager delegate, + final List<Artifact> exclusions ) + { + super(); + this.depth = depth; + this.delegate = delegate; + this.exclusions = exclusions; + } + + @Override + public DependencyManagement manageDependency( final org.eclipse.aether.graph.Dependency dependency ) + { + boolean excluded = false; + + for ( final Artifact exclusion : this.getExclusions() ) + { + final Artifact artifact = dependency.getArtifact(); + + if ( exclusion.getGroupId().equals( artifact.getGroupId() ) + && exclusion.getArtifactId().equals( artifact.getArtifactId() ) + && exclusion.getExtension().equals( artifact.getExtension() ) + && exclusion.getClassifier() != null + ? exclusion.getClassifier().equals( artifact.getClassifier() ) + : dependency.getArtifact().getClassifier() == null ) + { + excluded = true; + break; + } + } + + return !excluded && this.depth >= 2 && this.delegate != null + ? this.delegate.manageDependency( dependency ) + : null; + + } + + @Override + public DependencyManager deriveChildManager( final DependencyCollectionContext context ) + { + return new PluginDependencyManager( this.depth + 1, + this.delegate != null + ? this.delegate.deriveChildManager( context ) + : null, + this.exclusions ); + + } + + public List<Artifact> getExclusions() + { + return this.exclusions; + } + } - DependencyFilter collectionFilter = new ScopeDependencyFilter( "provided", "test" ); - DependencyFilter resolutionFilter = AndDependencyFilter.newInstance( collectionFilter, dependencyFilter ); + // 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 + { + + 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; + } - DependencyNode node; + return equal; + } + + @Override + public int hashCode() + { + int hash = 17; + hash = hash * 31 + ( ( (Boolean) this.transitive ).hashCode() ); + return hash; + } + + } + + 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, session, repositories ) + : this.createPluginArtifact( plugin, session, repositories ); + + final ComparableVersion prerequisites = + new ComparableVersion( pluginArtifact.getProperty( "requiredMavenVersion", DEFAULT_PREREQUISITES ) ); + + 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 ) ); - transformer = + } + 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( session.getDependencySelector(), new WagonExcluder() ); + + final DependencyGraphTransformer pluginDependencyGraphTransformer = ChainedDependencyGraphTransformer.newInstance( session.getDependencyGraphTransformer(), transformer ); + final PluginDependencyManager pluginDependencyManager = + new PluginDependencyManager( classicResolution + ? new ClassicDependencyManager() + : session.getDependencyManager() ); + DefaultRepositorySystemSession pluginSession = new DefaultRepositorySystemSession( session ); - pluginSession.setDependencySelector( selector ); - pluginSession.setDependencyGraphTransformer( transformer ); + pluginSession.setDependencySelector( pluginDependencySelector ); + pluginSession.setDependencyGraphTransformer( pluginDependencyGraphTransformer ); + pluginSession.setDependencyManager( pluginDependencyManager ); 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, session.getArtifactTypeRegistry() ); if ( !JavaScopes.SYSTEM.equals( pluginDep.getScope() ) ) { pluginDep = pluginDep.setScope( JavaScopes.RUNTIME ); } request.addDependency( pluginDep ); + + if ( logger.isDebugEnabled() ) + { + logger.debug( String.format( "Collecting plugin dependency %s from project.", pluginDep ) ); + } + + pluginDependencyManager.getExclusions(). + addAll( this.collectPluginDependencyArtifacts( session, repositories, 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,8 +374,9 @@ public class DefaultPluginDependenciesResolver depRequest.setRoot( node ); repoSystem.resolveDependencies( session, depRequest ); + return node; } - catch ( DependencyCollectionException e ) + catch ( ArtifactDescriptorException | DependencyCollectionException e ) { throw new PluginResolutionException( plugin, e ); } @@ -217,27 +384,110 @@ public class DefaultPluginDependenciesResolver { throw new PluginResolutionException( plugin, e.getCause() ); } + } - return node; + 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 pluginArtifact; + } + + private List<org.eclipse.aether.artifact.Artifact> collectPluginDependencyArtifacts( + final RepositorySystemSession session, final List<RemoteRepository> repositories, + final org.eclipse.aether.graph.Dependency pluginDependency ) + throws DependencyCollectionException + { + final CollectRequest request = new CollectRequest(); + request.setRequestContext( REPOSITORY_CONTEXT ); + request.setRepositories( repositories ); + request.setRoot( pluginDependency ); + request.setTrace( RequestTrace.newChild( null, pluginDependency ) ); + + final DependencyNode node = repoSystem.collectDependencies( session, request ).getRoot(); + + if ( logger.isDebugEnabled() ) + { + node.accept( new GraphLogger() ); + } + + final PreorderNodeListGenerator nodeListGenerator = new PreorderNodeListGenerator(); + final DependencyFilter scopeDependencyFilter = new ScopeDependencyFilter( "provided", "test" ); + node.accept( new FilteringDependencyVisitor( nodeListGenerator, scopeDependencyFilter ) ); + return nodeListGenerator.getArtifacts( true ); + } + + // Keep this class in sync with org.apache.maven.project.DefaultProjectDependenciesResolver.GraphLogger class GraphLogger implements DependencyVisitor { private String indent = ""; + GraphLogger() + { + super(); + } + public boolean visitEnter( DependencyNode node ) { + assert node.getDependency() != null : "Unexpected POM resolution result."; + StringBuilder buffer = new StringBuilder( 128 ); buffer.append( indent ); org.eclipse.aether.graph.Dependency dep = node.getDependency(); - if ( dep != null ) + org.eclipse.aether.artifact.Artifact art = dep.getArtifact(); + + buffer.append( art ); + buffer.append( ':' ).append( dep.getScope() ); + + String premanagedScope = DependencyManagerUtils.getPremanagedScope( node ); + if ( premanagedScope != null && !premanagedScope.equals( dep.getScope() ) ) + { + buffer.append( " (scope managed from " ).append( premanagedScope ).append( ')' ); + } + + String premanagedVersion = DependencyManagerUtils.getPremanagedVersion( node ); + if ( premanagedVersion != null && !premanagedVersion.equals( art.getVersion() ) ) { - Artifact art = dep.getArtifact(); + buffer.append( " (version managed from " ).append( premanagedVersion ).append( ')' ); + } - buffer.append( art ); - buffer.append( ':' ).append( dep.getScope() ); + Boolean premanagedOptional = DependencyManagerUtils.getPremanagedOptional( node ); + if ( premanagedOptional != null && !premanagedOptional.equals( dep.getOptional() ) ) + { + buffer.append( " (optionality managed from " ).append( premanagedOptional ).append( ')' ); } logger.debug( buffer.toString() ); http://git-wip-us.apache.org/repos/asf/maven/blob/f1a1fc6d/maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusUtilsInjector.java ---------------------------------------------------------------------- diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusUtilsInjector.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusUtilsInjector.java index 16a0b63..eb36957 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusUtilsInjector.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusUtilsInjector.java @@ -64,24 +64,32 @@ class PlexusUtilsInjector private DependencyNode findPlexusUtils( DependencyNode node ) { - Artifact artifact = node.getDependency().getArtifact(); + DependencyNode plexusUtils = null; - if ( AID.equals( artifact.getArtifactId() ) && GID.equals( artifact.getGroupId() ) - && EXT.equals( artifact.getExtension() ) && "".equals( artifact.getClassifier() ) ) + find: { - return node; - } + if ( AID.equals( node.getArtifact().getArtifactId() ) + && GID.equals( node.getArtifact().getGroupId() ) + && EXT.equals( node.getArtifact().getExtension() ) + && "".equals( node.getArtifact().getClassifier() ) ) + { + plexusUtils = node; + break find; + } - for ( DependencyNode child : node.getChildren() ) - { - DependencyNode result = findPlexusUtils( child ); - if ( result != null ) + for ( DependencyNode child : node.getChildren() ) { - return result; + DependencyNode result = findPlexusUtils( child ); + + if ( result != null ) + { + plexusUtils = result; + break find; + } } } - return null; + return plexusUtils; } } http://git-wip-us.apache.org/repos/asf/maven/blob/f1a1fc6d/maven-core/src/main/java/org/apache/maven/plugin/internal/WagonExcluder.java ---------------------------------------------------------------------- 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 43e8cfc..94a4814 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 )