[MNG-4633] Adjusted upstream reactor artifact resolution to resolve on every phase change

Changed build summary time to show effective mojo time in weave mode, it's the only thing that made sense.
Removed uneccessary synchronized block in DefaultMavenPluginManager because now everything JustWorks(TM)

git-svn-id: https://svn.apache.org/repos/asf/maven/maven-3/trunk@938149 13f79535-47bb-0310-9956-ffa450edef68

maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleWeaveBuilder.java

@@ -29,6 +29,7 @@ import org.codehaus.plexus.component.annotations.Requirement;
 import org.codehaus.plexus.logging.Logger;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -105,15 +106,13 @@ public class LifecycleWeaveBuilder
             for ( TaskSegment taskSegment : taskSegments )
             {
                 ProjectBuildList segmentChunks = projectBuilds.getByTaskSegment( taskSegment );
-                Set<String> projectArtifacts = new HashSet<String>();
+                Set<Artifact> projectArtifacts = new HashSet<Artifact>();
-                Set<Artifact> projectArtifactsA = new HashSet<Artifact>();
                 for ( ProjectSegment segmentChunk : segmentChunks )
                 {
                     Artifact artifact = segmentChunk.getProject().getArtifact();
                     if ( artifact != null )
                     {
-                        projectArtifacts.add( ArtifactUtils.key( artifact ) );
+                        projectArtifacts.add( artifact );
-                        projectArtifactsA.add( artifact );
                     }
                 }
                 for ( ProjectSegment projectBuild : segmentChunks )
@@ -122,17 +121,7 @@ public class LifecycleWeaveBuilder
                     {
                         MavenExecutionPlan executionPlan =
                             builderCommon.resolveBuildPlan( projectBuild.getSession(), projectBuild.getProject(),
-                                                            projectBuild.getTaskSegment(), projectArtifactsA );
+                                                            projectBuild.getTaskSegment(), projectArtifacts );
-                        for ( Artifact dependency : projectBuild.getProject().getDependencyArtifacts() )
-                        {
-                            String s = ArtifactUtils.key( dependency );
-                            if ( projectArtifacts.contains( s ) )
-                            {
-                                dependency.setFile( null );
-                                dependency.setResolved( false );
-                                dependency.setRepository( null );
-                            }
-                        }
 
                         executionPlans.put( projectBuild.getProject(), executionPlan );
                         DependencyContext dependencyContext =
@@ -193,18 +182,24 @@ public class LifecycleWeaveBuilder
 
                 eventCatapult.fire( ExecutionEvent.Type.ProjectStarted, projectBuild.getSession(), null );
 
+                Collection<ArtifactLink> dependencyLinks = getUpstreamReactorDependencies( projectBuild );
+
                 try
                 {
+                    PhaseRecorder phaseRecorder = new PhaseRecorder( projectBuild.getProject() );
+                    long totalMojoTime = 0;
+                    long mojoStart;
                     while ( current != null && !reactorBuildStatus.isHaltedOrBlacklisted( projectBuild.getProject() ) )
                     {
-                        PhaseRecorder phaseRecorder = new PhaseRecorder( projectBuild.getProject() );
 
                         BuildLogItem builtLogItem =
                             concurrentBuildLogger.createBuildLogItem( projectBuild.getProject(), current );
                         final Schedule schedule = current.getSchedule();
 
+                        mojoStart = System.currentTimeMillis();
                         buildExecutionPlanItem( current, phaseRecorder, schedule, reactorContext, projectBuild,
                                                 dependencyContext );
+                        totalMojoTime += ( System.currentTimeMillis() - mojoStart );
 
                         current.setComplete();
                         builtLogItem.setComplete();
@@ -219,14 +214,20 @@ public class LifecycleWeaveBuilder
                                 waitForAppropriateUpstreamExecutionsToFinish( builtLogItem, nextPlanItem,
                                                                               projectBuild );
                             }
-                            reResolveReactorDependencies( nextPlanItem, projectBuild );
+
+                            if ( phaseRecorder.isDifferentPhase( nextPlanItem.getMojoExecution() ) )
+                            {
+                                for ( ArtifactLink dependencyLink : dependencyLinks )
+                                {
+                                    dependencyLink.resolveFromUpstream();
+                                }
+                            }
                         }
                         current = nextPlanItem;
                     }
 
-                    final long wallClockTime = System.currentTimeMillis() - buildStartTime;
                     final BuildSuccess summary =
-                        new BuildSuccess( projectBuild.getProject(), wallClockTime ); // - waitingTime
+                        new BuildSuccess( projectBuild.getProject(), totalMojoTime ); // - waitingTime
                     reactorContext.getResult().addBuildSummary( summary );
                     eventCatapult.fire( ExecutionEvent.Type.ProjectSucceeded, projectBuild.getSession(), null );
                 }
@@ -249,18 +250,6 @@ public class LifecycleWeaveBuilder
         };
     }
 
-    private void reResolveReactorDependencies( ExecutionPlanItem nextPlanItem, ProjectSegment projectBuild )
-    {
-        if ( requiresReResolutionOfUpstreamReactorArtifacts( nextPlanItem ) )
-        {
-            reresolveUpstreamProjectArtifacts( projectBuild );
-        }
-        else if ( requiresReResolutionOfUpstreamTestScopedReactorArtifacts( nextPlanItem ) )
-        {
-            reresolveUpstreamTestScopedArtifacts( projectBuild );
-        }
-    }
-
     private void waitForAppropriateUpstreamExecutionsToFinish( BuildLogItem builtLogItem,
                                                                ExecutionPlanItem nextPlanItem,
                                                                ProjectSegment projectBuild )
@@ -293,38 +282,35 @@ public class LifecycleWeaveBuilder
         }
     }
 
-    private void reresolveUpstreamProjectArtifacts( ProjectSegment projectBuild )
+    private Collection<ArtifactLink> getUpstreamReactorDependencies( ProjectSegment projectBuild )
     {
+        Collection<ArtifactLink> result = new ArrayList<ArtifactLink>();
         for ( MavenProject upstreamProject : projectBuild.getTransitiveUpstreamProjects() )
         {
             Artifact upStreamArtifact = upstreamProject.getArtifact();
-            Artifact dependencyArtifact = findDependency( projectBuild.getProject(), upStreamArtifact );
+            if ( upStreamArtifact != null )
-            if ( dependencyArtifact != null )
             {
-                dependencyArtifact.setFile( upStreamArtifact.getFile() );
+                Artifact dependencyArtifact = findDependency( projectBuild.getProject(), upStreamArtifact );
-                dependencyArtifact.setResolved( true );
+                if ( dependencyArtifact != null )
-                dependencyArtifact.setRepository( upStreamArtifact.getRepository() );
+                {
+                    result.add( new ArtifactLink( dependencyArtifact, upStreamArtifact ) );
+                }
             }
 
-        }
+            Artifact upStreamTestScopedArtifact = findTestScopedArtifact( upstreamProject );
-    }
+            if ( upStreamTestScopedArtifact != null )
-
-    private void reresolveUpstreamTestScopedArtifacts( ProjectSegment projectBuild )
-    {
-        for ( MavenProject upstreamProject : projectBuild.getTransitiveUpstreamProjects() )
-        {
-            Artifact upStreamArtifact = findTestScopedArtifact( upstreamProject );
-            Artifact dependencyArtifact = findDependency( projectBuild.getProject(), upStreamArtifact );
-            if ( dependencyArtifact != null )
             {
-                dependencyArtifact.setFile( upStreamArtifact.getFile() );
+                Artifact dependencyArtifact = findDependency( projectBuild.getProject(), upStreamArtifact );
-                dependencyArtifact.setResolved( upStreamArtifact.isResolved() );
+                if ( dependencyArtifact != null )
-                dependencyArtifact.setRepository( upStreamArtifact.getRepository() );
+                {
+                    result.add( new ArtifactLink( dependencyArtifact, upStreamTestScopedArtifact ) );
+                }
             }
-
         }
+        return result;
     }
 
+
     private Artifact findTestScopedArtifact( MavenProject upstreamProject )
     {
         if ( upstreamProject == null )
@@ -365,19 +351,6 @@ public class LifecycleWeaveBuilder
 
     }
 
-    private boolean requiresReResolutionOfUpstreamReactorArtifacts( ExecutionPlanItem nextExecutionPlanItem )
-    {
-        final String phase = nextExecutionPlanItem.getLifecyclePhase();
-        return "package".equals( phase ) || "install".equals( phase ) || "compile".equals( phase );
-    }
-
-    private boolean requiresReResolutionOfUpstreamTestScopedReactorArtifacts( ExecutionPlanItem nextExecutionPlanItem )
-    {
-        final String phase = nextExecutionPlanItem.getLifecyclePhase();
-        return "package".equals( phase ) || "install".equals( phase ) || "compile".equals( phase ) ||
-            "test-compile".equals( phase );
-    }
-
     private void buildExecutionPlanItem( ExecutionPlanItem current, PhaseRecorder phaseRecorder, Schedule schedule,
                                          ReactorContext reactorContext, ProjectSegment projectBuild,
                                          DependencyContext dependencyContext )
@@ -440,4 +413,26 @@ public class LifecycleWeaveBuilder
     {
         properties.setProperty( "maven3.weaveMode", "true" );
     }
+
+    static class ArtifactLink
+    {
+        private final Artifact artifactInThis;
+
+        private final Artifact upstream;
+
+        ArtifactLink( Artifact artifactInThis, Artifact upstream )
+        {
+            this.artifactInThis = artifactInThis;
+            this.upstream = upstream;
+        }
+
+        public void resolveFromUpstream()
+        {
+            artifactInThis.setFile( upstream.getFile() );
+            artifactInThis.setRepository( upstream.getRepository() );
+            artifactInThis.setResolved( true ); // Or maybe upstream.isResolved()....
+
+        }
+    }
+
 }

maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseRecorder.java

@@ -19,7 +19,7 @@ import org.apache.maven.project.MavenProject;
 
 /**
  * @author Benjamin Bentmann
- * @author Kristian Rosenvold (extract class)
+ * @author Kristian Rosenvold
  *         <p/>
  *         NOTE: This class is not part of any public api and can be changed or deleted without prior notice.
  */
@@ -57,4 +57,16 @@ public class PhaseRecorder
         }
     }
 
+    public boolean isDifferentPhase( MojoExecution nextMojoExecution )
+    {
+        String lifecyclePhase = nextMojoExecution.getLifecyclePhase();
+        if ( lifecyclePhase == null )
+        {
+            return lastLifecyclePhase != null;
+        }
+        return !lifecyclePhase.equals( lastLifecyclePhase );
+
+    }
+
+
 }

maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java

@@ -475,7 +475,7 @@ public class DefaultMavenPluginManager
         }
     }
 
-    private synchronized void populatePluginFields( Object mojo, MojoDescriptor mojoDescriptor, ClassRealm pluginRealm,
+    private void populatePluginFields( Object mojo, MojoDescriptor mojoDescriptor, ClassRealm pluginRealm,
                                        PlexusConfiguration configuration, ExpressionEvaluator expressionEvaluator )
         throws PluginConfigurationException
     {

maven-core/src/test/java/org/apache/maven/lifecycle/internal/LifecycleTaskSegmentCalculatorImplTest.java

@@ -27,7 +27,7 @@ import org.apache.maven.lifecycle.internal.stub.ProjectDependencyGraphStub;
 import java.util.List;
 
 /**
- * @author <a href="mailto:kristian@zenior.no">Kristian Rosenvold</a>
+ * @author Kristian Rosenvold
  */
 public class LifecycleTaskSegmentCalculatorImplTest
     extends TestCase

maven-core/src/test/java/org/apache/maven/lifecycle/internal/PhaseRecorderTest.java

@@ -0,0 +1,46 @@
+package org.apache.maven.lifecycle.internal;
+/*
+ * 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.
+ */
+import junit.framework.TestCase;
+import org.apache.maven.lifecycle.MavenExecutionPlan;
+import org.apache.maven.lifecycle.internal.stub.LifecycleExecutionPlanCalculatorStub;
+import org.apache.maven.lifecycle.internal.stub.ProjectDependencyGraphStub;
+import org.apache.maven.plugin.MojoExecution;
+
+import java.util.List;
+
+/**
+ * @author Kristian Rosenvold
+ */
+public class PhaseRecorderTest extends TestCase
+{
+    public void testObserveExecution() throws Exception {
+        PhaseRecorder phaseRecorder = new PhaseRecorder( ProjectDependencyGraphStub.A);
+        MavenExecutionPlan plan = LifecycleExecutionPlanCalculatorStub.getProjectAExceutionPlan();
+        final List<MojoExecution> executions = plan.getMojoExecutions();
+
+        final MojoExecution mojoExecution1 = executions.get( 0 );
+        final MojoExecution mojoExecution2 = executions.get( 1 );
+        phaseRecorder.observeExecution( mojoExecution1 );
+
+        assertTrue( ProjectDependencyGraphStub.A.hasCompletedPhase( mojoExecution1.getLifecyclePhase() ));
+        assertFalse( ProjectDependencyGraphStub.A.hasCompletedPhase( mojoExecution2.getLifecyclePhase() ));
+
+        assertFalse( phaseRecorder.isDifferentPhase( mojoExecution1));
+        assertTrue( phaseRecorder.isDifferentPhase( mojoExecution2));
+
+    }
+
+}

maven-core/src/test/java/org/apache/maven/lifecycle/internal/stub/LifecycleExecutionPlanCalculatorStub.java

@@ -44,7 +44,7 @@ import java.util.List;
 import java.util.Set;
 
 /**
- * @author <a href="mailto:kristian@zenior.no">Kristian Rosenvold</a>
+ * @author Kristian Rosenvold
  */
 public class LifecycleExecutionPlanCalculatorStub
     implements LifecycleExecutionPlanCalculator