From 3c67c6d82d5251e7c46bc3b78bad1ab780b38f1f Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 22 Nov 2021 17:57:39 +0100 Subject: [PATCH 1/5] [MNG-7160] Ability to choose the classloading strategy for core extensions --- maven-embedder/pom.xml | 2 +- .../BootstrapCoreExtensionManager.java | 43 ++++++++++++++++--- .../src/main/mdo/core-extensions.mdo | 8 ++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/maven-embedder/pom.xml b/maven-embedder/pom.xml index a40eccac4619..842f86823954 100644 --- a/maven-embedder/pom.xml +++ b/maven-embedder/pom.xml @@ -175,7 +175,7 @@ under the License. org.codehaus.modello modello-maven-plugin - 1.0.0 + 1.1.0 src/main/mdo/core-extensions.mdo diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java index 74570eb38be7..608e3a7216c4 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java @@ -31,6 +31,8 @@ import org.apache.maven.RepositoryUtils; import org.apache.maven.cli.internal.extension.model.CoreExtension; import org.apache.maven.execution.MavenExecutionRequest; +import org.apache.maven.extension.internal.CoreExports; +import org.apache.maven.extension.internal.CoreExportsProvider; import org.apache.maven.extension.internal.CoreExtensionEntry; import org.apache.maven.internal.aether.DefaultRepositorySystemSessionFactory; import org.apache.maven.model.Plugin; @@ -66,6 +68,8 @@ public class BootstrapCoreExtensionManager private final DefaultRepositorySystemSessionFactory repositorySystemSessionFactory; + private final CoreExportsProvider coreExportsProvider; + private final ClassWorld classWorld; private final ClassRealm parentRealm; @@ -73,10 +77,12 @@ public class BootstrapCoreExtensionManager @Inject public BootstrapCoreExtensionManager( DefaultPluginDependenciesResolver pluginDependenciesResolver, DefaultRepositorySystemSessionFactory repositorySystemSessionFactory, + CoreExportsProvider coreExportsProvider, PlexusContainer container ) { this.pluginDependenciesResolver = pluginDependenciesResolver; this.repositorySystemSessionFactory = repositorySystemSessionFactory; + this.coreExportsProvider = coreExportsProvider; this.classWorld = ( (DefaultPlexusContainer) container ).getClassWorld(); this.parentRealm = container.getContainerRealm(); } @@ -121,14 +127,39 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List providedArtifacts = Collections.emptySet(); + if ( "parent-first".equals( extension.getClassloadingStrategy() ) ) + { + realm = classWorld.newRealm( realmId, null ); + realm.importFrom( parentRealm, "" ); + } + else if ( "plugin".equals( extension.getClassloadingStrategy() ) ) + { + realm = classWorld.newRealm( realmId, null ); + CoreExports coreExports = this.coreExportsProvider.get(); + coreExports.getExportedPackages().forEach( ( p, cl ) -> realm.importFrom( cl, p ) ); + providedArtifacts = coreExports.getExportedArtifacts(); + } + else + { + realm = classWorld.newRealm( realmId, null ); + realm.setParentRealm( parentRealm ); + } + 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 ); + realm.addURL( file.toURI().toURL() ); + } } return CoreExtensionEntry.discoverFrom( realm, Collections.singleton( artifacts.get( 0 ).getFile() ) ); } diff --git a/maven-embedder/src/main/mdo/core-extensions.mdo b/maven-embedder/src/main/mdo/core-extensions.mdo index 8a74aabb5856..2992ef436e40 100644 --- a/maven-embedder/src/main/mdo/core-extensions.mdo +++ b/maven-embedder/src/main/mdo/core-extensions.mdo @@ -82,6 +82,14 @@ true String + + classloadingStrategy + The classloading strategy (self-first, parent-first, plugin) + 1.1.0+ + self-first + false + String + From d9ad0c924d564124b125f943006446463f5ff49e Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 23 Nov 2021 11:15:51 +0100 Subject: [PATCH 2/5] [MNG-7160] Use an actual Provider interface --- .../maven/DefaultArtifactFilterManager.java | 6 ++--- .../classrealm/DefaultClassRealmManager.java | 8 +++---- .../internal/CoreExportsProvider.java | 22 +++++++++---------- .../BootstrapCoreExtensionManager.java | 8 +++---- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/DefaultArtifactFilterManager.java b/maven-core/src/main/java/org/apache/maven/DefaultArtifactFilterManager.java index a746846a8b2f..810baaef38f6 100644 --- a/maven-core/src/main/java/org/apache/maven/DefaultArtifactFilterManager.java +++ b/maven-core/src/main/java/org/apache/maven/DefaultArtifactFilterManager.java @@ -29,7 +29,7 @@ import org.apache.maven.artifact.resolver.filter.ArtifactFilter; import org.apache.maven.artifact.resolver.filter.ExclusionSetFilter; -import org.apache.maven.extension.internal.CoreExportsProvider; +import org.apache.maven.extension.internal.CoreExports; /** * @author Jason van Zyl @@ -50,10 +50,10 @@ public class DefaultArtifactFilterManager @Inject public DefaultArtifactFilterManager( List delegates, - CoreExportsProvider coreExports ) + CoreExports coreExports ) { this.delegates = delegates; - this.coreArtifacts = coreExports.get().getExportedArtifacts(); + this.coreArtifacts = coreExports.getExportedArtifacts(); } private synchronized Set getExcludedArtifacts() diff --git a/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java b/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java index b9e8d22d698c..50e0e3c6f16a 100644 --- a/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java +++ b/maven-core/src/main/java/org/apache/maven/classrealm/DefaultClassRealmManager.java @@ -37,7 +37,7 @@ import org.apache.maven.artifact.ArtifactUtils; import org.apache.maven.classrealm.ClassRealmRequest.RealmType; -import org.apache.maven.extension.internal.CoreExportsProvider; +import org.apache.maven.extension.internal.CoreExports; import org.apache.maven.model.Model; import org.apache.maven.model.Plugin; import org.codehaus.plexus.MutablePlexusContainer; @@ -94,19 +94,19 @@ public class DefaultClassRealmManager @Inject public DefaultClassRealmManager( PlexusContainer container, List delegates, - CoreExportsProvider exports ) + CoreExports exports ) { this.world = ( (MutablePlexusContainer) container ).getClassWorld(); this.containerRealm = container.getContainerRealm(); this.delegates = delegates; - Map foreignImports = exports.get().getExportedPackages(); + Map foreignImports = exports.getExportedPackages(); this.mavenApiRealm = createRealm( API_REALMID, RealmType.Core, null /* parent */, null /* parentImports */, foreignImports, null /* artifacts */ ); - this.providedArtifacts = exports.get().getExportedArtifacts(); + this.providedArtifacts = exports.getExportedArtifacts(); } private ClassRealm newRealm( String id ) diff --git a/maven-core/src/main/java/org/apache/maven/extension/internal/CoreExportsProvider.java b/maven-core/src/main/java/org/apache/maven/extension/internal/CoreExportsProvider.java index 3e6f9f7b6710..b1a30fe3c3aa 100644 --- a/maven-core/src/main/java/org/apache/maven/extension/internal/CoreExportsProvider.java +++ b/maven-core/src/main/java/org/apache/maven/extension/internal/CoreExportsProvider.java @@ -19,34 +19,34 @@ * under the License. */ +import java.util.Objects; + import javax.inject.Inject; import javax.inject.Named; +import javax.inject.Provider; import javax.inject.Singleton; import org.codehaus.plexus.PlexusContainer; -import org.eclipse.sisu.Nullable; /** * CoreExportsProvider */ @Named @Singleton -public class CoreExportsProvider +public class CoreExportsProvider implements Provider { private final CoreExports exports; @Inject - public CoreExportsProvider( PlexusContainer container, @Nullable CoreExports exports ) + public CoreExportsProvider( PlexusContainer container ) + { + this( new CoreExports( CoreExtensionEntry.discoverFrom( container.getContainerRealm() ) ) ); + } + + public CoreExportsProvider( CoreExports exports ) { - if ( exports == null ) - { - this.exports = new CoreExports( CoreExtensionEntry.discoverFrom( container.getContainerRealm() ) ); - } - else - { - this.exports = exports; - } + this.exports = Objects.requireNonNull( exports ); } public CoreExports get() diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java index 608e3a7216c4..9a3aff0c8227 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java @@ -32,7 +32,6 @@ import org.apache.maven.cli.internal.extension.model.CoreExtension; import org.apache.maven.execution.MavenExecutionRequest; import org.apache.maven.extension.internal.CoreExports; -import org.apache.maven.extension.internal.CoreExportsProvider; import org.apache.maven.extension.internal.CoreExtensionEntry; import org.apache.maven.internal.aether.DefaultRepositorySystemSessionFactory; import org.apache.maven.model.Plugin; @@ -68,7 +67,7 @@ public class BootstrapCoreExtensionManager private final DefaultRepositorySystemSessionFactory repositorySystemSessionFactory; - private final CoreExportsProvider coreExportsProvider; + private final CoreExports coreExports; private final ClassWorld classWorld; @@ -77,12 +76,12 @@ public class BootstrapCoreExtensionManager @Inject public BootstrapCoreExtensionManager( DefaultPluginDependenciesResolver pluginDependenciesResolver, DefaultRepositorySystemSessionFactory repositorySystemSessionFactory, - CoreExportsProvider coreExportsProvider, + CoreExports coreExports, PlexusContainer container ) { this.pluginDependenciesResolver = pluginDependenciesResolver; this.repositorySystemSessionFactory = repositorySystemSessionFactory; - this.coreExportsProvider = coreExportsProvider; + this.coreExports = coreExports; this.classWorld = ( (DefaultPlexusContainer) container ).getClassWorld(); this.parentRealm = container.getContainerRealm(); } @@ -137,7 +136,6 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List realm.importFrom( cl, p ) ); providedArtifacts = coreExports.getExportedArtifacts(); } From 30a7c2f64f2b9be039d677ad0db19cb6453507c6 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 23 Nov 2021 17:21:39 +0100 Subject: [PATCH 3/5] [MNG-7160] Fix naming and improve documentation --- .../maven/cli/internal/BootstrapCoreExtensionManager.java | 4 ++-- maven-embedder/src/main/mdo/core-extensions.mdo | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java index 9a3aff0c8227..207353d93432 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java @@ -128,12 +128,12 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List" + extension.getGroupId() + ":" + extension.getArtifactId() + ":" + extension.getVersion(); ClassRealm realm; Set providedArtifacts = Collections.emptySet(); - if ( "parent-first".equals( extension.getClassloadingStrategy() ) ) + if ( "parent-first".equals( extension.getClassLoadingStrategy() ) ) { realm = classWorld.newRealm( realmId, null ); realm.importFrom( parentRealm, "" ); } - else if ( "plugin".equals( extension.getClassloadingStrategy() ) ) + else if ( "plugin".equals( extension.getClassLoadingStrategy() ) ) { realm = classWorld.newRealm( realmId, null ); coreExports.getExportedPackages().forEach( ( p, cl ) -> realm.importFrom( cl, p ) ); diff --git a/maven-embedder/src/main/mdo/core-extensions.mdo b/maven-embedder/src/main/mdo/core-extensions.mdo index 2992ef436e40..968258d21dac 100644 --- a/maven-embedder/src/main/mdo/core-extensions.mdo +++ b/maven-embedder/src/main/mdo/core-extensions.mdo @@ -83,8 +83,8 @@ String - classloadingStrategy - The classloading strategy (self-first, parent-first, plugin) + classLoadingStrategy + The class loading strategy: 'self-first' (the default), 'parent-first' (loads classes from the parent, then from the extension) or 'plugin' (follows the rules from extensions defined as plugins). 1.1.0+ self-first false From 0c7a1a45b6e972ee5c6330675b183626f354fbe9 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 12 Jan 2022 10:47:17 +0100 Subject: [PATCH 4/5] [MNG-7160] Avoid magic constants, throw an exception if the value is not supported --- .../BootstrapCoreExtensionManager.java | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java index 207353d93432..bf918f3a1e94 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java @@ -61,6 +61,10 @@ @Named public class BootstrapCoreExtensionManager { + public static final String STRATEGY_PARENT_FIRST = "parent-first"; + public static final String STRATEGY_PLUGIN = "plugin"; + public static final String STRATEGY_SELF_FIRST = "self-first"; + private final Logger log = LoggerFactory.getLogger( getClass() ); private final DefaultPluginDependenciesResolver pluginDependenciesResolver; @@ -126,24 +130,28 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List providedArtifacts = Collections.emptySet(); - if ( "parent-first".equals( extension.getClassLoadingStrategy() ) ) + String classLoadingStrategy = extension.getClassLoadingStrategy(); + if ( STRATEGY_PARENT_FIRST.equals( classLoadingStrategy ) ) { - realm = classWorld.newRealm( realmId, null ); realm.importFrom( parentRealm, "" ); } - else if ( "plugin".equals( extension.getClassLoadingStrategy() ) ) + else if ( STRATEGY_PLUGIN.equals( classLoadingStrategy ) ) { - realm = classWorld.newRealm( realmId, null ); coreExports.getExportedPackages().forEach( ( p, cl ) -> realm.importFrom( cl, p ) ); providedArtifacts = coreExports.getExportedArtifacts(); } - else + else if ( STRATEGY_SELF_FIRST.equals( classLoadingStrategy ) ) { - realm = classWorld.newRealm( realmId, null ); 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 ) { From 929afb17cdfbf71cbb52179fd8aadf5f6951752a Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 17 Jan 2022 08:57:20 +0100 Subject: [PATCH 5/5] Display both id and path --- .../maven/cli/internal/BootstrapCoreExtensionManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java index bf918f3a1e94..f3583e224d35 100644 --- a/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java +++ b/maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java @@ -163,7 +163,7 @@ else if ( STRATEGY_SELF_FIRST.equals( classLoadingStrategy ) ) else { File file = artifact.getFile(); - log.debug( " Included {}", file ); + log.debug( " Included {} located at {}", id, file ); realm.addURL( file.toURI().toURL() ); } }