Repository: maven Updated Branches: refs/heads/master 1a05ae3de -> 3d2d8619b
[MNG-5687] Parallel Builds can build in wrong order Fixed JDK8 IT failure for MavenITmng3004ReactorFailureBehaviorMultithreadedTest#testitFailFastSingleThread It turns out the execution order of the modules in the build can be incorrect, in some cases severely incorrect. For parallel builds this can have all sorts of interesting side effects such as classpath appearing to be intermittently incorrect, missing jars/resources and similar. The -am options and -amd options may simply fail with the incorrect build order because expected dependencies have not been built and actual dependencies may not have been built. The underlying problem was that ProjectDependencyGraph#getDownstreamProjects and getUpstreamProjects did not actually obey the reactor build order as defined by ProjectDependencyGraph#getSortedProjects, even though the javadoc claims they should. This has only worked by accident on earlier JDK's and might not have worked at all (basically depends on Set iteration order being equal to insertion order). JDK8 has slightly different iteration order, which caused the IT failure. This problem may be the root cause of MNG-4996 and any other issue where the modules build in incorrect order. The bug affects: parallel builds command line -am (--also-make) option command line -amd (also-make-dependents) option On all java versions, although visibility might be somewhat different on different jdks. Added simple unit test that catches the problem. Project: http://git-wip-us.apache.org/repos/asf/maven/repo Commit: http://git-wip-us.apache.org/repos/asf/maven/commit/3d2d8619 Tree: http://git-wip-us.apache.org/repos/asf/maven/tree/3d2d8619 Diff: http://git-wip-us.apache.org/repos/asf/maven/diff/3d2d8619 Branch: refs/heads/master Commit: 3d2d8619b17abb781cc9fdccbfdd94f115c82c49 Parents: 1a05ae3 Author: Kristian Rosenvold <krosenv...@apache.org> Authored: Wed Sep 10 16:22:21 2014 +0200 Committer: Kristian Rosenvold <krosenv...@apache.org> Committed: Wed Sep 10 16:22:21 2014 +0200 ---------------------------------------------------------------------- .../maven/DefaultProjectDependencyGraph.java | 36 ++-- .../DefaultProjectDependencyGraphTest.java | 172 +++++++++++++++++++ 2 files changed, 190 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven/blob/3d2d8619/maven-core/src/main/java/org/apache/maven/DefaultProjectDependencyGraph.java ---------------------------------------------------------------------- diff --git a/maven-core/src/main/java/org/apache/maven/DefaultProjectDependencyGraph.java b/maven-core/src/main/java/org/apache/maven/DefaultProjectDependencyGraph.java index 84d2cc5..4074e58 100644 --- a/maven-core/src/main/java/org/apache/maven/DefaultProjectDependencyGraph.java +++ b/maven-core/src/main/java/org/apache/maven/DefaultProjectDependencyGraph.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.maven.execution.ProjectDependencyGraph; import org.apache.maven.project.DuplicateProjectException; @@ -32,7 +33,7 @@ import org.codehaus.plexus.util.dag.CycleDetectedException; /** * Describes the inter-dependencies between projects in the reactor. - * + * * @author Benjamin Bentmann */ class DefaultProjectDependencyGraph @@ -43,12 +44,13 @@ class DefaultProjectDependencyGraph /** * Creates a new project dependency graph based on the specified projects. - * + * * @param projects The projects to create the dependency graph with - * @throws DuplicateProjectException - * @throws CycleDetectedException + * @throws DuplicateProjectException + * @throws CycleDetectedException */ - public DefaultProjectDependencyGraph( Collection<MavenProject> projects ) throws CycleDetectedException, DuplicateProjectException + public DefaultProjectDependencyGraph( Collection<MavenProject> projects ) + throws CycleDetectedException, DuplicateProjectException { this.sorter = new ProjectSorter( projects ); } @@ -65,14 +67,14 @@ class DefaultProjectDependencyGraph throw new IllegalArgumentException( "project missing" ); } - Collection<String> projectIds = new HashSet<String>(); + Set<String> projectIds = new HashSet<String>(); getDownstreamProjects( ProjectSorter.getId( project ), projectIds, transitive ); - return getProjects( projectIds ); + return getSortedProjects( projectIds ); } - private void getDownstreamProjects( String projectId, Collection<String> projectIds, boolean transitive ) + private void getDownstreamProjects( String projectId, Set<String> projectIds, boolean transitive ) { for ( String id : sorter.getDependents( projectId ) ) { @@ -90,11 +92,11 @@ class DefaultProjectDependencyGraph throw new IllegalArgumentException( "project missing" ); } - Collection<String> projectIds = new HashSet<String>(); + Set<String> projectIds = new HashSet<String>(); getUpstreamProjects( ProjectSorter.getId( project ), projectIds, transitive ); - return getProjects( projectIds ); + return getSortedProjects( projectIds ); } private void getUpstreamProjects( String projectId, Collection<String> projectIds, boolean transitive ) @@ -108,21 +110,19 @@ class DefaultProjectDependencyGraph } } - private List<MavenProject> getProjects( Collection<String> projectIds ) + private List<MavenProject> getSortedProjects( Set<String> projectIds ) { - List<MavenProject> projects = new ArrayList<MavenProject>( projectIds.size() ); + List<MavenProject> result = new ArrayList<MavenProject>( projectIds.size() ); - for ( String projectId : projectIds ) + for ( MavenProject mavenProject : sorter.getSortedProjects() ) { - MavenProject project = sorter.getProjectMap().get( projectId ); - - if ( project != null ) + if ( projectIds.contains( ProjectSorter.getId( mavenProject ) ) ) { - projects.add( project ); + result.add( mavenProject ); } } - return projects; + return result; } @Override http://git-wip-us.apache.org/repos/asf/maven/blob/3d2d8619/maven-core/src/test/java/org/apache/maven/DefaultProjectDependencyGraphTest.java ---------------------------------------------------------------------- diff --git a/maven-core/src/test/java/org/apache/maven/DefaultProjectDependencyGraphTest.java b/maven-core/src/test/java/org/apache/maven/DefaultProjectDependencyGraphTest.java new file mode 100644 index 0000000..668dafb --- /dev/null +++ b/maven-core/src/test/java/org/apache/maven/DefaultProjectDependencyGraphTest.java @@ -0,0 +1,172 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.maven; + +import junit.framework.TestCase; +import org.apache.maven.execution.ProjectDependencyGraph; +import org.apache.maven.model.Dependency; +import org.apache.maven.project.DuplicateProjectException; +import org.apache.maven.project.MavenProject; +import org.codehaus.plexus.util.dag.CycleDetectedException; + +import java.util.Arrays; +import java.util.List; + +/** + * @author Kristian Rosenvold + */ +public class DefaultProjectDependencyGraphTest + extends TestCase +{ + + private final MavenProject aProject = createA(); + + private final MavenProject depender1 = createProject( Arrays.asList( toDependency( aProject ) ), "depender1" ); + + private final MavenProject depender2 = createProject( Arrays.asList( toDependency( aProject ) ), "depender2" ); + + private final MavenProject depender3 = createProject( Arrays.asList( toDependency( aProject ) ), "depender3" ); + + private final MavenProject depender4 = + createProject( Arrays.asList( toDependency( aProject ), toDependency( depender3 ) ), "depender4" ); + + private final MavenProject transitiveOnly = + createProject( Arrays.asList( toDependency( depender3 ) ), "depender5" ); + + public void testGetSortedProjects() + throws DuplicateProjectException, CycleDetectedException + { + ProjectDependencyGraph graph = new DefaultProjectDependencyGraph( Arrays.asList( depender1, aProject ) ); + final List<MavenProject> sortedProjects = graph.getSortedProjects(); + assertEquals( aProject, sortedProjects.get( 0 ) ); + assertEquals( depender1, sortedProjects.get( 1 ) ); + } + + public void testVerifyExpectedParentStructure() + throws CycleDetectedException, DuplicateProjectException + { + // This test verifies the baseline structure used in susequent tests. If this fails, the rest will fail. + ProjectDependencyGraph graph = threeProjectsDependingOnASingle(); + final List<MavenProject> sortedProjects = graph.getSortedProjects(); + assertEquals( aProject, sortedProjects.get( 0 ) ); + assertEquals( depender1, sortedProjects.get( 1 ) ); + assertEquals( depender2, sortedProjects.get( 2 ) ); + assertEquals( depender3, sortedProjects.get( 3 ) ); + } + + public void testVerifyThatDownsteamProjectsComeInSortedOrder() + throws CycleDetectedException, DuplicateProjectException + { + final List<MavenProject> downstreamProjects = + threeProjectsDependingOnASingle().getDownstreamProjects( aProject, true ); + assertEquals( depender1, downstreamProjects.get( 0 ) ); + assertEquals( depender2, downstreamProjects.get( 1 ) ); + assertEquals( depender3, downstreamProjects.get( 2 ) ); + } + + public void testTransitivesInOrder() + throws CycleDetectedException, DuplicateProjectException + { + final ProjectDependencyGraph graph = + new DefaultProjectDependencyGraph( Arrays.asList( depender1, depender4, depender2, depender3, aProject ) ); + + final List<MavenProject> downstreamProjects = graph.getDownstreamProjects( aProject, true ); + assertEquals( depender1, downstreamProjects.get( 0 ) ); + assertEquals( depender3, downstreamProjects.get( 1 ) ); + assertEquals( depender4, downstreamProjects.get( 2 ) ); + assertEquals( depender2, downstreamProjects.get( 3 ) ); + } + + public void testNonTransitivesInOrder() + throws CycleDetectedException, DuplicateProjectException + { + final ProjectDependencyGraph graph = + new DefaultProjectDependencyGraph( Arrays.asList( depender1, depender4, depender2, depender3, aProject ) ); + + final List<MavenProject> downstreamProjects = graph.getDownstreamProjects( aProject, false ); + assertEquals( depender1, downstreamProjects.get( 0 ) ); + assertEquals( depender3, downstreamProjects.get( 1 ) ); + assertEquals( depender4, downstreamProjects.get( 2 ) ); + assertEquals( depender2, downstreamProjects.get( 3 ) ); + } + + public void testWithTranistiveOnly() + throws CycleDetectedException, DuplicateProjectException + { + final ProjectDependencyGraph graph = new DefaultProjectDependencyGraph( + Arrays.asList( depender1, transitiveOnly, depender2, depender3, aProject ) ); + + final List<MavenProject> downstreamProjects = graph.getDownstreamProjects( aProject, true ); + assertEquals( depender1, downstreamProjects.get( 0 ) ); + assertEquals( depender3, downstreamProjects.get( 1 ) ); + assertEquals( transitiveOnly, downstreamProjects.get( 2 ) ); + assertEquals( depender2, downstreamProjects.get( 3 ) ); + } + + public void testWithMissingTranistiveOnly() + throws CycleDetectedException, DuplicateProjectException + { + final ProjectDependencyGraph graph = new DefaultProjectDependencyGraph( + Arrays.asList( depender1, transitiveOnly, depender2, depender3, aProject ) ); + + final List<MavenProject> downstreamProjects = graph.getDownstreamProjects( aProject, false ); + assertEquals( depender1, downstreamProjects.get( 0 ) ); + assertEquals( depender3, downstreamProjects.get( 1 ) ); + assertEquals( depender2, downstreamProjects.get( 2 ) ); + } + + public void testGetUpstreamProjects() + throws CycleDetectedException, DuplicateProjectException + { + ProjectDependencyGraph graph = threeProjectsDependingOnASingle(); + final List<MavenProject> downstreamProjects = graph.getUpstreamProjects( depender1, true ); + assertEquals( aProject, downstreamProjects.get( 0 ) ); + } + + private ProjectDependencyGraph threeProjectsDependingOnASingle() + throws CycleDetectedException, DuplicateProjectException + { + return new DefaultProjectDependencyGraph( Arrays.asList( depender1, depender2, depender3, aProject ) ); + } + + private static MavenProject createA() + { + MavenProject result = new MavenProject(); + result.setGroupId( "org.apache" ); + result.setArtifactId( "A" ); + result.setVersion( "1.2" ); + return result; + } + + static Dependency toDependency( MavenProject mavenProject ) + { + final Dependency dependency = new Dependency(); + dependency.setArtifactId( mavenProject.getArtifactId() ); + dependency.setGroupId( mavenProject.getGroupId() ); + dependency.setVersion( mavenProject.getVersion() ); + return dependency; + } + + private static MavenProject createProject( List<Dependency> dependencies, String artifactId ) + { + MavenProject result = new MavenProject(); + result.setGroupId( "org.apache" ); + result.setArtifactId( artifactId ); + result.setVersion( "1.2" ); + result.setDependencies( dependencies ); + return result; + } + +} \ No newline at end of file
