[GitHub] [maven] michael-o commented on a diff in pull request #639: [MNG-7310] Using the plexusContainer to prevent loading lifecycle defined by extensions from other submodules.

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

michael-o commented on code in PR #639:
URL: https://github.com/apache/maven/pull/639#discussion_r938569898


##########
maven-core/src/main/java/org/apache/maven/lifecycle/DefaultLifecycles.java:
##########
@@ -139,13 +138,34 @@ public List<Lifecycle> getLifeCycles()
             }
         };
 
+        Map<String, Lifecycle> lifecyclesMap = lookupLifecycleMap();
+
         // ensure canonical order of standard lifecycles
         return lifecyclesMap.values().stream()
                                 .peek( l -> Objects.requireNonNull( l.getId(), "A lifecycle must have an id." ) )
                                 .sorted( Comparator.comparing( Lifecycle::getId, comparator ) )
                                 .collect( Collectors.toList() );
     }
 
+    private Map<String, Lifecycle> lookupLifecycleMap()
+    {
+        // This code is here to ensure maven-compat's EmptyLifecycleExecutor keeps on working.
+        if ( plexusContainer == null )
+        {
+            return new HashMap<>();
+        }
+
+        // Lifecycles cannot be cached as extensions might add custom lifecycles later in the execution.
+        try
+        {
+            return plexusContainer.lookupMap( Lifecycle.class );
+        }
+        catch ( ComponentLookupException e )
+        {
+            throw new RuntimeException( "Unable to lookup lifecycles from the plexus container", e );

Review Comment:
   Would an `IllegalStateException` better here?



##########
maven-core/src/test/java/org/apache/maven/lifecycle/DefaultLifecyclesTest.java:
##########
@@ -81,7 +86,7 @@ public void testWrapperLifecycle()
     }
 
     @Test
-    public void testCustomLifecycle()
+    public void testCustomLifecycle() throws ComponentLookupException

Review Comment:
   Should be a new line if it is in the other test of this changeset



##########
maven-core/src/test/java/org/apache/maven/lifecycle/internal/stub/DefaultLifecyclesStub.java:
##########
@@ -25,15 +27,16 @@
 import java.util.Map;
 
 import static org.apache.maven.lifecycle.internal.stub.LifecycleExecutionPlanCalculatorStub.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 /**
  * @author Kristian Rosenvold
  */
 
 public class DefaultLifecyclesStub
 {
-    public static DefaultLifecycles createDefaultLifecycles()
-    {
+    public static DefaultLifecycles createDefaultLifecycles() throws ComponentLookupException {

Review Comment:
   Same here



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