From 2f8d3981f9d17c51368b8240a8d25e38b54021fa Mon Sep 17 00:00:00 2001 From: Benjamin Bentmann Date: Wed, 29 Jul 2009 13:48:19 +0000 Subject: [PATCH] [MNG-3814] Reactor builds fail due to erroneous cycle in project sorting which does not consider versions git-svn-id: https://svn.apache.org/repos/asf/maven/components/trunk@798906 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/maven/execution/ProjectSorter.java | 173 +++++++---- .../maven/execution/ProjectSorterTest.java | 284 +++++++++++++++--- 2 files changed, 358 insertions(+), 99 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/execution/ProjectSorter.java b/maven-core/src/main/java/org/apache/maven/execution/ProjectSorter.java index 4b5de7b2a3..48a8b402fc 100644 --- a/maven-core/src/main/java/org/apache/maven/execution/ProjectSorter.java +++ b/maven-core/src/main/java/org/apache/maven/execution/ProjectSorter.java @@ -29,10 +29,15 @@ import java.util.Map; import org.apache.maven.artifact.ArtifactUtils; import org.apache.maven.model.Dependency; +import org.apache.maven.model.Extension; +import org.apache.maven.model.Parent; +import org.apache.maven.model.Plugin; import org.apache.maven.project.MavenProject; +import org.codehaus.plexus.util.StringUtils; import org.codehaus.plexus.util.dag.CycleDetectedException; import org.codehaus.plexus.util.dag.DAG; import org.codehaus.plexus.util.dag.TopologicalSorter; +import org.codehaus.plexus.util.dag.Vertex; public class ProjectSorter { @@ -70,81 +75,88 @@ public class ProjectSorter { dag = new DAG(); - Map projectMap = new HashMap(); + // groupId:artifactId:version -> project + Map projectMap = new HashMap( projects.size() * 2 ); + + // groupId:artifactId -> (version -> vertex) + Map> vertexMap = new HashMap>( projects.size() * 2 ); for ( MavenProject project : projects ) { - String id = getId( project ); + String projectId = getId( project ); - if ( dag.getVertex( id ) != null ) + MavenProject conflictingProject = projectMap.put( projectId, project ); + + if ( conflictingProject != null ) { - MavenProject conflictingProject = projectMap.get( id ); - - throw new DuplicateProjectException( id, conflictingProject.getFile(), project.getFile(), "Project '" + id + "' is duplicated in the reactor" ); + throw new DuplicateProjectException( projectId, conflictingProject.getFile(), project.getFile(), + "Project '" + projectId + "' is duplicated in the reactor" ); } - dag.addVertex( id ); + String projectKey = ArtifactUtils.versionlessKey( project.getGroupId(), project.getArtifactId() ); - projectMap.put( id, project ); + Map vertices = vertexMap.get( projectKey ); + if ( vertices == null ) + { + vertices = new HashMap( 2, 1 ); + vertexMap.put( projectKey, vertices ); + } + vertices.put( project.getVersion(), dag.addVertex( projectId ) ); } - for ( MavenProject project : projects ) + for ( Vertex projectVertex : (List) dag.getVerticies() ) { - String id = getId( project ); + String projectId = projectVertex.getLabel(); - for( Dependency dependency : project.getDependencies() ) + MavenProject project = projectMap.get( projectId ); + + for ( Dependency dependency : project.getDependencies() ) { - String dependencyId = ArtifactUtils.versionlessKey( dependency.getGroupId(), dependency.getArtifactId() ); - - if ( dag.getVertex( dependencyId ) != null ) - { - project.addProjectReference( projectMap.get( dependencyId ) ); - - dag.addEdge( id, dependencyId ); - } + addEdge( projectMap, vertexMap, project, projectVertex, dependency.getGroupId(), + dependency.getArtifactId(), dependency.getVersion(), false, false ); } - MavenProject parent = project.getParent(); - + Parent parent = project.getModel().getParent(); + if ( parent != null ) { - String parentId = ArtifactUtils.versionlessKey( parent.getGroupId(), parent.getArtifactId() ); - if ( dag.getVertex( parentId ) != null ) - { - // Parent is added as an edge, but must not cause a cycle - so we remove any other edges it has in conflict - if ( dag.hasEdge( parentId, id ) ) - { - dag.removeEdge( parentId, id ); - } - - dag.addEdge( id, parentId ); - } + // Parent is added as an edge, but must not cause a cycle - so we remove any other edges it has in conflict + addEdge( projectMap, vertexMap, null, projectVertex, parent.getGroupId(), parent.getArtifactId(), + parent.getVersion(), true, false ); } - - /* - - TODO: Now that the build plan is fully fleshed out we have cycles - - if ( project.getBuildPlugins() != null ) + + List buildPlugins = project.getBuildPlugins(); + if ( buildPlugins != null ) { - for( Plugin plugin : project.getBuildPlugins() ) + for ( Plugin plugin : buildPlugins ) { - String pluginId = ArtifactUtils.versionlessKey( plugin.getGroupId(), plugin.getArtifactId() ); - - if ( ( dag.getVertex( pluginId ) != null ) && !pluginId.equals( id ) ) + addEdge( projectMap, vertexMap, project, projectVertex, plugin.getGroupId(), + plugin.getArtifactId(), plugin.getVersion(), false, true ); + + for ( Dependency dependency : plugin.getDependencies() ) { - addEdgeWithParentCheck( projectMap, pluginId, project, id ); + addEdge( projectMap, vertexMap, project, projectVertex, dependency.getGroupId(), + dependency.getArtifactId(), dependency.getVersion(), false, true ); } } } - */ + + List buildExtensions = project.getBuildExtensions(); + if ( buildExtensions != null ) + { + for ( Extension extension : buildExtensions ) + { + addEdge( projectMap, vertexMap, project, projectVertex, extension.getGroupId(), + extension.getArtifactId(), extension.getVersion(), false, true ); + } + } } - List sortedProjects = new ArrayList(); + List sortedProjects = new ArrayList( projects.size() ); List sortedProjectLabels = TopologicalSorter.sort( dag ); - - for( String id : sortedProjectLabels ) + + for ( String id : sortedProjectLabels ) { sortedProjects.add( projectMap.get( id ) ); } @@ -152,30 +164,73 @@ public class ProjectSorter this.sortedProjects = Collections.unmodifiableList( sortedProjects ); } - private void addEdgeWithParentCheck( Map projectMap, String projectRefId, MavenProject project, String id ) + private void addEdge( Map projectMap, Map> vertexMap, + MavenProject project, Vertex projectVertex, String groupId, String artifactId, + String version, boolean force, boolean safe ) throws CycleDetectedException { - MavenProject extProject = projectMap.get( projectRefId ); + String projectKey = ArtifactUtils.versionlessKey( groupId, artifactId ); - if ( extProject == null ) + Map vertices = vertexMap.get( projectKey ); + + if ( vertices != null ) + { + if ( isSpecificVersion( version ) ) + { + Vertex vertex = vertices.get( version ); + if ( vertex != null ) + { + addEdge( projectVertex, vertex, project, projectMap, force, safe ); + } + } + else + { + for ( Vertex vertex : vertices.values() ) + { + addEdge( projectVertex, vertex, project, projectMap, force, safe ); + } + } + } + } + + private void addEdge( Vertex fromVertex, Vertex toVertex, MavenProject fromProject, + Map projectMap, boolean force, boolean safe ) + throws CycleDetectedException + { + if ( fromVertex.equals( toVertex ) ) { return; } - project.addProjectReference( extProject ); - - MavenProject extParent = extProject.getParent(); - if ( extParent != null ) + if ( fromProject != null ) { - String parentId = ArtifactUtils.versionlessKey( extParent.getGroupId(), extParent.getArtifactId() ); - // Don't add edge from parent to extension if a reverse edge already exists - if ( !dag.hasEdge( projectRefId, id ) || !parentId.equals( id ) ) + MavenProject toProject = projectMap.get( toVertex.getLabel() ); + fromProject.addProjectReference( toProject ); + } + + if ( force && toVertex.getChildren().contains( fromVertex ) ) + { + dag.removeEdge( toVertex, fromVertex ); + } + + try + { + dag.addEdge( fromVertex, toVertex ); + } + catch ( CycleDetectedException e ) + { + if ( !safe ) { - dag.addEdge( id, projectRefId ); + throw e; } } } + private boolean isSpecificVersion( String version ) + { + return !( StringUtils.isEmpty( version ) || version.startsWith( "[" ) || version.startsWith( "(" ) ); + } + // TODO: !![jc; 28-jul-2005] check this; if we're using '-r' and there are aggregator tasks, this will result in weirdness. public MavenProject getTopLevelProject() { @@ -216,7 +271,7 @@ public class ProjectSorter public static String getId( MavenProject project ) { - return ArtifactUtils.versionlessKey( project.getGroupId(), project.getArtifactId() ); + return ArtifactUtils.key( project.getGroupId(), project.getArtifactId(), project.getVersion() ); } } diff --git a/maven-core/src/test/java/org/apache/maven/execution/ProjectSorterTest.java b/maven-core/src/test/java/org/apache/maven/execution/ProjectSorterTest.java index cab60c73d2..1ebbfb8286 100644 --- a/maven-core/src/test/java/org/apache/maven/execution/ProjectSorterTest.java +++ b/maven-core/src/test/java/org/apache/maven/execution/ProjectSorterTest.java @@ -29,6 +29,9 @@ import org.apache.maven.model.Build; import org.apache.maven.model.Dependency; import org.apache.maven.model.Extension; import org.apache.maven.model.Model; +import org.apache.maven.model.Parent; +import org.apache.maven.model.Plugin; +import org.apache.maven.model.PluginManagement; import org.apache.maven.project.MavenProject; import org.codehaus.plexus.util.dag.CycleDetectedException; @@ -42,25 +45,115 @@ public class ProjectSorterTest extends TestCase { + private Parent createParent( MavenProject project ) + { + return createParent( project.getGroupId(), project.getArtifactId(), project.getVersion() ); + } + + private Parent createParent( String groupId, String artifactId, String version ) + { + Parent plugin = new Parent(); + plugin.setGroupId( groupId ); + plugin.setArtifactId( artifactId ); + plugin.setVersion( version ); + return plugin; + } + + private Dependency createDependency( MavenProject project ) + { + return createDependency( project.getGroupId(), project.getArtifactId(), project.getVersion() ); + } + + private Dependency createDependency( String groupId, String artifactId, String version ) + { + Dependency depdendency = new Dependency(); + depdendency.setGroupId( groupId ); + depdendency.setArtifactId( artifactId ); + depdendency.setVersion( version ); + return depdendency; + } + + private Plugin createPlugin( MavenProject project ) + { + return createPlugin( project.getGroupId(), project.getArtifactId(), project.getVersion() ); + } + + private Plugin createPlugin( String groupId, String artifactId, String version ) + { + Plugin plugin = new Plugin(); + plugin.setGroupId( groupId ); + plugin.setArtifactId( artifactId ); + plugin.setVersion( version ); + return plugin; + } + + private Extension createExtension( String groupId, String artifactId, String version ) + { + Extension extension = new Extension(); + extension.setGroupId( groupId ); + extension.setArtifactId( artifactId ); + extension.setVersion( version ); + return extension; + } + + private static MavenProject createProject( String groupId, String artifactId, String version ) + { + Model model = new Model(); + model.setGroupId( groupId ); + model.setArtifactId( artifactId ); + model.setVersion( version ); + model.setBuild( new Build() ); + return new MavenProject( model ); + } + + public void testShouldNotFailWhenPluginDepReferencesCurrentProject() + throws CycleDetectedException, DuplicateProjectException + { + MavenProject project = createProject( "group", "artifact", "1.0" ); + + Build build = project.getModel().getBuild(); + + Plugin plugin = createPlugin( "other.group", "other-artifact", "1.0" ); + + Dependency dep = createDependency( "group", "artifact", "1.0" ); + + plugin.addDependency( dep ); + + build.addPlugin( plugin ); + + new ProjectSorter( Collections.singletonList( project ) ); + } + + public void testShouldNotFailWhenManagedPluginDepReferencesCurrentProject() + throws CycleDetectedException, DuplicateProjectException + { + MavenProject project = createProject( "group", "artifact", "1.0" ); + + Build build = project.getModel().getBuild(); + + PluginManagement pMgmt = new PluginManagement(); + + Plugin plugin = createPlugin( "other.group", "other-artifact", "1.0" ); + + Dependency dep = createDependency( "group", "artifact", "1.0" ); + + plugin.addDependency( dep ); + + pMgmt.addPlugin( plugin ); + + build.setPluginManagement( pMgmt ); + + new ProjectSorter( Collections.singletonList( project ) ); + } + public void testShouldNotFailWhenProjectReferencesNonExistentProject() throws CycleDetectedException, DuplicateProjectException { MavenProject project = createProject( "group", "artifact", "1.0" ); - Model model = project.getModel(); - Build build = model.getBuild(); + Build build = project.getModel().getBuild(); - if ( build == null ) - { - build = new Build(); - model.setBuild( build ); - } - - Extension extension = new Extension(); - - extension.setArtifactId( "other-artifact" ); - extension.setGroupId( "other.group" ); - extension.setVersion( "1.0" ); + Extension extension = createExtension( "other.group", "other-artifact", "1.0" ); build.addExtension( extension ); @@ -70,7 +163,7 @@ public class ProjectSorterTest public void testMatchingArtifactIdsDifferentGroupIds() throws CycleDetectedException, DuplicateProjectException { - List projects = new ArrayList(); + List projects = new ArrayList(); MavenProject project1 = createProject( "groupId1", "artifactId", "1.0" ); projects.add( project1 ); MavenProject project2 = createProject( "groupId2", "artifactId", "1.0" ); @@ -86,7 +179,7 @@ public class ProjectSorterTest public void testMatchingGroupIdsDifferentArtifactIds() throws CycleDetectedException, DuplicateProjectException { - List projects = new ArrayList(); + List projects = new ArrayList(); MavenProject project1 = createProject( "groupId", "artifactId1", "1.0" ); projects.add( project1 ); MavenProject project2 = createProject( "groupId", "artifactId2", "1.0" ); @@ -102,7 +195,7 @@ public class ProjectSorterTest public void testMatchingIdsAndVersions() throws CycleDetectedException { - List projects = new ArrayList(); + List projects = new ArrayList(); MavenProject project1 = createProject( "groupId", "artifactId", "1.0" ); projects.add( project1 ); MavenProject project2 = createProject( "groupId", "artifactId", "1.0" ); @@ -121,41 +214,152 @@ public class ProjectSorterTest } public void testMatchingIdsAndDifferentVersions() - throws CycleDetectedException + throws CycleDetectedException, DuplicateProjectException { - List projects = new ArrayList(); + List projects = new ArrayList(); MavenProject project1 = createProject( "groupId", "artifactId", "1.0" ); projects.add( project1 ); MavenProject project2 = createProject( "groupId", "artifactId", "2.0" ); projects.add( project2 ); - try - { - projects = new ProjectSorter( projects ).getSortedProjects(); - fail( "Duplicate projects should fail" ); - } - catch ( DuplicateProjectException e ) - { - // expected - assertTrue( true ); - } + projects = new ProjectSorter( projects ).getSortedProjects(); + assertEquals( project1, projects.get( 0 ) ); + assertEquals( project2, projects.get( 1 ) ); } - private Dependency createDependency( MavenProject project ) + public void testPluginDependenciesInfluenceSorting() + throws Exception { - Dependency depdendency = new Dependency(); - depdendency.setArtifactId( project.getArtifactId() ); - depdendency.setGroupId( project.getGroupId() ); - depdendency.setVersion( project.getVersion() ); - return depdendency; + List projects = new ArrayList(); + + MavenProject parentProject = createProject( "groupId", "parent", "1.0" ); + projects.add( parentProject ); + + MavenProject declaringProject = createProject( "groupId", "declarer", "1.0" ); + declaringProject.setParent( parentProject ); + declaringProject.getModel().setParent( createParent( parentProject ) ); + projects.add( declaringProject ); + + MavenProject pluginLevelDepProject = createProject( "groupId", "plugin-level-dep", "1.0" ); + pluginLevelDepProject.setParent( parentProject ); + pluginLevelDepProject.getModel().setParent( createParent( parentProject ) ); + projects.add( pluginLevelDepProject ); + + MavenProject pluginProject = createProject( "groupId", "plugin", "1.0" ); + pluginProject.setParent( parentProject ); + pluginProject.getModel().setParent( createParent( parentProject ) ); + projects.add( pluginProject ); + + Plugin plugin = createPlugin( pluginProject ); + + plugin.addDependency( createDependency( pluginLevelDepProject ) ); + + Build build = declaringProject.getModel().getBuild(); + + build.addPlugin( plugin ); + + projects = new ProjectSorter( projects ).getSortedProjects(); + + assertEquals( parentProject, projects.get( 0 ) ); + + // the order of these two is non-deterministic, based on when they're added to the reactor. + assertTrue( projects.contains( pluginProject ) ); + assertTrue( projects.contains( pluginLevelDepProject ) ); + + // the declaring project MUST be listed after the plugin and its plugin-level dep, though. + assertEquals( declaringProject, projects.get( 3 ) ); } - private static MavenProject createProject( String groupId, String artifactId, String version ) + public void testPluginDependenciesInfluenceSorting_DeclarationInParent() + throws Exception { - Model model = new Model(); - model.setGroupId( groupId ); - model.setArtifactId( artifactId ); - model.setVersion( version ); - return new MavenProject( model ); + List projects = new ArrayList(); + + MavenProject parentProject = createProject( "groupId", "parent-declarer", "1.0" ); + projects.add( parentProject ); + + MavenProject pluginProject = createProject( "groupId", "plugin", "1.0" ); + pluginProject.setParent( parentProject ); + pluginProject.getModel().setParent( createParent( parentProject ) ); + projects.add( pluginProject ); + + MavenProject pluginLevelDepProject = createProject( "groupId", "plugin-level-dep", "1.0" ); + pluginLevelDepProject.setParent( parentProject ); + pluginLevelDepProject.getModel().setParent( createParent( parentProject ) ); + projects.add( pluginLevelDepProject ); + + Plugin plugin = createPlugin( pluginProject ); + + plugin.addDependency( createDependency( pluginLevelDepProject ) ); + + Build build = parentProject.getModel().getBuild(); + + build.addPlugin( plugin ); + + projects = new ProjectSorter( projects ).getSortedProjects(); + + System.out.println( projects ); + + assertEquals( parentProject, projects.get( 0 ) ); + + // the order of these two is non-deterministic, based on when they're added to the reactor. + assertTrue( projects.contains( pluginProject ) ); + assertTrue( projects.contains( pluginLevelDepProject ) ); } + + public void testPluginVersionsAreConsidered() + throws Exception + { + List projects = new ArrayList(); + + MavenProject pluginProjectA = createProject( "group", "plugin-a", "2.0-SNAPSHOT" ); + projects.add( pluginProjectA ); + pluginProjectA.getModel().getBuild().addPlugin( createPlugin( "group", "plugin-b", "1.0" ) ); + + MavenProject pluginProjectB = createProject( "group", "plugin-b", "2.0-SNAPSHOT" ); + projects.add( pluginProjectB ); + pluginProjectB.getModel().getBuild().addPlugin( createPlugin( "group", "plugin-a", "1.0" ) ); + + projects = new ProjectSorter( projects ).getSortedProjects(); + + assertTrue( projects.contains( pluginProjectA ) ); + assertTrue( projects.contains( pluginProjectB ) ); + } + + public void testDependencyPrecedesProjectThatUsesSpecificDependencyVersion() + throws Exception + { + List projects = new ArrayList(); + + MavenProject usingProject = createProject( "group", "project", "1.0" ); + projects.add( usingProject ); + usingProject.getModel().addDependency( createDependency( "group", "dependency", "1.0" ) ); + + MavenProject pluginProject = createProject( "group", "dependency", "1.0" ); + projects.add( pluginProject ); + + projects = new ProjectSorter( projects ).getSortedProjects(); + + assertEquals( pluginProject, projects.get( 0 ) ); + assertEquals( usingProject, projects.get( 1 ) ); + } + + public void testDependencyPrecedesProjectThatUsesUnresolvedDependencyVersion() + throws Exception + { + List projects = new ArrayList(); + + MavenProject usingProject = createProject( "group", "project", "1.0" ); + projects.add( usingProject ); + usingProject.getModel().addDependency( createDependency( "group", "dependency", "[1.0,)" ) ); + + MavenProject pluginProject = createProject( "group", "dependency", "1.0" ); + projects.add( pluginProject ); + + projects = new ProjectSorter( projects ).getSortedProjects(); + + assertEquals( pluginProject, projects.get( 0 ) ); + assertEquals( usingProject, projects.get( 1 ) ); + } + }