gnodet commented on a change in pull request #616:
URL: https://github.com/apache/maven/pull/616#discussion_r782944181
##########
File path:
maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java
##########
@@ -114,14 +123,42 @@ private CoreExtensionEntry createExtension( CoreExtension
extension, List<Artifa
{
String realmId =
"coreExtension>" + extension.getGroupId() + ":" +
extension.getArtifactId() + ":" + extension.getVersion();
- ClassRealm realm = classWorld.newRealm( realmId, null );
- log.debug( "Populating class realm " + realm.getId() );
- realm.setParentRealm( parentRealm );
+ final ClassRealm realm = classWorld.newRealm( realmId, null );
+ Set<String> providedArtifacts = Collections.emptySet();
+ String classLoadingStrategy = extension.getClassLoadingStrategy();
+ if ( STRATEGY_PARENT_FIRST.equals( classLoadingStrategy ) )
+ {
+ realm.importFrom( parentRealm, "" );
+ }
+ else if ( STRATEGY_PLUGIN.equals( classLoadingStrategy ) )
+ {
+ coreExports.getExportedPackages().forEach( ( p, cl ) ->
realm.importFrom( cl, p ) );
+ providedArtifacts = coreExports.getExportedArtifacts();
+ }
+ else if ( STRATEGY_SELF_FIRST.equals( classLoadingStrategy ) )
+ {
+ realm.setParentRealm( parentRealm );
+ }
+ else
+ {
+ throw new IllegalArgumentException( "Unsupported class-loading
strategy '"
+ + classLoadingStrategy + "'. Supported values are: " +
STRATEGY_PARENT_FIRST
+ + ", " + STRATEGY_PLUGIN + " and " + STRATEGY_SELF_FIRST );
+ }
+ log.debug( "Populating class realm {}", realm.getId() );
for ( Artifact artifact : artifacts )
{
- File file = artifact.getFile();
- log.debug( " Included " + file );
- realm.addURL( file.toURI().toURL() );
+ String id = artifact.getGroupId() + ":" + artifact.getArtifactId();
+ if ( providedArtifacts.contains( id ) )
+ {
+ log.debug( " Excluded {}", id );
+ }
+ else
+ {
+ File file = artifact.getFile();
+ log.debug( " Included {}", file );
Review comment:
> That's exactly what I have expected. Notice that the first log message
must be `trace`. Likely just a typo.
Not a typo at all. If I want a trace statement, I'd have to use
```
log.debug( " Included {}", id );
if ( log.isTraceEnabled() )
{
log.trace( " located at {}", file );
}
```
but then, the log output contains two lines at different levels which makes
things less readable (if needed). That's why I propose to log at level debug,
but have a different output at debug/trace. This is an usual pattern but I've
used it several times in order to provide more information at certain level,
see
https://github.com/apache/maven-mvnd/blob/71631fd1919513aa8765008fb8c96fb7b4c82b6e/daemon/src/main/java/org/apache/maven/project/CachingProjectBuilder.java#L828-L834
for example
I think this makes the log output more readable while still conditioning the
level of data that appears.
I haven't seen this pattern used in maven, hence the question which was very
specific and without typo ;-)
--
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]
