mthmulders commented on a change in pull request #667:
URL: https://github.com/apache/maven/pull/667#discussion_r795403559
##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -367,11 +354,37 @@ private MavenExecutionResult doExecute(
MavenExecutionRequest request, MavenSess
return result;
}
- private void afterSessionStart( MavenSession session )
+ private void finishRepositorySystemSession( MavenSession session,
DefaultRepositorySystemSession repoSession )
+ throws ComponentLookupException
+ {
+ WorkspaceReader reactorWorkspace = container.lookup(
WorkspaceReader.class, ReactorReader.HINT );
+ //
+ // Desired order of precedence for local artifact repositories
+ //
+ // Reactor
+ // Workspace
+ // User Local Repository
+ //
+
+ // we only lookup the project scoped ones here, lookup at system level
will also find the system components
+ // (like ReactorReader) but that does not work well as these are
special handled!
+ for ( WorkspaceReader workspaceReader :
getProjectScopedExtensionComponents( session.getProjects(),
+
WorkspaceReader.class ) )
+ {
+ repoSession.setWorkspaceReader(
ChainedWorkspaceReader.newInstance( repoSession.getWorkspaceReader(),
Review comment:
If there are multiple `WorkspaceReader` implementations, this would
create a chain that is larger than necessary; each new extension would create a
new `ChainedWorkspaceReader` that points to the previous
`ChainedWorkspaceReader` and adds the one it just detected in the extension.
Wouldn't it make more sense to first find all the extension components and
then build only one `ChainedWorkspaceReader` with all `WorkspaceReaders` found
in all the extensions?
##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -463,51 +482,59 @@ private void validateLocalRepository(
MavenExecutionRequest request )
}
}
- private Collection<AbstractMavenLifecycleParticipant>
getLifecycleParticipants( Collection<MavenProject> projects )
+ private <T> Collection<T> getExtensionComponents( Collection<MavenProject>
projects, Class<T> role )
{
- Collection<AbstractMavenLifecycleParticipant> lifecycleListeners = new
LinkedHashSet<>();
+ Collection<T> foundComponents = new LinkedHashSet<>();
ClassLoader originalClassLoader =
Thread.currentThread().getContextClassLoader();
try
{
try
{
- lifecycleListeners.addAll( container.lookupList(
AbstractMavenLifecycleParticipant.class ) );
+ foundComponents.addAll( container.lookupList( role ) );
}
catch ( ComponentLookupException e )
{
// this is just silly, lookupList should return an empty list!
- logger.warn( "Failed to lookup lifecycle participants: " +
e.getMessage() );
+ logger.warn( "Failed to lookup " + role + ": " +
e.getMessage() );
}
- Collection<ClassLoader> scannedRealms = new HashSet<>();
+ foundComponents.addAll( getProjectScopedExtensionComponents(
projects, role ) );
+ }
+ finally
+ {
+ Thread.currentThread().setContextClassLoader( originalClassLoader
);
+ }
+
+ return foundComponents;
+ }
+
+ protected <T> Collection<T> getProjectScopedExtensionComponents(
Collection<MavenProject> projects, Class<T> role )
+ {
+
+ Collection<T> foundComponents = new LinkedHashSet<>();
+ Collection<ClassLoader> scannedRealms = new HashSet<>();
+
+ for ( MavenProject project : projects )
+ {
+ ClassLoader projectRealm = project.getClassRealm();
- for ( MavenProject project : projects )
+ if ( projectRealm != null && scannedRealms.add( projectRealm ) )
{
- ClassLoader projectRealm = project.getClassRealm();
+ Thread.currentThread().setContextClassLoader( projectRealm );
Review comment:
I'm far from familiar with how class loading works in Maven, but if we
_change_ something here, shouldn't we _restore_ it by the end of the method
(just like we do in `getExtensionComponents()`)?
##########
File path: maven-core/src/main/java/org/apache/maven/DefaultMaven.java
##########
@@ -367,11 +354,37 @@ private MavenExecutionResult doExecute(
MavenExecutionRequest request, MavenSess
return result;
}
- private void afterSessionStart( MavenSession session )
+ private void finishRepositorySystemSession( MavenSession session,
DefaultRepositorySystemSession repoSession )
Review comment:
The name of this method sounds a bit odd to me. Seems more like it "adds
workspace readers" or something like that, than that it "finishes" something.
But maybe in the context of this change, it makes sense?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
