svn commit: r639520 - /maven/components/branches/maven-2.0.x/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java

[jd...@apache.org]

Author: jdcasey
Date: Thu Mar 20 19:29:36 2008
New Revision: 639520

URL: http://svn.apache.org/viewvc?rev=639520&view=rev
Log:
[MNG-3426] Cleaned up plugin-level-dependency handling to avoid two Artifacts being resolved together that are different versions of the same thing. This is a prime use case for plugin-level deps, and in handling normal POM dependencies duplicate specification is meant to result in a validation error, so this brings plugin handling into consistency with that. It also avoids any subtleties in the ArtifactCollector that could result in the old version being chosen over the new one.

Modified:
    maven/components/branches/maven-2.0.x/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java

Modified: maven/components/branches/maven-2.0.x/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java
URL: http://svn.apache.org/viewvc/maven/components/branches/maven-2.0.x/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java?rev=639520&r1=639519&r2=639520&view=diff
==============================================================================
--- maven/components/branches/maven-2.0.x/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java (original)
+++ maven/components/branches/maven-2.0.x/maven-core/src/main/java/org/apache/maven/plugin/DefaultPluginManager.java Thu Mar 20 19:29:36 2008
@@ -91,6 +91,8 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -696,15 +698,60 @@
 
             checkPlexusUtils( resolutionGroup, artifactFactory );
 
-            // FIXME: We need to control the ordering of plugin dependencies,
-            // but this ordering change currently kills at least the plugin-plugin.
-            // Not sure why (yet).
-            // Set dependencies = new LinkedHashSet();
-            Set dependencies = new HashSet();
-            dependencies.addAll( pluginDescriptor.getIntroducedDependencyArtifacts() );
-            dependencies.addAll( resolutionGroup.getArtifacts() );
+            // [jdcasey; 20-March-2008]:
+            // This is meant to eliminate the introduction of duplicated artifacts.
+            // Since much of the reasoning for reversing the order of introduction of
+            // plugin dependencies rests on the notion that we need to be able to
+            // introduce upgraded versions of plugin dependencies on a case-by-case
+            // basis, we need to remove the original version prior to artifact
+            // resolution. This is consistent with recent thinking on duplicated
+            // dependency specifications within a POM, where that case should
+            // throw a model validation exception.
+            //
+            // Here, we just want to remove any chance that the ArtifactCollector
+            // could make a bad choice, and use the old version in spite of our
+            // explicit preference otherwise.
+
+            // First, we're going to accumulate plugin dependencies in an ordered map,
+            // keyed by dependencyConflictId (the ordered map is meant to preserve relative
+            // ordering of the dependencies that do make the cut).
+            Map dependencyMap = new LinkedHashMap();
+
+            // Next, we need to accumulate all dependencies in a List, to make it
+            // simpler to iterate through them all and add them to the map.
+            List all = new ArrayList();
+
+            // plugin-level dependencies from the consuming POM override dependencies
+            // from the plugin's own POM.
+            all.addAll( pluginDescriptor.getIntroducedDependencyArtifacts() );
 
-            getLogger().debug( "Plugin dependencies for:\n\n" + pluginDescriptor.getId() + "\n\nare:\n\n" + StringUtils.join( dependencies.iterator(), "\n" ) + "\n\n" );
+            // add in the deps from the plugin POM now.
+            all.addAll( resolutionGroup.getArtifacts() );
+
+            for ( Iterator it = all.iterator(); it.hasNext(); )
+            {
+                Artifact artifact = (Artifact) it.next();
+                String conflictId = artifact.getDependencyConflictId();
+
+                // if the map already contains this dependencyConflictId, it constitutes an
+                // overridden dependency. Don't use the old one (we know it's old from the
+                // order in which dependencies were added to this list).
+                if ( !dependencyMap.containsKey( conflictId ) )
+                {
+                    dependencyMap.put( conflictId, artifact );
+                }
+            }
+
+            // Create an ordered set of dependencies from the ordered map we used above, to feed into the resolver.
+            Set dependencies = new LinkedHashSet( dependencyMap.values() );
+
+            if ( getLogger().isDebugEnabled() )
+            {
+                // list all dependencies to be used by this plugin (first-level deps, not transitive ones).
+                getLogger().debug( "Plugin dependencies for:\n\n" + pluginDescriptor.getId()
+                                   + "\n\nare:\n\n"
+                                   + StringUtils.join( dependencies.iterator(), "\n" ) + "\n\n" );
+            }
 
             List repositories = new ArrayList();
             repositories.addAll( resolutionGroup.getResolutionRepositories() );
@@ -761,8 +808,27 @@
 
             unresolved.removeAll( resolved );
 
+            if ( getLogger().isDebugEnabled() )
+            {
+                // list all artifacts that were filtered out during the resolution process.
+                // these are already present in the core container.
+                getLogger().debug( " The following artifacts were filtered out for plugin: "
+                                   + pluginDescriptor.getId()
+                                   + " because they're already in the core of Maven:\n\n"
+                                   + StringUtils.join( unresolved.iterator(), "\n" )
+                                   + "\n\nThese will use the artifact files already in the core ClassRealm instead, to allow them to be included in PluginDescriptor.getArtifacts().\n\n" );
+            }
+
+            // Grab a file for all filtered artifacts, even if it means resolving them. This
+            // is necessary in order to present a full complement of a plugin's transitive
+            // dependencies to anyone who calls PluginDescriptor.getArtifacts().
             resolveCoreArtifacts( unresolved, localRepository, resolutionGroup.getResolutionRepositories() );
 
+            // Re-join resolved and filtered-but-now-resolved artifacts.
+            // NOTE: The process of filtering then re-adding some artifacts will
+            // result in different ordering within the PluginDescriptor.getArtifacts()
+            // List than should have happened if none had been filtered. All filtered
+            // artifacts will be listed last...
             List allResolved = new ArrayList( resolved.size() + unresolved.size() );
 
             allResolved.addAll( resolved );