michael-o commented on a change in pull request #668:
URL: https://github.com/apache/maven/pull/668#discussion_r805199186



##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -382,34 +397,50 @@ private void validateLocalRepository( 
MavenExecutionRequest request )
                 logger.warn( "Failed to lookup lifecycle participants: " + 
e.getMessage() );
             }
 
-            Collection<ClassLoader> scannedRealms = new HashSet<>();
+            lifecycleListeners.addAll( getProjectScopedExtensions( projects,
+                                                                   
AbstractMavenLifecycleParticipant.class ) );
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader( originalClassLoader 
);
+        }

Review comment:
       The classloader is never overridden here, leftover?

##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -232,25 +232,40 @@ private MavenExecutionResult doExecute( 
MavenExecutionRequest request, MavenSess
             return addExceptionToResult( result, e );
         }
 
-        WorkspaceReader reactorWorkspace;
+        List<WorkspaceReader> workspaceReaders = new ArrayList<>();
+        WorkspaceReader repoWorkspaceReader = repoSession.getWorkspaceReader();
+        //
+        // Desired order of precedence for local artifact repositories
+        //
         try
         {
-            reactorWorkspace = container.lookup( WorkspaceReader.class, 
ReactorReader.HINT );
+            // 1) Reactor workspace reader
+            workspaceReaders.add( container.lookup( WorkspaceReader.class, 
ReactorReader.HINT ) );
         }
         catch ( ComponentLookupException e )
         {
             return addExceptionToResult( result, e );
         }
-
-        //
-        // Desired order of precedence for local artifact repositories
-        //
-        // Reactor
-        // Workspace
+        if ( repoWorkspaceReader != null )
+        {
+            // 2) Repository system session scoped workspace reader
+            workspaceReaders.add( repoWorkspaceReader );
+        }
+        for ( WorkspaceReader workspaceReader : getProjectScopedExtensions( 
session.getProjects(),
+                                                                            
WorkspaceReader.class ) )
+        {
+            if ( workspaceReaders.contains( workspaceReader ) )
+            {
+                continue;
+            }
+            // 3) .. n) Project scoped workspace reader

Review comment:
       This comment is misplaced compared to the master PR.

##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -232,25 +232,36 @@ private MavenExecutionResult doExecute( 
MavenExecutionRequest request, MavenSess
             return addExceptionToResult( result, e );
         }
 
-        WorkspaceReader reactorWorkspace;
+        List<WorkspaceReader> workspaceReaders = new ArrayList<>();
+        WorkspaceReader repoWorkspaceReader = repoSession.getWorkspaceReader();
+        // Desired order of precedence for workspace reader before quiering 
the local artifact repositories
+        // 1) Reactor workspace reader
         try
         {
-            reactorWorkspace = container.lookup( WorkspaceReader.class, 
ReactorReader.HINT );
+            workspaceReaders.add( container.lookup( WorkspaceReader.class, 
ReactorReader.HINT ) );
         }
         catch ( ComponentLookupException e )
         {
             return addExceptionToResult( result, e );
         }
+        // 2) Repository system session scoped workspace reader

Review comment:
       Must read: Repository system session-scoped workspace reader

##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -369,47 +380,54 @@ private void validateLocalRepository( 
MavenExecutionRequest request )
     {
         Collection<AbstractMavenLifecycleParticipant> lifecycleListeners = new 
LinkedHashSet<>();
 
-        ClassLoader originalClassLoader = 
Thread.currentThread().getContextClassLoader();
         try
         {
-            try
-            {
-                lifecycleListeners.addAll( container.lookupList( 
AbstractMavenLifecycleParticipant.class ) );
-            }
-            catch ( ComponentLookupException e )
-            {
-                // this is just silly, lookupList should return an empty list!
-                logger.warn( "Failed to lookup lifecycle participants: " + 
e.getMessage() );
-            }
+            lifecycleListeners.addAll( container.lookupList( 
AbstractMavenLifecycleParticipant.class ) );
+        }
+        catch ( ComponentLookupException e )
+        {
+            // this is just silly, lookupList should return an empty list!
+            logger.warn( "Failed to lookup lifecycle participants: " + 
e.getMessage() );
+        }
+
+        lifecycleListeners.addAll( getProjectScopedExtensions( projects, 
AbstractMavenLifecycleParticipant.class ) );
 
-            Collection<ClassLoader> scannedRealms = new HashSet<>();
+        return lifecycleListeners;
+    }
 
+    protected <T> Collection<T> getProjectScopedExtensions( 
Collection<MavenProject> projects, Class<T> role )

Review comment:
       This method looks inconsistent with master:
   * Name differs
   * return value is at a different position
   * returned var name is different

##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -232,25 +232,36 @@ private MavenExecutionResult doExecute( 
MavenExecutionRequest request, MavenSess
             return addExceptionToResult( result, e );
         }
 
-        WorkspaceReader reactorWorkspace;
+        List<WorkspaceReader> workspaceReaders = new ArrayList<>();
+        WorkspaceReader repoWorkspaceReader = repoSession.getWorkspaceReader();

Review comment:
       This one is not required here, but at `2)`

##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -232,25 +232,36 @@ private MavenExecutionResult doExecute( 
MavenExecutionRequest request, MavenSess
             return addExceptionToResult( result, e );
         }
 
-        WorkspaceReader reactorWorkspace;
+        List<WorkspaceReader> workspaceReaders = new ArrayList<>();
+        WorkspaceReader repoWorkspaceReader = repoSession.getWorkspaceReader();
+        // Desired order of precedence for workspace reader before quiering 
the local artifact repositories
+        // 1) Reactor workspace reader
         try
         {
-            reactorWorkspace = container.lookup( WorkspaceReader.class, 
ReactorReader.HINT );
+            workspaceReaders.add( container.lookup( WorkspaceReader.class, 
ReactorReader.HINT ) );
         }
         catch ( ComponentLookupException e )
         {
             return addExceptionToResult( result, e );
         }
+        // 2) Repository system session scoped workspace reader
+        if ( repoWorkspaceReader != null )
+        {
+            workspaceReaders.add( repoWorkspaceReader );
+        }
+        // 3) .. n) Project scoped workspace reader

Review comment:
       Must read: Project-scoped workspace readers

##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -232,25 +232,36 @@ private MavenExecutionResult doExecute( 
MavenExecutionRequest request, MavenSess
             return addExceptionToResult( result, e );
         }
 
-        WorkspaceReader reactorWorkspace;
+        List<WorkspaceReader> workspaceReaders = new ArrayList<>();
+        WorkspaceReader repoWorkspaceReader = repoSession.getWorkspaceReader();
+        // Desired order of precedence for workspace reader before quiering 
the local artifact repositories

Review comment:
       Must read: Desired order of precedence for workspace readers before 
querying the local artifact repositories




-- 
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-unsubscr...@maven.apache.org

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


Reply via email to