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 2023/01/03 21:40:54 UTC
[GitHub] [maven-integration-testing] psiroky opened a new pull request, #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
psiroky opened a new pull request, #223:
URL: https://github.com/apache/maven-integration-testing/pull/223
This fixes these kinds of warnings:
```
[INFO] --- maven-plugin-plugin:3.6.4:descriptor (default-descriptor) @ maven-it-plugin-bootstrap ---
[ERROR] Some dependencies of Maven Plugins are expected to be in provided scope.
Please make sure that dependencies listed below declared in POM
have set '<scope>provided</scope>' as well.The following dependencies are in wrong scope:
* org.apache.maven:maven-plugin-api:jar:3.8.6:compile
* org.apache.maven:maven-model:jar:3.8.6:compile
* org.apache.maven:maven-artifact:jar:3.8.6:compile
* org.apache.maven:maven-core:jar:3.8.6:compile
* org.apache.maven:maven-settings:jar:3.8.6:compile
* org.apache.maven:maven-settings-builder:jar:3.8.6:compile
* org.apache.maven:maven-builder-support:jar:3.8.6:compile
* org.apache.maven:maven-repository-metadata:jar:3.8.6:compile
* org.apache.maven:maven-model-builder:jar:3.8.6:compile
* org.apache.maven:maven-resolver-provider:jar:3.8.6:compile
```
--
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-integration-testing] psiroky commented on a diff in pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#discussion_r1060979574
##########
core-it-support/maven-it-plugin-bootstrap/src/main/java/org/apache/maven/its/bootstrap/DownloadMojo.java:
##########
@@ -132,7 +131,7 @@ static Dependency toDependency( String artifact )
throws MojoFailureException
{
Dependency coordinate = new Dependency();
- String[] tokens = StringUtils.split( artifact, ":" );
Review Comment:
Similar as above to avoid adding direct dependency on `plexus-utils`
--
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-integration-testing] psiroky commented on pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#issuecomment-1375506665
Hello @slawekjaranowski! Just checking in to see how to proceed with these clean-up PRs (this one and others with deprecated verifier methods).
Would you have time to take a look at these? Or should I perhaps e-mail to the dev list to ask for reviews?
I don't want to pressure you or anything, just trying to make sure the PRs won't be forgotten. If you plan to take a look in the future and just don't have time right now, that's of course 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-integration-testing] psiroky commented on a diff in pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#discussion_r1060978463
##########
core-it-support/core-it-plugins/maven-it-plugin-settings/src/main/java/org/apache/maven/plugin/coreit/SettingsReadItMojo.java:
##########
@@ -59,22 +58,14 @@ public void execute()
dumpFile.delete();
}
dumpFile.getParentFile().mkdirs();
- FileWriter fw = null;
- try
+ try ( FileWriter fw = new FileWriter( dumpFile ) )
{
- fw = new FileWriter( dumpFile );
SettingsXpp3Writer writer = new SettingsXpp3Writer();
writer.write( fw, settings );
}
catch ( IOException e )
{
throw new MojoExecutionException( e.getMessage(), e );
}
- finally
- {
- IOUtil.close( fw );
- }
}
Review Comment:
These simple changes are to avoid the direct dependency on plexus-utils in this plugin (as it is brought in by one the maven deps, which are now in provided scope - and thus pluxus-utils ends up in provided scope 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-integration-testing] psiroky commented on pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#issuecomment-1371036304
Yeah, that is the case in one of the plugins here as well, hence the exclusions.
The `dependencyManagement` solution is quite elegant though as it means way less changes and we can also get rid of the exclusions. I don't have a strong preference at this point, so I will leave the current solution. If someone feels strongly about this I can use the other way with dep management.
--
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-integration-testing] psiroky commented on pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
psiroky commented on PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#issuecomment-1370261138
Once I had these changes done I noticed @slawekjaranowski solved this a bit differently in `maven-enforcer` (https://github.com/apache/maven-enforcer/commit/74228deccd43ec88dadf59fa70d98dfd5ead33c5), by adding the `<scope>provided</scope> into `dependencyManagement`.
For some reason I thought that having `scope` inside `dependencyManagement` is not a good practice (with the exception of BOM `import`) as it should just define the version and then modules using that dependency should decide which scope is appropriate for that specific use case. That being said, in this case it actually makes sense as we want these always provided for the IT plugins. If preferred, I can amend the PR with similar changes leveraging `dependencyManagement`.
--
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-integration-testing] psiroky commented on a diff in pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#discussion_r1060979196
##########
core-it-support/core-it-plugins/maven-it-plugin-site/pom.xml:
##########
@@ -67,6 +68,40 @@ under the License.
<groupId>org.apache.maven.reporting</groupId>
<artifactId>maven-reporting-exec</artifactId>
<version>1.6.0</version>
+ <exclusions>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-artifact</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-core</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-repository-metadata</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-model-builder</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-aether-provider</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-model</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-settings</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-settings-builder</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
Not 100% sure about the `exclusion` approach here. Seems to work as the plugin does not need these.
I guess the alternative would be to add these as `provided` dependencies into the POM.
--
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-integration-testing] psiroky commented on a diff in pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#discussion_r1061572431
##########
core-it-support/core-it-plugins/maven-it-plugin-site/pom.xml:
##########
@@ -67,6 +68,40 @@ under the License.
<groupId>org.apache.maven.reporting</groupId>
<artifactId>maven-reporting-exec</artifactId>
<version>1.6.0</version>
+ <exclusions>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-artifact</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-core</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-repository-metadata</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-model-builder</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-aether-provider</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-model</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-settings</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-settings-builder</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
Hmm, good point. Thanks!
I will try to exclude all the related Maven artifacts with one exclusion.
Edit: changed to just one exclusion for `org.apache.maven:*` as I think that's what we need. Excluding everything may be too much (but I haven't tested 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-integration-testing] slawekjaranowski commented on a diff in pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#discussion_r1061495477
##########
core-it-support/core-it-plugins/maven-it-plugin-site/pom.xml:
##########
@@ -67,6 +68,40 @@ under the License.
<groupId>org.apache.maven.reporting</groupId>
<artifactId>maven-reporting-exec</artifactId>
<version>1.6.0</version>
+ <exclusions>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-artifact</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-core</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-repository-metadata</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-model-builder</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-aether-provider</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-model</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-settings</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-settings-builder</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
or exclusion for `*:*`
--
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-integration-testing] psiroky commented on a diff in pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
psiroky commented on code in PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#discussion_r1061572431
##########
core-it-support/core-it-plugins/maven-it-plugin-site/pom.xml:
##########
@@ -67,6 +68,40 @@ under the License.
<groupId>org.apache.maven.reporting</groupId>
<artifactId>maven-reporting-exec</artifactId>
<version>1.6.0</version>
+ <exclusions>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-artifact</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-core</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-repository-metadata</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-model-builder</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-aether-provider</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-model</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-settings</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.maven</groupId>
+ <artifactId>maven-settings-builder</artifactId>
+ </exclusion>
+ </exclusions>
Review Comment:
Hmm, good point. Thanks!
I will try to exclude all the related Maven artifacts with one exclusion.
--
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-integration-testing] slawekjaranowski commented on pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223#issuecomment-1370951680
I used `dependencyManagement` in maven-enforcer because one Maven core artifact from transitive dependencies was in compile scope.
--
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-integration-testing] slawekjaranowski merged pull request #223: [MNG-7661] Use provided scope for Maven deps in IT plugins
Posted by GitBox <gi...@apache.org>.
slawekjaranowski merged PR #223:
URL: https://github.com/apache/maven-integration-testing/pull/223
--
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