You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/10/22 02:44:22 UTC

[GitHub] [maven-build-cache-extension] AlexanderAshitkin opened a new pull request, #30: [MBUILDCACHE-24] IllegalArgumentException in forked executions

AlexanderAshitkin opened a new pull request, #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30

   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. Proposed fix is to resolve `lifecyclePhase` from the originating mojo and use it as a lifecycle phase for forked mojos. This looks quite natural, because forked execution are driven by the originating mojo thus belonging to the same phase.
    
   Sample exception:
   ```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)
   ```
   
    - [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed 
          for the change (usually before you start working on it).  Trivial changes like typos do not 
          require a JIRA issue.  Your pull request should address just this issue, without 
          pulling in other changes.
    - [ ] Each commit in the pull request should have a meaningful subject line and body.
    - [ ] Format the pull request title like `[MNG-XXX] - Fixes bug in ApproximateQuantiles`,
          where you replace `MNG-XXX` with the appropriate JIRA issue. Best practice
          is to use the JIRA issue title in the pull request title and in the first line of the 
          commit message.
    - [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will 
          be performed on your pull request automatically.
    - [ ] You have run the [Core IT][core-its] successfully.
   
   If your pull request is about ~20 lines of code you don't need to sign an
   [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
   please ask on the developers list.
   
   To make clear that you license your contribution under 
   the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   you have to acknowledge this by using the following check-box.
   
    - [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   
    - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   [core-its]: https://maven.apache.org/core-its/core-it-suite/
   


-- 
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


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

Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#issuecomment-1288960572

   I'd like to start a release of `maven-build-cache-extension` this week after merging #29.  @AlexanderAshitkin Is this PR something you want in this first release ?


-- 
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


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

Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin commented on PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#issuecomment-1289338778

   @gnodet @maximilian-novikov-db Please review


-- 
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


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

Posted by GitBox <gi...@apache.org>.
athkalia commented on PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#issuecomment-1380409800

   Hi, I am stumbling on this issue when using either the `1.0.0` or the `1.0.0-SNAPSHOT` version of the maven build cache extension. I have created a small sample project to reproduce the issue I am facing [here ](https://github.com/athkalia/maven-build-cache-forked-executions).


-- 
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


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

Posted by 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


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

Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin commented on PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#issuecomment-1289033331

   > I'd like to start a release of `maven-build-cache-extension` this week after merging #29. @AlexanderAshitkin Is this PR something you want in this first release ?
   
   HI. So, current situation is that plugin falls with error for a number of popular plugins (pod and spot bugs among them). I think we need fix this before releasing first version. The fix looks pretty safe for me, but it requires to get execution events which are only available in "core extension mode". 
   In "build extension" mode `EventSpyDispatcher` is not available to extension and it is not possible to subscribe for events easily. If I add EventSpy into extension, it disables cache because `BuildCacheMojosExecutionStrategy` is not discovered by guide. So I'm trying to find solution which will allow to run extension in both modes
   


-- 
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


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

Posted by GitBox <gi...@apache.org>.
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 the map from `ForkedProjectStarted` execution events



-- 
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


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

Posted by GitBox <gi...@apache.org>.
gnodet merged PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30


-- 
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


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

Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin commented on PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#issuecomment-1289185657

   > How are `EventSpy` and `BuildCacheMojosExecutionStrategy` related ?
   I haven't traced it down and so far do not understand how exactly it interferes, but everything points to Guice initialization lifecycle. As I understand, adding `EventSpy` forces earlier forces earlier resolution in extension realm. After that for some reason `BuildCacheMojosExecutionStrategy` strategy is not picked up and build continues with default non-caching strategy.
   
   It could be reproduced in master branch with 2 steps:
   * delete `CacheLifecycleParticipant` (to avoid circular dependency conflict)
   * make `LifecyclePhasesHelper` extend `AbstractEventSpy`
   Expected: `BuildExtensionTest` starts failing failing because cache is not intialized
   
   If there is a proper way to resolve this - I'm happily ready to pick it up


-- 
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


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

Posted by GitBox <gi...@apache.org>.
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



-- 
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


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

Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin commented on PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#issuecomment-1383048755

   > Hi, I am stumbling on this issue when using either the `1.0.0` or the `1.0.0-SNAPSHOT` version of the maven build cache extension. I have created a small sample project to reproduce the issue I am facing [here ](https://github.com/athkalia/maven-build-cache-forked-executions).
   
   Hi @athkalia 
   Thanks a lot for the reproducing project! I will try to fix the issue nearest time. But to be clear, so far cache works for lifecycle executions, not individual plugin goals. So fix will only allow running the command `com.athkalia:module-1:level0` but this execution will not be cached. The cache will be applied only to the regular root project build eg `clean install`
   I created [MBUILDCACHE-38](https://issues.apache.org/jira/browse/MBUILDCACHE-38) and fix will be provided under that ticket
   
   Thank you!


-- 
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


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

Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin commented on PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#issuecomment-1380520600

   > reproduce
   
   


-- 
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


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

Posted by GitBox <gi...@apache.org>.
gnodet commented on PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#issuecomment-1289108667

   > For now plugin fails with error for a number of popular plugins (pmd and spot bugs among them) and it looks like a serious gap to me. I think we need fix this before releasing first version. The fix looks pretty safe for me, but it requires to get execution events which I was able to properly get in "core extension mode" only.
   > In "build extension" mode `EventSpyDispatcher` is not available to extension and it is not possible to subscribe for events easily. If I add EventSpy into extension, it disables cache because `BuildCacheMojosExecutionStrategy` is not discovered by guice. So I'm trying to find solution which will allow to run extension in both modes
   
   How are `EventSpy` and `BuildCacheMojosExecutionStrategy` related ?


-- 
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


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

Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin commented on code in PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#discussion_r1004423748


##########
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:
   Fixed



-- 
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


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

Posted by GitBox <gi...@apache.org>.
AlexanderAshitkin commented on code in PR #30:
URL: https://github.com/apache/maven-build-cache-extension/pull/30#discussion_r1004424697


##########
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:
   Exactly. This is the best option i found. 



-- 
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