michael-o commented on a change in pull request #665: URL: https://github.com/apache/maven/pull/665#discussion_r805187207
########## File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java ########## @@ -127,30 +134,39 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List<Artifa } private List<Artifact> resolveExtension( CoreExtension extension, RepositorySystemSession repoSession, - List<RemoteRepository> repositories, DependencyFilter dependencyFilter ) + List<RemoteRepository> repositories, DependencyFilter dependencyFilter, + Interpolator interpolator ) throws ExtensionResolutionException { try { // TODO: enhance the PluginDependenciesResolver to provide a - // TODO: resolveCoreExtension method which uses a CoreExtension - // TODO: object instead of a Plugin as this makes no sense + // TODO: resolveCoreExtension method which uses a CoreExtension + // TODO: object instead of a Plugin as this makes no sense Plugin plugin = new Plugin(); - plugin.setGroupId( extension.getGroupId() ); - plugin.setArtifactId( extension.getArtifactId() ); - plugin.setVersion( extension.getVersion() ); + plugin.setGroupId( interpolator.interpolate( extension.getGroupId() ) ); + plugin.setArtifactId( interpolator.interpolate( extension.getArtifactId() ) ); + plugin.setVersion( interpolator.interpolate( extension.getVersion() ) ); - DependencyNode root = pluginDependenciesResolver - .resolveCoreExtension( plugin, dependencyFilter, repositories, repoSession ); + DependencyNode root = + pluginDependenciesResolver.resolveCoreExtension( plugin, dependencyFilter, repositories, repoSession ); PreorderNodeListGenerator nlg = new PreorderNodeListGenerator(); root.accept( nlg ); return nlg.getArtifacts( false ); } - catch ( PluginResolutionException e ) + catch ( PluginResolutionException | InterpolationException e ) { throw new ExtensionResolutionException( extension, e.getCause() ); } Review comment: Is it guaranteed that `InterpolationException` will have a cause? I guess information will be swallowed here. ########## File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java ########## @@ -127,30 +134,39 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List<Artifa } private List<Artifact> resolveExtension( CoreExtension extension, RepositorySystemSession repoSession, - List<RemoteRepository> repositories, DependencyFilter dependencyFilter ) + List<RemoteRepository> repositories, DependencyFilter dependencyFilter, + Interpolator interpolator ) throws ExtensionResolutionException { try { // TODO: enhance the PluginDependenciesResolver to provide a - // TODO: resolveCoreExtension method which uses a CoreExtension - // TODO: object instead of a Plugin as this makes no sense + // TODO: resolveCoreExtension method which uses a CoreExtension + // TODO: object instead of a Plugin as this makes no sense Plugin plugin = new Plugin(); - plugin.setGroupId( extension.getGroupId() ); - plugin.setArtifactId( extension.getArtifactId() ); - plugin.setVersion( extension.getVersion() ); + plugin.setGroupId( interpolator.interpolate( extension.getGroupId() ) ); + plugin.setArtifactId( interpolator.interpolate( extension.getArtifactId() ) ); + plugin.setVersion( interpolator.interpolate( extension.getVersion() ) ); - DependencyNode root = pluginDependenciesResolver - .resolveCoreExtension( plugin, dependencyFilter, repositories, repoSession ); + DependencyNode root = + pluginDependenciesResolver.resolveCoreExtension( plugin, dependencyFilter, repositories, repoSession ); PreorderNodeListGenerator nlg = new PreorderNodeListGenerator(); root.accept( nlg ); return nlg.getArtifacts( false ); } - catch ( PluginResolutionException e ) + catch ( PluginResolutionException | InterpolationException e ) { throw new ExtensionResolutionException( extension, e.getCause() ); } Review comment: I do agree, but that should be done separately, outside of this PR. ########## File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java ########## @@ -127,30 +134,39 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List<Artifa } private List<Artifact> resolveExtension( CoreExtension extension, RepositorySystemSession repoSession, - List<RemoteRepository> repositories, DependencyFilter dependencyFilter ) + List<RemoteRepository> repositories, DependencyFilter dependencyFilter, + Interpolator interpolator ) throws ExtensionResolutionException { try { // TODO: enhance the PluginDependenciesResolver to provide a - // TODO: resolveCoreExtension method which uses a CoreExtension - // TODO: object instead of a Plugin as this makes no sense + // TODO: resolveCoreExtension method which uses a CoreExtension + // TODO: object instead of a Plugin as this makes no sense Plugin plugin = new Plugin(); - plugin.setGroupId( extension.getGroupId() ); - plugin.setArtifactId( extension.getArtifactId() ); - plugin.setVersion( extension.getVersion() ); + plugin.setGroupId( interpolator.interpolate( extension.getGroupId() ) ); + plugin.setArtifactId( interpolator.interpolate( extension.getArtifactId() ) ); + plugin.setVersion( interpolator.interpolate( extension.getVersion() ) ); - DependencyNode root = pluginDependenciesResolver - .resolveCoreExtension( plugin, dependencyFilter, repositories, repoSession ); + DependencyNode root = + pluginDependenciesResolver.resolveCoreExtension( plugin, dependencyFilter, repositories, repoSession ); PreorderNodeListGenerator nlg = new PreorderNodeListGenerator(); root.accept( nlg ); return nlg.getArtifacts( false ); } - catch ( PluginResolutionException e ) + catch ( PluginResolutionException | InterpolationException e ) { throw new ExtensionResolutionException( extension, e.getCause() ); } Review comment: Yes, add two please. -- 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