[GitHub] [maven-build-cache-extension] gnodet commented on a diff in pull request #30: [MBUILDCACHE-24] - Fixes IllegalArgumentException in forked executions

[GitBox <gi...@apache.org>]

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 ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org