You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/01/04 15:55:53 UTC
[GitHub] [maven] gnodet opened a new pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
gnodet opened a new pull request #648:
URL: https://github.com/apache/maven/pull/648
Following this checklist to help us incorporate your
contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[MNG-XXX] SUMMARY`, where you replace `MNG-XXX`
and `SUMMARY` with the appropriate JIRA issue. Best practice is to use the JIRA issue
title in the pull request title and in the first line of the commit message.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [ ] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1004949878
Does it make sense to provide an IT for that?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1007229374
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1007215742
> > >
> >
> >
> > I did add an IT with [apache/maven-integration-testing#133](https://github.com/apache/maven-integration-testing/pull/133), but it works locally but not on github CI, not sure why yet...
>
> The test works for me correctly. Though I have noticed that Maven is terminated prematurely. I don't see the `BUILD FAILURE` output since this happens very early in the build. Do you think this can be reasonably improved?
No, the `BUILD FAILURE` and `BUILD SUCCESS` messages are displayed when the session ends. In our case, the exception is thrown before the session is even created.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779831944
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
+ this.extension = extension;
+ }
+
+ public CoreExtension getExtension()
+ {
+ return extension;
+ }
+
+ public static String getId( CoreExtension extension )
+ {
+ StringBuilder id = new StringBuilder( 128 );
+
+ id.append( ( extension.getGroupId() == null ) ? "[unknown-group-id]" : extension.getGroupId() );
Review comment:
I don't see any place where this is checked, so my best guess is that if the user does not fill it, it will be null.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1006984567
> >
>
> I did add an IT with [apache/maven-integration-testing#133](https://github.com/apache/maven-integration-testing/pull/133), but it works locally but not on github CI, not sure why yet...
The test works for me correctly. Though I have noticed that Maven is terminated prematurely. I don't see the `BUILD FAILURE` output since this happens very early in the build. Do you think this can be reasonably improved?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779834038
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java
##########
@@ -128,19 +128,27 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List<Artifa
private List<Artifact> resolveExtension( CoreExtension extension, RepositorySystemSession repoSession,
List<RemoteRepository> repositories, DependencyFilter dependencyFilter )
- throws PluginResolutionException
+ throws ExtensionResolutionException
{
- Plugin plugin = new Plugin();
- plugin.setGroupId( extension.getGroupId() );
- plugin.setArtifactId( extension.getArtifactId() );
- plugin.setVersion( extension.getVersion() );
-
- DependencyNode root =
- pluginDependenciesResolver.resolveCoreExtension( plugin, dependencyFilter, repositories, repoSession );
- PreorderNodeListGenerator nlg = new PreorderNodeListGenerator();
- root.accept( nlg );
- List<Artifact> artifacts = nlg.getArtifacts( false );
-
- return artifacts;
+ try
+ {
+ Plugin plugin = new Plugin();
+ plugin.setGroupId( extension.getGroupId() );
+ plugin.setArtifactId( extension.getArtifactId() );
+ plugin.setVersion( extension.getVersion() );
+
+ DependencyNode root = pluginDependenciesResolver
Review comment:
Fixed
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1004981418
>
I did add an IT, but it works locally but not on github CI, not sure why yet...
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet merged pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet merged pull request #648:
URL: https://github.com/apache/maven/pull/648
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779877572
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
Review comment:
Agreed, make it wrong, but consistent with the rest.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779916524
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
+ this.extension = extension;
+ }
+
+ public CoreExtension getExtension()
+ {
+ return extension;
+ }
+
+ public static String getId( CoreExtension extension )
+ {
+ StringBuilder id = new StringBuilder( 128 );
+
+ id.append( ( extension.getGroupId() == null ) ? "[unknown-group-id]" : extension.getGroupId() );
Review comment:
Fixed
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r778212262
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
##########
@@ -717,75 +718,62 @@ protected void configure()
return Collections.emptyList();
}
- try
+ List<CoreExtension> extensions = readCoreExtensionsDescriptor( extensionsFile );
+ if ( extensions.isEmpty() )
{
- List<CoreExtension> extensions = readCoreExtensionsDescriptor( extensionsFile );
- if ( extensions.isEmpty() )
- {
- return Collections.emptyList();
- }
+ return Collections.emptyList();
+ }
- ContainerConfiguration cc = new DefaultContainerConfiguration() //
- .setClassWorld( cliRequest.classWorld ) //
- .setRealm( containerRealm ) //
- .setClassPathScanning( PlexusConstants.SCANNING_INDEX ) //
- .setAutoWiring( true ) //
- .setJSR250Lifecycle( true ) //
- .setName( "maven" );
+ ContainerConfiguration cc = new DefaultContainerConfiguration() //
+ .setClassWorld( cliRequest.classWorld ) //
+ .setRealm( containerRealm ) //
+ .setClassPathScanning( PlexusConstants.SCANNING_INDEX ) //
+ .setAutoWiring( true ) //
+ .setJSR250Lifecycle( true ) //
+ .setName( "maven" );
- DefaultPlexusContainer container = new DefaultPlexusContainer( cc, new AbstractModule()
+ DefaultPlexusContainer container = new DefaultPlexusContainer( cc, new AbstractModule()
+ {
+ @Override
+ protected void configure()
{
- @Override
- protected void configure()
- {
- bind( ILoggerFactory.class ).toInstance( slf4jLoggerFactory );
- }
- } );
+ bind( ILoggerFactory.class ).toInstance( slf4jLoggerFactory );
+ }
+ } );
- try
- {
- container.setLookupRealm( null );
+ try
+ {
+ container.setLookupRealm( null );
- container.setLoggerManager( plexusLoggerManager );
+ container.setLoggerManager( plexusLoggerManager );
- container.getLoggerManager().setThresholds( cliRequest.request.getLoggingLevel() );
+ container.getLoggerManager().setThresholds( cliRequest.request.getLoggingLevel() );
- Thread.currentThread().setContextClassLoader( container.getContainerRealm() );
+ Thread.currentThread().setContextClassLoader( container.getContainerRealm() );
- executionRequestPopulator = container.lookup( MavenExecutionRequestPopulator.class );
+ executionRequestPopulator = container.lookup( MavenExecutionRequestPopulator.class );
- configurationProcessors = container.lookupMap( ConfigurationProcessor.class );
+ configurationProcessors = container.lookupMap( ConfigurationProcessor.class );
- configure( cliRequest );
+ configure( cliRequest );
- MavenExecutionRequest request = DefaultMavenExecutionRequest.copy( cliRequest.request );
+ MavenExecutionRequest request = DefaultMavenExecutionRequest.copy( cliRequest.request );
- request = populateRequest( cliRequest, request );
+ request = populateRequest( cliRequest, request );
- request = executionRequestPopulator.populateDefaults( request );
+ request = executionRequestPopulator.populateDefaults( request );
- BootstrapCoreExtensionManager resolver = container.lookup( BootstrapCoreExtensionManager.class );
+ BootstrapCoreExtensionManager resolver = container.lookup( BootstrapCoreExtensionManager.class );
- return Collections.unmodifiableList( resolver.loadCoreExtensions( request, providedArtifacts,
- extensions ) );
+ return Collections.unmodifiableList( resolver.loadCoreExtensions( request, providedArtifacts,
+ extensions ) );
- }
- finally
- {
- executionRequestPopulator = null;
- container.dispose();
- }
- }
- catch ( RuntimeException e )
- {
- // runtime exceptions are most likely bugs in maven, let them bubble up to the user
- throw e;
}
- catch ( Exception e )
+ finally
{
- slf4jLogger.warn( "Failed to read extensions descriptor {}: {}", extensionsFile, e.getMessage() );
Review comment:
Insane, that stupid simple!
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779771833
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
+ this.extension = extension;
+ }
+
+ public CoreExtension getExtension()
+ {
+ return extension;
+ }
+
+ public static String getId( CoreExtension extension )
+ {
+ StringBuilder id = new StringBuilder( 128 );
+
+ id.append( ( extension.getGroupId() == null ) ? "[unknown-group-id]" : extension.getGroupId() );
Review comment:
Do we expect them to be be null??
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet merged pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet merged pull request #648:
URL: https://github.com/apache/maven/pull/648
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779877025
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
+ this.extension = extension;
+ }
+
+ public CoreExtension getExtension()
+ {
+ return extension;
+ }
+
+ public static String getId( CoreExtension extension )
+ {
+ StringBuilder id = new StringBuilder( 128 );
+
+ id.append( ( extension.getGroupId() == null ) ? "[unknown-group-id]" : extension.getGroupId() );
Review comment:
I wonder what the resolver will say when the input in non-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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779832412
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
Review comment:
Right, but if the interface is enhanced, the exception will be moved along the new method I suppose.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779900286
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
+ this.extension = extension;
+ }
+
+ public CoreExtension getExtension()
+ {
+ return extension;
+ }
+
+ public static String getId( CoreExtension extension )
+ {
+ StringBuilder id = new StringBuilder( 128 );
+
+ id.append( ( extension.getGroupId() == null ) ? "[unknown-group-id]" : extension.getGroupId() );
Review comment:
I suppose an exception will be thrown. Fwiw, the `getId()` is simply copied from the `Plugin` class and that's what is used in the `PluginResolutionException`.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1007215742
> > >
> >
> >
> > I did add an IT with [apache/maven-integration-testing#133](https://github.com/apache/maven-integration-testing/pull/133), but it works locally but not on github CI, not sure why yet...
>
> The test works for me correctly. Though I have noticed that Maven is terminated prematurely. I don't see the `BUILD FAILURE` output since this happens very early in the build. Do you think this can be reasonably improved?
No, the `BUILD FAILURE` and `BUILD SUCCESS` messages are displayed when the session ends. In our case, the exception is thrown before the session is even created.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1007229930
Please apply to master as well.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet edited a comment on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet edited a comment on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1004981418
>
I did add an IT with https://github.com/apache/maven-integration-testing/pull/133, but it works locally but not on github CI, not sure why yet...
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779772113
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
Review comment:
I guess this duplicated the previous exception, but duplicating the cause message isn't right.
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java
##########
@@ -128,19 +128,27 @@ private CoreExtensionEntry createExtension( CoreExtension extension, List<Artifa
private List<Artifact> resolveExtension( CoreExtension extension, RepositorySystemSession repoSession,
List<RemoteRepository> repositories, DependencyFilter dependencyFilter )
- throws PluginResolutionException
+ throws ExtensionResolutionException
{
- Plugin plugin = new Plugin();
- plugin.setGroupId( extension.getGroupId() );
- plugin.setArtifactId( extension.getArtifactId() );
- plugin.setVersion( extension.getVersion() );
-
- DependencyNode root =
- pluginDependenciesResolver.resolveCoreExtension( plugin, dependencyFilter, repositories, repoSession );
- PreorderNodeListGenerator nlg = new PreorderNodeListGenerator();
- root.accept( nlg );
- List<Artifact> artifacts = nlg.getArtifacts( false );
-
- return artifacts;
+ try
+ {
+ Plugin plugin = new Plugin();
+ plugin.setGroupId( extension.getGroupId() );
+ plugin.setArtifactId( extension.getArtifactId() );
+ plugin.setVersion( extension.getVersion() );
+
+ DependencyNode root = pluginDependenciesResolver
Review comment:
Can you add a TODO that the interface likely needs to change to receive `CoreExtension` directly?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
gnodet commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779832412
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
Review comment:
Right, but if the PluginDependenciesResolver interface is enhanced, the exception will be moved along the new method I suppose.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #648:
URL: https://github.com/apache/maven/pull/648#discussion_r779906259
##########
File path: maven-embedder/src/main/java/org/apache/maven/cli/internal/ExtensionResolutionException.java
##########
@@ -0,0 +1,59 @@
+package org.apache.maven.cli.internal;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.cli.internal.extension.model.CoreExtension;
+
+/**
+ * Exception occurring trying to resolve a plugin.
+ *
+ * @author <a href="mailto:brett@apache.org">Brett Porter</a>
+ */
+public class ExtensionResolutionException
+ extends Exception
+{
+
+ private final CoreExtension extension;
+
+ public ExtensionResolutionException( CoreExtension extension, Throwable cause )
+ {
+ super( "Extension " + getId( extension ) + " or one of its dependencies could not be resolved: "
+ + cause.getMessage(), cause );
+ this.extension = extension;
+ }
+
+ public CoreExtension getExtension()
+ {
+ return extension;
+ }
+
+ public static String getId( CoreExtension extension )
+ {
+ StringBuilder id = new StringBuilder( 128 );
+
+ id.append( ( extension.getGroupId() == null ) ? "[unknown-group-id]" : extension.getGroupId() );
Review comment:
Then why not have it generated by Modello just like for `Plugin`?
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #648: [MNG-6326] Make the build fail if core extensions can not be loaded
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #648:
URL: https://github.com/apache/maven/pull/648#issuecomment-1007229374
> > > >
> > >
> > >
> > > I did add an IT with [apache/maven-integration-testing#133](https://github.com/apache/maven-integration-testing/pull/133), but it works locally but not on github CI, not sure why yet...
> >
> >
> > The test works for me correctly. Though I have noticed that Maven is terminated prematurely. I don't see the `BUILD FAILURE` output since this happens very early in the build. Do you think this can be reasonably improved?
>
> No, the `BUILD FAILURE` and `BUILD SUCCESS` messages are displayed when the session ends. In our case, the exception is thrown before the session is even created.
Makes sense. Fail early, let's leave it like that.
--
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-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org