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

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

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


##########
src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java:
##########
@@ -604,7 +605,7 @@ public int getMaxLocalBuildsCached()
     public String getLocalRepositoryLocation()
     {
         checkInitializedState();
-        return getLocal().getLocation();
+        return System.getProperty( CACHE_LOCATION_PROPERTY_NAME, 
getLocal().getLocation() );

Review Comment:
   Shouldn't that be 
   ```
   return getProperty( CACHE_LOCATION_PROPERTY_NAME, getLocal().getLocation() );
   ```



##########
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 );

Review Comment:
   So that's the workaround for not being to extend `EventSpy` from within the 
`SessionScope`, right ?





> 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