From ef776a9a1760a85abf7b163ae0a3e11642dad8bf Mon Sep 17 00:00:00 2001 From: Kristian Rosenvold Date: Mon, 26 Apr 2010 17:43:51 +0000 Subject: [PATCH] [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 --- .../internal/LifecycleWeaveBuilder.java | 123 +++++++++--------- .../lifecycle/internal/PhaseRecorder.java | 14 +- .../internal/DefaultMavenPluginManager.java | 2 +- ...ifecycleTaskSegmentCalculatorImplTest.java | 2 +- .../lifecycle/internal/PhaseRecorderTest.java | 46 +++++++ .../LifecycleExecutionPlanCalculatorStub.java | 2 +- 6 files changed, 121 insertions(+), 68 deletions(-) create mode 100644 maven-core/src/test/java/org/apache/maven/lifecycle/internal/PhaseRecorderTest.java diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleWeaveBuilder.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleWeaveBuilder.java index 301b2fbf77..b89caf2bc6 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleWeaveBuilder.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/LifecycleWeaveBuilder.java @@ -29,6 +29,7 @@ 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 void build( ProjectBuildList projectBuilds, ReactorContext buildContext, for ( TaskSegment taskSegment : taskSegments ) { ProjectBuildList segmentChunks = projectBuilds.getByTaskSegment( taskSegment ); - Set projectArtifacts = new HashSet(); - Set projectArtifactsA = new HashSet(); + Set projectArtifacts = new HashSet(); for ( ProjectSegment segmentChunk : segmentChunks ) { Artifact artifact = segmentChunk.getProject().getArtifact(); if ( artifact != null ) { - projectArtifacts.add( ArtifactUtils.key( artifact ) ); - projectArtifactsA.add( artifact ); + projectArtifacts.add( artifact ); } } for ( ProjectSegment projectBuild : segmentChunks ) @@ -122,17 +121,7 @@ public void build( ProjectBuildList projectBuilds, ReactorContext buildContext, { MavenExecutionPlan executionPlan = builderCommon.resolveBuildPlan( projectBuild.getSession(), projectBuild.getProject(), - projectBuild.getTaskSegment(), projectArtifactsA ); - for ( Artifact dependency : projectBuild.getProject().getDependencyArtifacts() ) - { - String s = ArtifactUtils.key( dependency ); - if ( projectArtifacts.contains( s ) ) - { - dependency.setFile( null ); - dependency.setResolved( false ); - dependency.setRepository( null ); - } - } + projectBuild.getTaskSegment(), projectArtifacts ); executionPlans.put( projectBuild.getProject(), executionPlan ); DependencyContext dependencyContext = @@ -193,18 +182,24 @@ public ProjectSegment call() eventCatapult.fire( ExecutionEvent.Type.ProjectStarted, projectBuild.getSession(), null ); + Collection 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 ProjectSegment call() 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 ProjectSegment call() }; } - 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 @@ else if ( !upstreamPlan.containsPhase( nextPhase ) ) } } - private void reresolveUpstreamProjectArtifacts( ProjectSegment projectBuild ) + private Collection getUpstreamReactorDependencies( ProjectSegment projectBuild ) { + Collection result = new ArrayList(); for ( MavenProject upstreamProject : projectBuild.getTransitiveUpstreamProjects() ) { Artifact upStreamArtifact = upstreamProject.getArtifact(); - Artifact dependencyArtifact = findDependency( projectBuild.getProject(), upStreamArtifact ); - if ( dependencyArtifact != null ) + if ( upStreamArtifact != null ) { - dependencyArtifact.setFile( upStreamArtifact.getFile() ); - dependencyArtifact.setResolved( true ); - dependencyArtifact.setRepository( upStreamArtifact.getRepository() ); + Artifact dependencyArtifact = findDependency( projectBuild.getProject(), upStreamArtifact ); + if ( dependencyArtifact != null ) + { + result.add( new ArtifactLink( dependencyArtifact, upStreamArtifact ) ); + } } - } - } - - private void reresolveUpstreamTestScopedArtifacts( ProjectSegment projectBuild ) - { - for ( MavenProject upstreamProject : projectBuild.getTransitiveUpstreamProjects() ) - { - Artifact upStreamArtifact = findTestScopedArtifact( upstreamProject ); - Artifact dependencyArtifact = findDependency( projectBuild.getProject(), upStreamArtifact ); - if ( dependencyArtifact != null ) + Artifact upStreamTestScopedArtifact = findTestScopedArtifact( upstreamProject ); + if ( upStreamTestScopedArtifact != null ) { - dependencyArtifact.setFile( upStreamArtifact.getFile() ); - dependencyArtifact.setResolved( upStreamArtifact.isResolved() ); - dependencyArtifact.setRepository( upStreamArtifact.getRepository() ); + Artifact dependencyArtifact = findDependency( projectBuild.getProject(), upStreamArtifact ); + if ( dependencyArtifact != null ) + { + result.add( new ArtifactLink( dependencyArtifact, upStreamTestScopedArtifact ) ); + } } - } + return result; } + private Artifact findTestScopedArtifact( MavenProject upstreamProject ) { if ( upstreamProject == null ) @@ -365,19 +351,6 @@ private static Artifact findDependency( MavenProject project, Artifact upStreamA } - 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 static void setWeaveMode( Properties properties ) { 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().... + + } + } + } \ No newline at end of file diff --git a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseRecorder.java b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseRecorder.java index 69f1bf3ffc..ba25fb0675 100644 --- a/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseRecorder.java +++ b/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseRecorder.java @@ -19,7 +19,7 @@ /** * @author Benjamin Bentmann - * @author Kristian Rosenvold (extract class) + * @author Kristian Rosenvold *

* NOTE: This class is not part of any public api and can be changed or deleted without prior notice. */ @@ -57,4 +57,16 @@ else if ( !lifecyclePhase.equals( lastLifecyclePhase ) ) } } + public boolean isDifferentPhase( MojoExecution nextMojoExecution ) + { + String lifecyclePhase = nextMojoExecution.getLifecyclePhase(); + if ( lifecyclePhase == null ) + { + return lastLifecyclePhase != null; + } + return !lifecyclePhase.equals( lastLifecyclePhase ); + + } + + } diff --git a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java index a7dec04c8a..5dc88b1985 100644 --- a/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java +++ b/maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java @@ -475,7 +475,7 @@ else if ( cause instanceof LinkageError ) } } - 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 { diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/internal/LifecycleTaskSegmentCalculatorImplTest.java b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/LifecycleTaskSegmentCalculatorImplTest.java index 21c5fecfef..5db316f875 100644 --- a/maven-core/src/test/java/org/apache/maven/lifecycle/internal/LifecycleTaskSegmentCalculatorImplTest.java +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/LifecycleTaskSegmentCalculatorImplTest.java @@ -27,7 +27,7 @@ import java.util.List; /** - * @author Kristian Rosenvold + * @author Kristian Rosenvold */ public class LifecycleTaskSegmentCalculatorImplTest extends TestCase diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/internal/PhaseRecorderTest.java b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/PhaseRecorderTest.java new file mode 100644 index 0000000000..8fce9e0f61 --- /dev/null +++ b/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 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)); + + } + +} diff --git a/maven-core/src/test/java/org/apache/maven/lifecycle/internal/stub/LifecycleExecutionPlanCalculatorStub.java b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/stub/LifecycleExecutionPlanCalculatorStub.java index 26f0738d96..54de230071 100644 --- a/maven-core/src/test/java/org/apache/maven/lifecycle/internal/stub/LifecycleExecutionPlanCalculatorStub.java +++ b/maven-core/src/test/java/org/apache/maven/lifecycle/internal/stub/LifecycleExecutionPlanCalculatorStub.java @@ -44,7 +44,7 @@ import java.util.Set; /** - * @author Kristian Rosenvold + * @author Kristian Rosenvold */ public class LifecycleExecutionPlanCalculatorStub implements LifecycleExecutionPlanCalculator