Skip to content

[MNG-7572] Allow plugin descriptor to require loading extensions #833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
kwin wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

[MNG-7572] Allow plugin descriptor to require loading extensions #833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
20fe5ec
[MNG-7572] Allow plugin descriptor to require loading extensions
kwin Oct 18, 2022
f76df3c
only request main artifact of plugins which potentially have plugins
kwin Nov 18, 2022
21d23c3
fix another test
kwin Nov 18, 2022
3515e5b
disable broken legacy compat tests
kwin Nov 18, 2022
c15a792
only get main JAR for retrieving plugin descriptor (without resolving
kwin Nov 19, 2022
c21dee4
fix condition
kwin Nov 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...compat/src/test/java/org/apache/maven/project/inheritance/t02/ProjectInheritanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.maven.model.PluginExecution;
import org.apache.maven.project.MavenProject;
import org.apache.maven.project.inheritance.AbstractProjectInheritanceTestCase;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -64,6 +65,7 @@ public class ProjectInheritanceTest
// ----------------------------------------------------------------------

@Test
@Disabled("Cannot resolve plugin main artifact necessary due to MNG-7572")
public void testProjectInheritance()
throws Exception
{
Expand Down
2 changes: 2 additions & 0 deletions ...compat/src/test/java/org/apache/maven/project/inheritance/t03/ProjectInheritanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import org.apache.maven.project.MavenProject;
import org.apache.maven.project.inheritance.AbstractProjectInheritanceTestCase;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -53,6 +54,7 @@ public class ProjectInheritanceTest
// ----------------------------------------------------------------------

@Test
@Disabled("Cannot resolve plugin main artifact necessary due to MNG-7572")
public void testProjectInheritance()
throws Exception
{
Expand Down
23 changes: 21 additions & 2 deletions maven-core/src/main/java/org/apache/maven/plugin/MavenPluginManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/

import java.util.List;
import java.util.Optional;

import org.apache.maven.execution.MavenSession;
import org.apache.maven.model.Plugin;
Expand Down Expand Up @@ -94,12 +95,30 @@ void setupPluginRealm( PluginDescriptor pluginDescriptor, MavenSession session,
throws PluginResolutionException, PluginContainerException;

/**
* Sets up class realm for the specified build extensions plugin.
* Sets up class realm for the specified build extensions plugin
* or regular plugin containing extensions.
*
* @since 3.3.0
*
* @deprecated Use {@link #setupExtensionsRealm(MavenProject, Plugin, RepositorySystemSession, Boolean)} instead.
*/
ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject project, Plugin plugin,
@Deprecated
default ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject project, Plugin plugin,
RepositorySystemSession session )
throws PluginManagerException
{
return setupExtensionsRealm( project, plugin, session, null ).get();
}


/**
* Sets up class realm for the specified build extensions plugin
* or regular plugin containing extensions.
*
* @since 4.0.0
*/
Optional<ExtensionRealmCache.CacheRecord> setupExtensionsRealm( MavenProject project, Plugin plugin,
RepositorySystemSession session, Boolean isBuildExtension )
throws PluginManagerException;

/**
Expand Down
56 changes: 43 additions & 13 deletions maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultMavenPluginManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.jar.JarFile;
import java.util.stream.Collectors;
import java.util.zip.ZipEntry;
Expand Down Expand Up @@ -348,13 +349,13 @@ public synchronized void setupPluginRealm( PluginDescriptor pluginDescriptor, Ma
Plugin plugin = pluginDescriptor.getPlugin();
MavenProject project = session.getCurrentProject();

if ( plugin.isExtensions() )
if ( plugin.isExtensions() || pluginDescriptor.isHasExtensions() )
{
ExtensionRealmCache.CacheRecord extensionRecord;
try
{
RepositorySystemSession repositorySession = session.getRepositorySession();
extensionRecord = setupExtensionsRealm( project, plugin, repositorySession );
extensionRecord = setupExtensionsRealm( project, plugin, repositorySession, false ).get();
}
catch ( PluginManagerException e )
{
Expand Down Expand Up @@ -834,8 +835,10 @@ public void releaseMojo( Object mojo, MojoExecution mojoExecution )
}
}

public ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject project, Plugin plugin,
RepositorySystemSession session )
@Override
public Optional<ExtensionRealmCache.CacheRecord> setupExtensionsRealm( MavenProject project, Plugin plugin,
RepositorySystemSession session,
Boolean isBuildExtension )
throws PluginManagerException
{
@SuppressWarnings( "unchecked" ) Map<String, ExtensionRealmCache.CacheRecord> pluginRealms =
Expand All @@ -851,7 +854,7 @@ public ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject projec
ExtensionRealmCache.CacheRecord extensionRecord = pluginRealms.get( pluginKey );
if ( extensionRecord != null )
{
return extensionRecord;
return Optional.of( extensionRecord );
}

final List<RemoteRepository> repositories = project.getRemotePluginRepositories();
Expand All @@ -876,6 +879,7 @@ public ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject projec
List<Artifact> artifacts;
PluginArtifactsCache.Key cacheKey = pluginArtifactsCache.createKey( plugin, null, repositories, session );
PluginArtifactsCache.CacheRecord recordArtifacts;
PluginDescriptor pluginDescriptor = null;
try
{
recordArtifacts = pluginArtifactsCache.get( cacheKey );
Expand All @@ -892,6 +896,25 @@ public ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject projec
{
try
{
// potentially a plugin with extensions?
if ( !plugin.isExtensions() && isBuildExtension == Boolean.FALSE )
{
Artifact pluginMainArtifact = resolveExtensionMainArtifact( plugin, repositories, session );
Copy link
Copy Markdown
Member Author

@kwin kwin Jun 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to deal with exceptions here in case the plugin artifact cannot be found?

try
{
pluginDescriptor = extractPluginDescriptor( pluginMainArtifact, plugin );
// for backwards compatibility reason never return empty when isBuildExtension == null
if ( isBuildExtension != null && !plugin.isExtensions() && !pluginDescriptor.isHasExtensions() )
{
return Optional.empty();
}
}
catch ( PluginDescriptorParsingException | InvalidPluginDescriptorException e )
{
throw new PluginManagerException( plugin, e.getMessage(), e );
}
}

artifacts = resolveExtensionArtifacts( plugin, repositories, session );
recordArtifacts = pluginArtifactsCache.put( cacheKey, artifacts );
}
Expand All @@ -909,13 +932,8 @@ public ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject projec
extensionRecord = extensionRealmCache.get( extensionKey );
if ( extensionRecord == null )
{
ClassRealm extensionRealm =
classRealmManager.createExtensionRealm( plugin, toAetherArtifacts( artifacts ) );

// TODO figure out how to use the same PluginDescriptor when running mojos

PluginDescriptor pluginDescriptor = null;
if ( plugin.isExtensions() && !artifacts.isEmpty() )
boolean requirePluginDescriptor = plugin.isExtensions() || isBuildExtension == Boolean.FALSE;
if ( requirePluginDescriptor && !artifacts.isEmpty() && pluginDescriptor == null )
{
// ignore plugin descriptor parsing errors at this point
// these errors will reported during calculation of project build execution plan
Expand All @@ -928,7 +946,11 @@ public ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject projec
// ignore, see above
}
}

ClassRealm extensionRealm =
classRealmManager.createExtensionRealm( plugin, toAetherArtifacts( artifacts ) );

// TODO figure out how to use the same PluginDescriptor when running mojos
discoverPluginComponents( extensionRealm, plugin, pluginDescriptor );

ExtensionDescriptor extensionDescriptor = null;
Expand All @@ -954,7 +976,15 @@ public ExtensionRealmCache.CacheRecord setupExtensionsRealm( MavenProject projec
extensionRealmCache.register( project, extensionKey, extensionRecord );
pluginRealms.put( pluginKey, extensionRecord );

return extensionRecord;
return Optional.of( extensionRecord );
}

private Artifact resolveExtensionMainArtifact( Plugin extensionPlugin, List<RemoteRepository> repositories,
RepositorySystemSession session )
throws PluginResolutionException
{
return RepositoryUtils.toArtifact( pluginDependenciesResolver.resolveArtifactOnly( extensionPlugin,
repositories, session ) );
}

private List<Artifact> resolveExtensionArtifacts( Plugin extensionPlugin, List<RemoteRepository> repositories,
Expand Down
25 changes: 20 additions & 5 deletions ...ore/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,23 +130,38 @@ public Artifact resolve( Plugin plugin, List<RemoteRepository> repositories, Rep
props.put( "requiredMavenVersion", requiredMavenVersion );
pluginArtifact = pluginArtifact.setProperties( props );
}
return internalResolveArtifactOnly( trace, pluginArtifact, repositories, session );
}
catch ( ArtifactDescriptorException e )
catch ( ArtifactDescriptorException | ArtifactResolutionException e )
{
throw new PluginResolutionException( plugin, e );
}

try
}

@Override
public Artifact resolveArtifactOnly( Plugin plugin, List<RemoteRepository> repositories,
RepositorySystemSession session ) throws PluginResolutionException
{
RequestTrace trace = RequestTrace.newChild( null, plugin );

Artifact pluginArtifact = toArtifact( plugin, session );
try
{
ArtifactRequest request = new ArtifactRequest( pluginArtifact, repositories, REPOSITORY_CONTEXT );
request.setTrace( trace );
pluginArtifact = repoSystem.resolveArtifact( session, request ).getArtifact();
return internalResolveArtifactOnly( trace, pluginArtifact, repositories, session );
}
catch ( ArtifactResolutionException e )
{
throw new PluginResolutionException( plugin, e );
}
}

private Artifact internalResolveArtifactOnly( RequestTrace trace, Artifact pluginArtifact,
List<RemoteRepository> repositories, RepositorySystemSession session ) throws ArtifactResolutionException
{
ArtifactRequest request = new ArtifactRequest( pluginArtifact, repositories, REPOSITORY_CONTEXT );
request.setTrace( trace );
pluginArtifact = repoSystem.resolveArtifact( session, request ).getArtifact();
return pluginArtifact;
}

Expand Down
13 changes: 13 additions & 0 deletions maven-core/src/main/java/org/apache/maven/plugin/internal/PluginDependenciesResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,17 @@ DependencyNode resolve( Plugin plugin, Artifact pluginArtifact, DependencyFilter
List<RemoteRepository> repositories, RepositorySystemSession session )
throws PluginResolutionException;

/**
* Resolves the main artifact of the specified plugin only (without getting the artifact descriptor,
* i.e. getting the effective pom.xml).
* @param plugin
* @param repositories
* @param session
* @return
* @throws PluginResolutionException
* @since 4.0.0
*/
Artifact resolveArtifactOnly( Plugin plugin, List<RemoteRepository> repositories, RepositorySystemSession session )
throws PluginResolutionException;

}
69 changes: 38 additions & 31 deletions maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import javax.inject.Inject;
Expand Down Expand Up @@ -168,27 +169,19 @@ public synchronized ProjectRealmCache.CacheRecord createProjectRealm( MavenProje
plugin.setGroupId( extension.getGroupId() );
plugin.setArtifactId( extension.getArtifactId() );
plugin.setVersion( extension.getVersion() );
// abuse string type of extensions in order to distinguish regular plugin from build extensions
plugin.setExtensions( "build" );
extensionPlugins.add( plugin );
}

for ( Plugin plugin : build.getPlugins() )
{
if ( plugin.isExtensions() )
{
extensionPlugins.add( plugin );
}
// potentially all plugins may carry extensions, filtering happens in
// MavenPluginManager#setupExtensionsRealms as this requires evaluation of plugin descriptor
extensionPlugins.add( plugin );
}
}

if ( extensionPlugins.isEmpty() )
{
if ( logger.isDebugEnabled() )
{
logger.debug( "Extension realms for project " + model.getId() + ": (none)" );
}

return new ProjectRealmCache.CacheRecord( null, null );
}

List<ClassRealm> extensionRealms = new ArrayList<>();

Expand All @@ -200,31 +193,45 @@ public synchronized ProjectRealmCache.CacheRecord createProjectRealm( MavenProje

for ( Plugin plugin : extensionPlugins )
{
ExtensionRealmCache.CacheRecord recordRealm =
pluginManager.setupExtensionsRealm( project, plugin, request.getRepositorySession() );

final ClassRealm extensionRealm = recordRealm.getRealm();
final ExtensionDescriptor extensionDescriptor = recordRealm.getDescriptor();
final List<Artifact> artifacts = recordRealm.getArtifacts();
Optional<ExtensionRealmCache.CacheRecord> recordRealm =
pluginManager.setupExtensionsRealm( project, plugin, request.getRepositorySession(),
"build".equals( plugin.getExtensions() ) );

extensionRealms.add( extensionRealm );
if ( extensionDescriptor != null )
if ( recordRealm.isPresent() )
{
exportedPackages.put( extensionRealm, extensionDescriptor.getExportedPackages() );
exportedArtifacts.put( extensionRealm, extensionDescriptor.getExportedArtifacts() );
final ClassRealm extensionRealm = recordRealm.get().getRealm();
final ExtensionDescriptor extensionDescriptor = recordRealm.get().getDescriptor();
final List<Artifact> artifacts = recordRealm.get().getArtifacts();

extensionRealms.add( extensionRealm );
if ( extensionDescriptor != null )
{
exportedPackages.put( extensionRealm, extensionDescriptor.getExportedPackages() );
exportedArtifacts.put( extensionRealm, extensionDescriptor.getExportedArtifacts() );
}

if ( !plugin.isExtensions() && artifacts.size() == 1 && artifacts.get( 0 ).getFile() != null )
{
/*
* This is purely for backward-compat with 2.x where <extensions> consisting of a single artifact
* were loaded into the core and hence available to plugins, in contrast to bigger extensions that
* were loaded into a dedicated realm which is invisible to plugins (MNG-2749).
*/
publicArtifacts.addAll( artifacts );
}
}
}

if ( !plugin.isExtensions() && artifacts.size() == 1 && artifacts.get( 0 ).getFile() != null )
if ( extensionRealms.isEmpty() )
{
if ( logger.isDebugEnabled() )
{
/*
* This is purely for backward-compat with 2.x where <extensions> consisting of a single artifact where
* loaded into the core and hence available to plugins, in contrast to bigger extensions that were
* loaded into a dedicated realm which is invisible to plugins (MNG-2749).
*/
publicArtifacts.addAll( artifacts );
logger.debug( "Extension realms for project " + model.getId() + ": (none)" );
}
}

return new ProjectRealmCache.CacheRecord( null, null );
}

if ( logger.isDebugEnabled() )
{
logger.debug( "Extension realms for project " + model.getId() + ": " + extensionRealms );
Expand Down
6 changes: 4 additions & 2 deletions maven-core/src/test/java/org/apache/maven/AbstractCoreMavenComponentTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.apache.maven.project.DefaultProjectBuildingRequest;
import org.apache.maven.project.MavenProject;
import org.apache.maven.project.ProjectBuildingRequest;
import org.apache.maven.project.ProjectBuildingRequest.RepositoryMerging;
import org.apache.maven.repository.RepositorySystem;
import org.apache.maven.repository.internal.MavenRepositorySystemUtils;
import org.codehaus.plexus.PlexusContainer;
Expand Down Expand Up @@ -131,9 +132,12 @@ protected MavenSession createMavenSession( File pom, Properties executionPropert
.setLocalRepository( request.getLocalRepository() )
.setRemoteRepositories( request.getRemoteRepositories() )
.setPluginArtifactRepositories( request.getPluginArtifactRepositories() )
// make sure to always resolve from test remote repo (and not from repo from super pom) due to usage of TestRepositoryConnector
.setRepositoryMerging( RepositoryMerging.REQUEST_DOMINANT )
.setSystemProperties( executionProperties )
.setUserProperties( new Properties() );

initRepoSession( configuration );
List<MavenProject> projects = new ArrayList<>();

if ( pom != null )
Expand Down Expand Up @@ -162,8 +166,6 @@ protected MavenSession createMavenSession( File pom, Properties executionPropert
projects.add( project );
}

initRepoSession( configuration );

MavenSession session =
new MavenSession( getContainer(), configuration.getRepositorySession(), request,
new DefaultMavenExecutionResult() );
Expand Down
Loading