[ 
https://issues.apache.org/jira/browse/MBUILDCACHE-24?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17623341#comment-17623341
 ] 

ASF GitHub Bot commented on MBUILDCACHE-24:
-------------------------------------------

AlexanderAshitkin commented on code in PR #30:
URL: 
https://github.com/apache/maven-build-cache-extension/pull/30#discussion_r1003477563


##########
src/main/java/org/apache/maven/buildcache/LifecyclePhasesHelper.java:
##########
@@ -21,35 +21,95 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 import java.util.stream.Collectors;
+import javax.annotation.Nonnull;
+import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Named;
-import javax.inject.Singleton;
+import org.apache.maven.SessionScoped;
 import org.apache.maven.buildcache.xml.Build;
+import org.apache.maven.execution.AbstractExecutionListener;
+import org.apache.maven.execution.ExecutionEvent;
+import org.apache.maven.execution.MavenExecutionRequest;
+import org.apache.maven.execution.MavenSession;
 import org.apache.maven.lifecycle.DefaultLifecycles;
 import org.apache.maven.lifecycle.Lifecycle;
 import org.apache.maven.plugin.MojoExecution;
+import org.apache.maven.project.MavenProject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-@Singleton
+@SessionScoped
 @Named
-public class LifecyclePhasesHelper
+public class LifecyclePhasesHelper extends AbstractExecutionListener
 {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger( 
LifecyclePhasesHelper.class );
+
+    private final MavenSession session;
     private final DefaultLifecycles defaultLifecycles;
     private final List<String> phases;
     private final String lastCleanPhase;
 
+    private final ConcurrentMap<MavenProject, MojoExecution> 
forkedProjectToOrigin = new ConcurrentHashMap<>();
+
     @Inject
-    public LifecyclePhasesHelper( DefaultLifecycles defaultLifecycles,
+    public LifecyclePhasesHelper( MavenSession session,
+            DefaultLifecycles defaultLifecycles,
             @Named( "clean" ) Lifecycle cleanLifecycle )
     {
+        this.session = session;
         this.defaultLifecycles = Objects.requireNonNull( defaultLifecycles );
         this.phases = defaultLifecycles.getLifeCycles().stream()
                 .flatMap( lf -> lf.getPhases().stream() )
                 .collect( Collectors.toList() );
         this.lastCleanPhase = CacheUtils.getLast( cleanLifecycle.getPhases() );
     }
 
+    @PostConstruct
+    public void init()
+    {
+        MavenExecutionRequest request = session.getRequest();
+        ChainedListener lifecycleListener = new ChainedListener( 
request.getExecutionListener() );
+        lifecycleListener.chainListener( this );
+        request.setExecutionListener( lifecycleListener );
+    }
+
+    @Override
+    public void forkedProjectStarted( ExecutionEvent event )
+    {
+        LOGGER.debug( "Started forked project. Project: {}, instance: {}, 
originating mojo: {}", event.getProject(),
+                System.identityHashCode( event.getProject() ), 
event.getMojoExecution() );
+        forkedProjectToOrigin.put( event.getProject(), 
event.getMojoExecution() );
+    }
+
+    @Override
+    public void forkedProjectSucceeded( ExecutionEvent event )
+    {
+        LOGGER.debug( "Finished forked project. Project: {}, instance: {}", 
event.getProject(),
+                System.identityHashCode( event.getProject() ) );
+        forkedProjectToOrigin.remove( event.getProject(), 
event.getMojoExecution() );
+    }
+
+    @Override
+    public void forkedProjectFailed( ExecutionEvent event )
+    {
+        LOGGER.debug( "Finished forked project. Project: {}, instance: {}", 
event.getProject(),
+                System.identityHashCode( event.getProject() ) );
+        forkedProjectToOrigin.remove( event.getProject(), 
event.getMojoExecution() );
+    }
+
+    @Nonnull
+    public String resolveHighestLifecyclePhase( MavenProject project, 
List<MojoExecution> mojoExecutions )
+    {
+        // if forked, take originating mojo as the highest phase, else last 
mojo phase
+        return forkedProjectToOrigin.getOrDefault( project, 
CacheUtils.getLast( mojoExecutions ) )

Review Comment:
   this is the essence of the change - if forked project present, then 
lifecycle of the originating mojo (which originated forked executions) is 
taken, otherwise the last execution in the build is taken (as previously)
   
   In order to make this work, need to populate `forkedProjectToOrigin` map 
from `ForkedProjectStarted` execution events





> Cache cannot be processed in presence of forked executions
> ----------------------------------------------------------
>
>                 Key: MBUILDCACHE-24
>                 URL: https://issues.apache.org/jira/browse/MBUILDCACHE-24
>             Project: Maven Build Cache Extension
>          Issue Type: Bug
>            Reporter: Alexander Ashitkin
>            Priority: Major
>              Labels: pull-request-available
>
> Current implementation relies on lifecycle phases presence in 
> `MojoExecution`. In case of forked execution 
> `MojoExecution#getLifecyclePhase` could return null which result in IAEs in 
> multiples places in cache:
>  
> {code:java}
> java.lang.IllegalArgumentException: Unsupported phase: null
>     at org.apache.maven.buildcache.LifecyclePhasesHelper.isLaterPhase 
> (LifecyclePhasesHelper.java:121)
>     at 
> org.apache.maven.buildcache.LifecyclePhasesHelper.isLaterPhaseThanClean 
> (LifecyclePhasesHelper.java:105)
>     at org.apache.maven.buildcache.LifecyclePhasesHelper.getCleanSegment 
> (LifecyclePhasesHelper.java:139)
>     at org.apache.maven.buildcache.BuildCacheMojosExecutionStrategy.execute 
> (BuildCacheMojosExecutionStrategy.java:101)
>     at org.apache.maven.lifecycle.internal.MojoExecutor.execute 
> (MojoExecutor.java:153)
>     at 
> org.apache.maven.lifecycle.internal.MojoExecutor.executeForkedExecutions 
> (MojoExecutor.java:366)
>     at org.apache.maven.lifecycle.internal.MojoExecutor.execute 
> (MojoExecutor.java:211)
>     at org.apache.maven.lifecycle.internal.MojoExecutor.execute 
> (MojoExecutor.java:167)
>     at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 
> (MojoExecutor.java:66)
>     at org.apache.maven.lifecycle.internal.MojoExecutor$1.run 
> (MojoExecutor.java:158)
>     at org.apache.maven.buildcache.BuildCacheMojosExecutionStrategy.execute 
> (BuildCacheMojosExecutionStrategy.java:127)
>     at org.apache.maven.lifecycle.internal.MojoExecutor.execute 
> (MojoExecutor.java:153)
>     at 
> org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject 
> (LifecycleModuleBuilder.java:117)
>     at 
> org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call
>  (MultiThreadedBuilder.java:196)
>     at 
> org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call
>  (MultiThreadedBuilder.java:186)
>     at java.util.concurrent.FutureTask.run (FutureTask.java:266)
>     at java.util.concurrent.Executors$RunnableAdapter.call 
> (Executors.java:511)
>     at java.util.concurrent.FutureTask.run (FutureTask.java:266)
>     at java.util.concurrent.ThreadPoolExecutor.runWorker 
> (ThreadPoolExecutor.java:1149)
>     at java.util.concurrent.ThreadPoolExecutor$Worker.run 
> (ThreadPoolExecutor.java:624)
>  
> {code}
> Proposed fix is to resolve `lifecyclePhase` from the originating mojo and use 
> it as a lifecycle phase for forked mojos. 
> Optionally, the issue probably could be resolved in maven-core by properly 
> setting execution phase. 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to