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/13 08:57:29 UTC
[GitHub] [maven] gnodet opened a new pull request #657: [MNG-7349] Superfluous relocation warning messages
gnodet opened a new pull request #657:
URL: https://github.com/apache/maven/pull/657
JIRA issue: https://issues.apache.org/jira/browse/MNG-7349
--
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 #657: [3.8.x][MNG-7349] Limit relocation warning message to direct dependencies only
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #657:
URL: https://github.com/apache/maven/pull/657#issuecomment-1014598664
Stupid question: Will it also apply to extensions?
--
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 #657: [3.8.x][MNG-7349] Limit relocation warning message to direct dependencies only
Posted by GitBox <gi...@apache.org>.
gnodet commented on pull request #657:
URL: https://github.com/apache/maven/pull/657#issuecomment-1014666813
> Stupid question: Will it also apply to extensions?
Yes, both boot and build extensions use the `DefaultPluginDependenciesResolver`, so that should be fine.
--
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] hboutemy commented on a change in pull request #657: [3.8.x][MNG-7349] Limit relocation warning message to direct dependencies only
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #657:
URL: https://github.com/apache/maven/pull/657#discussion_r789480339
##########
File path: maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
##########
@@ -205,6 +205,22 @@ private DependencyNode resolveInternal( Plugin plugin, Artifact pluginArtifact,
node = repoSystem.collectDependencies( pluginSession, request ).getRoot();
+ if ( logger.isWarnEnabled() )
+ {
+ for ( DependencyNode child : node.getChildren() )
+ {
+ if ( !child.getRelocations().isEmpty() )
+ {
+ org.eclipse.aether.artifact.Artifact relocated = child.getDependency().getArtifact();
+ String message = relocated instanceof org.apache.maven.repository.internal.RelocatedArtifact
+ ? ( ( org.apache.maven.repository.internal.RelocatedArtifact ) relocated ).getMessage()
+ : null;
+ logger.warn( "The artifact " + child.getRelocations().get( 0 ) + " has been relocated to "
+ + relocated + ( message != null ? ": " + message : "" ) );
+ }
+ }
+ }
+
Review comment:
by definition, aren't plugin dependencies transitives (of the plugin)?
shouldn't those warnings finally be limited to current reactor projects AND only on transitive dependencies (and fully ignore plugins: too late to fix anything)?
--
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] hboutemy commented on a change in pull request #657: [3.8.x][MNG-7349] Limit relocation warning message to direct dependencies only
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #657:
URL: https://github.com/apache/maven/pull/657#discussion_r789481375
##########
File path: maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/RelocatedArtifact.java
##########
@@ -29,7 +29,7 @@
/**
* @author Benjamin Bentmann
*/
-final class RelocatedArtifact
+public final class RelocatedArtifact
Review comment:
why switching to public this class from an internal package?
is there a real intent or just a unintended commit from local tests?
--
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 #657: [3.8.x][MNG-7349] Limit relocation warning message to direct dependencies only
Posted by GitBox <gi...@apache.org>.
gnodet commented on a change in pull request #657:
URL: https://github.com/apache/maven/pull/657#discussion_r789505567
##########
File path: maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
##########
@@ -205,6 +205,22 @@ private DependencyNode resolveInternal( Plugin plugin, Artifact pluginArtifact,
node = repoSystem.collectDependencies( pluginSession, request ).getRoot();
+ if ( logger.isWarnEnabled() )
+ {
+ for ( DependencyNode child : node.getChildren() )
+ {
+ if ( !child.getRelocations().isEmpty() )
+ {
+ org.eclipse.aether.artifact.Artifact relocated = child.getDependency().getArtifact();
+ String message = relocated instanceof org.apache.maven.repository.internal.RelocatedArtifact
+ ? ( ( org.apache.maven.repository.internal.RelocatedArtifact ) relocated ).getMessage()
+ : null;
+ logger.warn( "The artifact " + child.getRelocations().get( 0 ) + " has been relocated to "
+ + relocated + ( message != null ? ": " + message : "" ) );
+ }
+ }
+ }
+
Review comment:
Good catch, I've been fooled by the not strict enough testing I added in https://github.com/apache/maven-integration-testing/pull/134/files. It's testing the plugin's first level dependencies relocations instead of the plugin itself.
--
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 edited a comment on pull request #657: [MNG-7349] Superfluous relocation warning messages
Posted by GitBox <gi...@apache.org>.
michael-o edited a comment on pull request #657:
URL: https://github.com/apache/maven/pull/657#issuecomment-1012043722
If you plan to add an IT for that, I have recently added a test for relocated plugins and their config. You can reuse (copy) that and test it.
--
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 #657: [MNG-7349] Superfluous relocation warning messages
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #657:
URL: https://github.com/apache/maven/pull/657#discussion_r783864534
##########
File path: maven-core/src/main/java/org/apache/maven/project/DefaultProjectDependenciesResolver.java
##########
@@ -181,6 +181,23 @@ public DependencyResolutionResult resolve( DependencyResolutionRequest request )
depRequest.setRoot( node );
+ if ( logger.isWarnEnabled() )
+ {
+ for ( DependencyNode child : node.getChildren() )
+ {
+ if ( !child.getRelocations().isEmpty() )
+ {
+ org.eclipse.aether.artifact.Artifact relocated = child.getDependency().getArtifact();
+ String message = relocated instanceof org.apache.maven.repository.internal.RelocatedArtifact
+ ? ( ( org.apache.maven.repository.internal.RelocatedArtifact ) relocated ).getMessage()
+ : null;
+ logger.warn( "The artifact " + child.getRelocations().get( 0 ) + " has been relocated to "
+ + child.getDependency().getArtifact()
Review comment:
`child.getDependency().getArtifact()` is already available as `relocated`
--
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 #657: [MNG-7349] Superfluous relocation warning messages
Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #657:
URL: https://github.com/apache/maven/pull/657#issuecomment-1012043722
If you plan do add an IT for that, I have recently added a test for relocated plugins and their config. You can reuse (copy) that and test it.
--
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] hboutemy commented on a change in pull request #657: [3.8.x][MNG-7349] Limit relocation warning message to direct dependencies only
Posted by GitBox <gi...@apache.org>.
hboutemy commented on a change in pull request #657:
URL: https://github.com/apache/maven/pull/657#discussion_r789482569
##########
File path: maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/RelocatedArtifact.java
##########
@@ -29,7 +29,7 @@
/**
* @author Benjamin Bentmann
*/
-final class RelocatedArtifact
+public final class RelocatedArtifact
Review comment:
ok, I saw the reference required in DefaultProjectDependenciesResolver: I answered to my question, it is needed for this improvement...
--
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 #657: [3.8.x][MNG-7349] Limit relocation warning message to direct dependencies only
Posted by GitBox <gi...@apache.org>.
gnodet merged pull request #657:
URL: https://github.com/apache/maven/pull/657
--
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