You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2023/05/19 16:49:05 UTC
[GitHub] [maven] cstamas opened a new pull request, #1115: [MNG-7789] Dependency validation rules used wrong data
cstamas opened a new pull request, #1115:
URL: https://github.com/apache/maven/pull/1115
Refactored and now split, we have "plugin dependency" (using POM) and "plugin descriptor" (using PluginDescriptor/dependencies) to perform validation.
Also, two "inlined" validation from plugin dependencies resolver factored out to components, along with 3 existing moved to newly added (moved) validators.
---
https://issues.apache.org/jira/browse/MNG-7789
--
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] cstamas commented on pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1557880119
Will do that tomorrow, agreed.
--
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] hgschmie commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201128767
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginValidationManager.java:
##########
@@ -22,11 +22,7 @@
import javax.inject.Singleton;
import java.io.File;
-import java.util.Arrays;
-import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
-import java.util.Locale;
-import java.util.Map;
+import java.util.*;
Review Comment:
*cringe*
--
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 #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1557729532
> Today, descriptor/dependencies are not used anywhere (maybe in some UTs) in maven, the POM drives plugin dependencies. Hence the realization and this PR (it moves existing code to check POM originating deps).
> The descriptor/deps is at (plugin) build time derived from POM, and as we saw, is affected by possible mpp bugs like that one in 3.6.2
Then, why checking those and add validation warnings ? Wouldn't it make sense to actually deprecate them, not generate them anymore with the next m-p-p plugins ?
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201846556
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
readArtifactDescriptor == POM
resolveInternal = collection and resolving of POM (that above) augmented with things like WagonExcluder, ExclusionsDependencyFilter (excludes core and extension exported artifacts).
So, first is kinda "raw" while second is "processed", and due exclusions, 2nd will have core stuff excluded always,
--
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] cstamas commented on pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1554993253
This resolves my personal problem as well in really natural way: "plugin dependencies" are validated per-plugin level, while "plugin descriptor dependencies" were validated (and reported) per Mojo level, while it is plugin POM (so applicable to all Mojos within plugin) that may contains dependency problems, not standalone Mojos...
--
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] cstamas commented on pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1557705730
Today, descriptor/dependencies are not used anywhere (maybe in some UTs) in maven, the POM drives plugin dependencies. Hence the realization and this PR (it moves existing code to check POM originating deps).
The descriptor/deps is at (plugin) build time derived from POM, and as we saw, is affected by possible mpp bugs like that one in 3.6.2
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201606569
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusContainerDefaultDependenciesValidator.java:
##########
@@ -40,14 +41,17 @@ class PlexusContainerDefaultDependenciesValidator extends AbstractMavenPluginDep
super(pluginValidationManager);
}
- protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
- boolean pcdPresent = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
- .filter(d -> "org.codehaus.plexus".equals(d.getGroupId()))
- .anyMatch(d -> "plexus-container-default".equals(d.getArtifactId()));
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ boolean pcdPresent = artifactDescriptorResult.getDependencies().stream()
+ .filter(d -> "org.codehaus.plexus".equals(d.getArtifact().getGroupId()))
+ .anyMatch(d -> "plexus-container-default".equals(d.getArtifact().getArtifactId()));
if (pcdPresent) {
pluginValidationManager.reportPluginValidationIssue(
- mavenSession, mojoDescriptor, "Plugin depends on plexus-container-default, which is EOL");
+ session, pluginArtifact, "Plugin depends on plexus-container-default, which is EOL");
Review Comment:
For that, we'd need some "EOL marker" or some metadata?
Historically, there was p-c-d (from maven2 times), that was 14 years ago stopped being developed (essentially) as it got drop-in replacement in form of "plexus-shim" (that is sisu plexus). Furthermore, JSR330 migration started around same time, so today, there is really no reason to depend on p-c-d artifact (nor plexus shim, except in test scope, as PlexusTestCase is in there still, but also advisable to move off from that Junir3 class).
--
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] slawekjaranowski commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201834506
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
I think that dependency list returned by `repoSystem.readArtifactDescriptor` is different than `repoSystem.collectDependencies`
But for building plugin classloder we use output of `resolveInternal` method ... so maybe we should check dependencies returned by `repoSystem.collectDependencies`
--
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] hgschmie commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201134456
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/MavenScopeDependenciesValidator.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.
+ */
+package org.apache.maven.plugin.internal;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.maven.plugin.PluginValidationManager;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.resolution.ArtifactDescriptorResult;
+import org.eclipse.aether.util.artifact.JavaScopes;
+
+/**
+ * Detects Maven3 dependencies scope.
+ *
+ * @since 3.9.3
+ */
+@Singleton
+@Named
+class MavenScopeDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
+
+ @Inject
+ MavenScopeDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ @Override
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ Set<String> mavenArtifacts = artifactDescriptorResult.getDependencies().stream()
+ .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
+ .map(org.eclipse.aether.graph.Dependency::getArtifact)
+ .filter(a -> "org.apache.maven".equals(a.getGroupId()))
+ .filter(a -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
+ a.getGroupId() + ":" + a.getArtifactId()))
+ .filter(a -> a.getVersion().startsWith("3."))
+ .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
+ .collect(Collectors.toSet());
+
+ if (!mavenArtifacts.isEmpty()) {
+ pluginValidationManager.reportPluginValidationIssue(
+ session,
+ pluginArtifact,
+ "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
Review Comment:
same as above. Adding the line "if the plugin already declares them in `provided` scope, update the plugin-plugin to at least version x.y.z (see <link to maven bug here)` goes a *long* way to acceptance of bug reports.
--
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] hgschmie commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201130613
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/Maven2DependenciesValidator.java:
##########
@@ -45,19 +46,22 @@ class Maven2DependenciesValidator extends AbstractMavenPluginDependenciesValidat
}
@Override
- protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
- Set<String> maven2Versions = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ Set<String> maven2Versions = artifactDescriptorResult.getDependencies().stream()
+ .map(Dependency::getArtifact)
.filter(d -> "org.apache.maven".equals(d.getGroupId()))
- .filter(d -> !EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(d.getGroupId() + ":" + d.getArtifactId()))
- .map(ComponentDependency::getVersion)
+ .filter(d -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
+ d.getGroupId() + ":" + d.getArtifactId()))
+ .map(Artifact::getVersion)
.filter(v -> v.startsWith("2."))
.collect(Collectors.toSet());
if (!maven2Versions.isEmpty()) {
pluginValidationManager.reportPluginValidationIssue(
- mavenSession,
- mojoDescriptor,
- "Plugin is a Maven 2.x plugin, which will be not supported in Maven 4.x");
+ session, pluginArtifact, "Plugin is a Maven 2.x plugin, which will be not supported in Maven 4.x");
Review Comment:
this says "will not be supported"
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1202517112
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
@slawekjaranowski unsure what dependency plugin uses to display tree, but (to me) it seems it lacks information. Here is the 'verbose" tree that current resolver emits (in verbose mode): Don't be mislead by war version, as there is java snippet as well, I manipulate war POM BEFORE collect (basically making maven-core dep provided in war 3.3.2.
My point is, that "enhanced" DependencyGraphDumper (since 1.9.8) prints out extra info (more than dep plugin).
This is the "verbose" (and dirty) tree as there was no graph transformation applied). The fact that line has `(xxx)` at ends, means that node **is eliminated**, but is left for verbosity.
So we may want to look into here, but IMHO there is no "two sources" and the dep-tree seems to lack information...
edit: the mising gist https://gist.github.com/cstamas/ff2c22c8c3289991f5d3b5863bc6a4bd
--
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] slawekjaranowski commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1202017682
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
I know that every core artifacts are stripped ...
but we check if plugin has such on dependency list - so we have two different source of checked and used dependencies
In my example we have:
```
[DEBUG] org.apache.maven.shared:maven-mapping:jar:3.0.0:compile
[DEBUG] org.apache.maven:maven-core:jar:3.0:compile
```
but validator do not detect that `maven-core` is in compile scope on dependencies
--
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] slawekjaranowski commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201865643
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
Not every artifacts is excluded, pleas look: https://gist.github.com/slawekjaranowski/c65612fca4094ec8a2d09c6db3b8f78a
like this:
```
[DEBUG] Populating class realm plugin>org.apache.maven.plugins:maven-war-plugin:3.4.0-SNAPSHOT
...
[DEBUG] Included: org.sonatype.sisu:sisu-inject-bean:jar:1.4.2
[DEBUG] Included: org.sonatype.sisu:sisu-guice:jar:noaop:2.1.7
[DEBUG] Included: org.codehaus.plexus:plexus-component-annotations:jar:1.5.5
[DEBUG] Included: org.sonatype.plexus:plexus-sec-dispatcher:jar:1.3
[DEBUG] Included: org.sonatype.plexus:plexus-cipher:jar:1.4
...
```
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201606569
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusContainerDefaultDependenciesValidator.java:
##########
@@ -40,14 +41,17 @@ class PlexusContainerDefaultDependenciesValidator extends AbstractMavenPluginDep
super(pluginValidationManager);
}
- protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
- boolean pcdPresent = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
- .filter(d -> "org.codehaus.plexus".equals(d.getGroupId()))
- .anyMatch(d -> "plexus-container-default".equals(d.getArtifactId()));
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ boolean pcdPresent = artifactDescriptorResult.getDependencies().stream()
+ .filter(d -> "org.codehaus.plexus".equals(d.getArtifact().getGroupId()))
+ .anyMatch(d -> "plexus-container-default".equals(d.getArtifact().getArtifactId()));
if (pcdPresent) {
pluginValidationManager.reportPluginValidationIssue(
- mavenSession, mojoDescriptor, "Plugin depends on plexus-container-default, which is EOL");
+ session, pluginArtifact, "Plugin depends on plexus-container-default, which is EOL");
Review Comment:
For that, we'd need some "EOL marker" or some metadata?
Historically, there was p-c-d (from maven2 times), that was 14 years ago stopped being developed (essentially) got drop-in replacement in form of "plexus-shim" (that is sisu plexus). Furthermore, JSR330 migration started around same time, so today, there is really no reason to depend on p-c-d artifact (nor plexus shim, except in test scope, as PlexusTestCase is in there still, but also advisable to move off from that Junir3 class).
--
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] cstamas commented on pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1558632867
Pushed two commits:
* removed descriptor dependencies validator (3 classes)
* reworded/aligned emitted messages
--
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] hgschmie commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1202722928
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusContainerDefaultDependenciesValidator.java:
##########
@@ -40,14 +41,17 @@ class PlexusContainerDefaultDependenciesValidator extends AbstractMavenPluginDep
super(pluginValidationManager);
}
- protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
- boolean pcdPresent = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
- .filter(d -> "org.codehaus.plexus".equals(d.getGroupId()))
- .anyMatch(d -> "plexus-container-default".equals(d.getArtifactId()));
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ boolean pcdPresent = artifactDescriptorResult.getDependencies().stream()
+ .filter(d -> "org.codehaus.plexus".equals(d.getArtifact().getGroupId()))
+ .anyMatch(d -> "plexus-container-default".equals(d.getArtifact().getArtifactId()));
if (pcdPresent) {
pluginValidationManager.reportPluginValidationIssue(
- mavenSession, mojoDescriptor, "Plugin depends on plexus-container-default, which is EOL");
+ session, pluginArtifact, "Plugin depends on plexus-container-default, which is EOL");
Review Comment:
I meant "we maintain a list of EOLed artifacts here and report them if found in the tree", not "look up one specifically". I guess we can rewrite this if a second "this is EOL" artifact shows up.
--
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 #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1557773421
> This PR does not add any check to it, it actually moves them off from it. One check remains, thay yes, may be killed off actually.
Can we then delete the AbstractMavenPluginDescriptorDependenciesValidator and MavenArtifactsMavenPluginDescriptorDependenciesValidator classes which are new ?
This would remove all usage of `PluginDescriptor#getDependencies()` from maven (apart from the `clone` and UTs).
--
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] cstamas commented on pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1557749704
This PR does not add any check to it, it actually moves them off from it. One check remains, thay yes, may be killed off actually.
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1202517112
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
@slawekjaranowski unsure what dependency plugin uses to display tree, but (to me) it seems it lacks information. Here is the 'verbose" tree that current resolver emits (in verbose mode): Don't be mislead by war version, as there is java snippet as well, I manipulate war POM BEFORE collect (basically making maven-core dep provided in war 3.3.2.
My point is, that "enhanced" DependencyGraphDumper (since 1.9.8) prints out extra info (more than dep plugin).
This is the "verbose" (and dirty) tree as there was no graph transformation applied). The fact that line has `(xxx)` at ends, means that node **is eliminated** (by conflict resolver), but is left for verbosity.
So we may want to look into here, but IMHO there is no "two sources" and the dep-tree seems to lack information...
edit: the mising gist https://gist.github.com/cstamas/ff2c22c8c3289991f5d3b5863bc6a4bd
Explanation: "you line" is https://gist.github.com/cstamas/ff2c22c8c3289991f5d3b5863bc6a4bd#file-gistfile1-txt-L88
and it has `(conflicts with 3.1.0)` so is eliminated.
--
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] slawekjaranowski commented on pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1558021863
Plugin pom/descriptor dependencies can something else that is resolved by dependency collection and what finally is used to construct plugin class realm
like: https://gist.github.com/slawekjaranowski/c65612fca4094ec8a2d09c6db3b8f78a
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201583518
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/MavenArtifactsMavenPluginDescriptorDependenciesValidator.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.
+ */
+package org.apache.maven.plugin.internal;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.PluginValidationManager;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+
+/**
+ * Detects presence of unwanted Maven3 artifacts in plugin descriptor, possibly caused by multitude of reasons, among
+ * them is "wrong scope" dependency declaration as well.
+ * <p>
+ * Historically, this class was named as "MavenScopeDependenciesValidator" due original intent to check "wrong Maven
+ * Artifact scopes". Since then, it turned out that the values validated (the plugin descriptor dependencies, that is
+ * produced at plugin build time by maven-plugin-plugin) may be off (for example due maven-plugin-plugin bug), and
+ * is potentially not inline with "reality" (actual plugin dependencies).
+ * <p>
+ * The original intent related check is moved to
+ * {@link DefaultPluginDependenciesResolver#resolve(org.apache.maven.model.Plugin, java.util.List, org.eclipse.aether.RepositorySystemSession)}
+ * method instead.
+ *
+ * @since 3.9.3
+ */
+@Singleton
+@Named
+class MavenArtifactsMavenPluginDescriptorDependenciesValidator
+ extends AbstractMavenPluginDescriptorDependenciesValidator {
+
+ @Inject
+ MavenArtifactsMavenPluginDescriptorDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ @Override
+ protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
+ Set<String> mavenArtifacts = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
+ .filter(d -> "org.apache.maven".equals(d.getGroupId()))
+ .filter(d -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
+ d.getGroupId() + ":" + d.getArtifactId()))
+ .filter(d -> d.getVersion().startsWith("3."))
+ .map(d -> d.getGroupId() + ":" + d.getArtifactId() + ":" + d.getVersion())
+ .collect(Collectors.toSet());
+
+ if (!mavenArtifacts.isEmpty()) {
+ pluginValidationManager.reportPluginValidationIssue(
+ mavenSession,
+ mojoDescriptor,
+ "Plugin descriptor should not contain these Maven artifacts: " + mavenArtifacts);
Review Comment:
This class (that uses plugin descriptor / dependencies) is about to be removed.
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201619428
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/MavenScopeDependenciesValidator.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.
+ */
+package org.apache.maven.plugin.internal;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.maven.plugin.PluginValidationManager;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.resolution.ArtifactDescriptorResult;
+import org.eclipse.aether.util.artifact.JavaScopes;
+
+/**
+ * Detects Maven3 dependencies scope.
+ *
+ * @since 3.9.3
+ */
+@Singleton
+@Named
+class MavenScopeDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
+
+ @Inject
+ MavenScopeDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ @Override
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ Set<String> mavenArtifacts = artifactDescriptorResult.getDependencies().stream()
+ .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
+ .map(org.eclipse.aether.graph.Dependency::getArtifact)
+ .filter(a -> "org.apache.maven".equals(a.getGroupId()))
+ .filter(a -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
+ a.getGroupId() + ":" + a.getArtifactId()))
+ .filter(a -> a.getVersion().startsWith("3."))
+ .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
+ .collect(Collectors.toSet());
+
+ if (!mavenArtifacts.isEmpty()) {
+ pluginValidationManager.reportPluginValidationIssue(
+ session,
+ pluginArtifact,
+ "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
Review Comment:
While I agree, I just went with "upgrade to latest", as I cannot wholeheartedly recommend 3.6.3 (that would fix your case), nor any other "historical" version, as if you look at release notes, each release do contain a bug fix, that this or that way just improves things.
--
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] cstamas merged pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #1115:
URL: https://github.com/apache/maven/pull/1115
--
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] hgschmie commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201132331
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/MavenArtifactsMavenPluginDescriptorDependenciesValidator.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.
+ */
+package org.apache.maven.plugin.internal;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.PluginValidationManager;
+import org.apache.maven.plugin.descriptor.MojoDescriptor;
+
+/**
+ * Detects presence of unwanted Maven3 artifacts in plugin descriptor, possibly caused by multitude of reasons, among
+ * them is "wrong scope" dependency declaration as well.
+ * <p>
+ * Historically, this class was named as "MavenScopeDependenciesValidator" due original intent to check "wrong Maven
+ * Artifact scopes". Since then, it turned out that the values validated (the plugin descriptor dependencies, that is
+ * produced at plugin build time by maven-plugin-plugin) may be off (for example due maven-plugin-plugin bug), and
+ * is potentially not inline with "reality" (actual plugin dependencies).
+ * <p>
+ * The original intent related check is moved to
+ * {@link DefaultPluginDependenciesResolver#resolve(org.apache.maven.model.Plugin, java.util.List, org.eclipse.aether.RepositorySystemSession)}
+ * method instead.
+ *
+ * @since 3.9.3
+ */
+@Singleton
+@Named
+class MavenArtifactsMavenPluginDescriptorDependenciesValidator
+ extends AbstractMavenPluginDescriptorDependenciesValidator {
+
+ @Inject
+ MavenArtifactsMavenPluginDescriptorDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ @Override
+ protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
+ Set<String> mavenArtifacts = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
+ .filter(d -> "org.apache.maven".equals(d.getGroupId()))
+ .filter(d -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
+ d.getGroupId() + ":" + d.getArtifactId()))
+ .filter(d -> d.getVersion().startsWith("3."))
+ .map(d -> d.getGroupId() + ":" + d.getArtifactId() + ":" + d.getVersion())
+ .collect(Collectors.toSet());
+
+ if (!mavenArtifacts.isEmpty()) {
+ pluginValidationManager.reportPluginValidationIssue(
+ mavenSession,
+ mojoDescriptor,
+ "Plugin descriptor should not contain these Maven artifacts: " + mavenArtifacts);
Review Comment:
there should be an explanation *why* the descriptor should not contain them (and what a plugin author can do, e.g. "update the version of the maven-plugin-plugin to at least ...").
--
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] hgschmie commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201137338
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusContainerDefaultDependenciesValidator.java:
##########
@@ -40,14 +41,17 @@ class PlexusContainerDefaultDependenciesValidator extends AbstractMavenPluginDep
super(pluginValidationManager);
}
- protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
- boolean pcdPresent = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
- .filter(d -> "org.codehaus.plexus".equals(d.getGroupId()))
- .anyMatch(d -> "plexus-container-default".equals(d.getArtifactId()));
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ boolean pcdPresent = artifactDescriptorResult.getDependencies().stream()
+ .filter(d -> "org.codehaus.plexus".equals(d.getArtifact().getGroupId()))
+ .anyMatch(d -> "plexus-container-default".equals(d.getArtifact().getArtifactId()));
if (pcdPresent) {
pluginValidationManager.reportPluginValidationIssue(
- mavenSession, mojoDescriptor, "Plugin depends on plexus-container-default, which is EOL");
+ session, pluginArtifact, "Plugin depends on plexus-container-default, which is EOL");
Review Comment:
this looks like it could be a more generic "check for end of life" component checker if there other stuff that is EOLed, too.
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201619428
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/MavenScopeDependenciesValidator.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.
+ */
+package org.apache.maven.plugin.internal;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.maven.plugin.PluginValidationManager;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.resolution.ArtifactDescriptorResult;
+import org.eclipse.aether.util.artifact.JavaScopes;
+
+/**
+ * Detects Maven3 dependencies scope.
+ *
+ * @since 3.9.3
+ */
+@Singleton
+@Named
+class MavenScopeDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
+
+ @Inject
+ MavenScopeDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ @Override
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ Set<String> mavenArtifacts = artifactDescriptorResult.getDependencies().stream()
+ .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
+ .map(org.eclipse.aether.graph.Dependency::getArtifact)
+ .filter(a -> "org.apache.maven".equals(a.getGroupId()))
+ .filter(a -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
+ a.getGroupId() + ":" + a.getArtifactId()))
+ .filter(a -> a.getVersion().startsWith("3."))
+ .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
+ .collect(Collectors.toSet());
+
+ if (!mavenArtifacts.isEmpty()) {
+ pluginValidationManager.reportPluginValidationIssue(
+ session,
+ pluginArtifact,
+ "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
Review Comment:
While I agree, I just went with "upgrade to latest", as I cannot wholeheartedly recommend 3.6.3 (that would fix your case), nor any other "historical" version, as if you look at release notes, each release do contain several bug fixes, that in this or that way just improves things.
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201872854
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
All core exported artifacts ARE stripped. Check this list and you will not find any artifact on that list in your debug log.
https://github.com/apache/maven/blob/maven-3.9.x/maven-core/src/main/resources/META-INF/maven/extension.xml#L133
--
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 diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1200735153
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginValidationManager.java:
##########
@@ -46,6 +42,13 @@
@Singleton
@Named
public final class DefaultPluginValidationManager extends AbstractEventSpy implements PluginValidationManager {
+ /**
+ * The collection of "G:A" combinations that do NOT belong to Maven Core, hence, should be excluded from
+ * "expected in provided scope" type of checks.
+ */
+ static final Collection<String> EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA =
+ Collections.unmodifiableCollection(Arrays.asList(
+ "org.apache.maven:maven-archiver", "org.apache.maven:maven-jxr", "org.apache.maven:plexus-utils"));
Review Comment:
I know those values have not changed, but is the `org.apache.maven` groupId expected for `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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1200887062
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginValidationManager.java:
##########
@@ -46,6 +42,13 @@
@Singleton
@Named
public final class DefaultPluginValidationManager extends AbstractEventSpy implements PluginValidationManager {
+ /**
+ * The collection of "G:A" combinations that do NOT belong to Maven Core, hence, should be excluded from
+ * "expected in provided scope" type of checks.
+ */
+ static final Collection<String> EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA =
+ Collections.unmodifiableCollection(Arrays.asList(
+ "org.apache.maven:maven-archiver", "org.apache.maven:maven-jxr", "org.apache.maven:plexus-utils"));
Review Comment:
You tell me 😄 https://repo.maven.apache.org/maven2/org/apache/maven/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] hgschmie commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201131597
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/Maven3CompatDependenciesValidator.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * 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.
+ */
+package org.apache.maven.plugin.internal;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.apache.maven.plugin.PluginValidationManager;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.resolution.ArtifactDescriptorResult;
+import org.eclipse.aether.util.artifact.JavaScopes;
+
+/**
+ * Detects Maven3 plugins using maven-compat Maven2 compatibility layer.
+ *
+ * @since 3.9.3
+ */
+@Singleton
+@Named
+class Maven3CompatDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
+
+ @Inject
+ Maven3CompatDependenciesValidator(PluginValidationManager pluginValidationManager) {
+ super(pluginValidationManager);
+ }
+
+ @Override
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ for (org.eclipse.aether.graph.Dependency dependency : artifactDescriptorResult.getDependencies()) {
+ if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
+ && "maven-compat".equals(dependency.getArtifact().getArtifactId())
+ && !JavaScopes.TEST.equals(dependency.getScope())) {
+ pluginValidationManager.reportPluginValidationIssue(
+ session,
+ pluginArtifact,
+ "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
Review Comment:
this says "may not be supported". It implies that there is chance that it may still be supported. This should be the same wording as above.
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1201606569
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/PlexusContainerDefaultDependenciesValidator.java:
##########
@@ -40,14 +41,17 @@ class PlexusContainerDefaultDependenciesValidator extends AbstractMavenPluginDep
super(pluginValidationManager);
}
- protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
- boolean pcdPresent = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
- .filter(d -> "org.codehaus.plexus".equals(d.getGroupId()))
- .anyMatch(d -> "plexus-container-default".equals(d.getArtifactId()));
+ protected void doValidate(
+ RepositorySystemSession session,
+ Artifact pluginArtifact,
+ ArtifactDescriptorResult artifactDescriptorResult) {
+ boolean pcdPresent = artifactDescriptorResult.getDependencies().stream()
+ .filter(d -> "org.codehaus.plexus".equals(d.getArtifact().getGroupId()))
+ .anyMatch(d -> "plexus-container-default".equals(d.getArtifact().getArtifactId()));
if (pcdPresent) {
pluginValidationManager.reportPluginValidationIssue(
- mavenSession, mojoDescriptor, "Plugin depends on plexus-container-default, which is EOL");
+ session, pluginArtifact, "Plugin depends on plexus-container-default, which is EOL");
Review Comment:
For that, we'd need some "EOL marker" or some metadata?
Historically, the was p-c-d, that was 14 years ago stopped being developed (essentially) got drop-in replacement in form of "plexus-shim" (that is sisu plexus). Furthermore, JSR330 migration started around same time, so today, there is really no reason to depend on p-c-d artifact (nor plexus shim, except in test scope, as PlexusTestCase is in there still, but also advisable to move off from that Junir3 class).
--
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] cstamas commented on pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1115:
URL: https://github.com/apache/maven/pull/1115#issuecomment-1558640878
So this PR now contains all the changes I would like to have in here (as these needs to be ported to master as well).
Once this merged, then we should create another PR that _splits_ (by concern bearer) the emitted messages and/or reports them different or maybe even at different channels. That PR will also need to be ported to master as well, I guess.
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1202517112
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
@slawekjaranowski unsure what dependency plugin uses to display tree, but (to me) it seems it lacks information. Here is the 'verbose" tree that current resolver emits (in verbose mode): Don't be mislead by war version, as there is java snippet as well, I manipulate war POM BEFORE collect (basically making maven-core dep provided in war 3.3.2.
My point is, that "enhanced" DependencyGraphDumper (since 1.9.8) prints out extra info (more than dep plugin).
This is the "verbose" (and dirty) tree as there was no graph transformation applied). The fact that line has `(xxx)` at ends, means that node **is eliminated** (by conflict resolver), but is left for verbosity.
So we may want to look into here, but IMHO there is no "two sources" and the dep-tree seems to lack information...
edit: the mising gist https://gist.github.com/cstamas/ff2c22c8c3289991f5d3b5863bc6a4bd
--
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] cstamas commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1202517112
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
@slawekjaranowski unsure what dependency plugin uses to display tree, but (to me) it seems it lacks information. Here is the 'verbose" tree that current resolver emits (in verbose mode): Don't be mislead by war version, as there is java snippet as well, I manipulate war POM BEFORE collect (basically making maven-core dep provided in war 3.3.2.
My point is, that "enhanced" DependencyGraphDumper (since 1.9.8) prints out extra info (more than dep plugin).
This is the "verbose" (and dirty) tree as there was no graph transformation applied). The fact that line has `(xxx)` at ends, means that node **is eliminated**, but is left for verbosity.
So we may want to look into here, but IMHO there is no "two sources" and the dep-tree seems to lack information...
--
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] slawekjaranowski commented on a diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1202645758
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java:
##########
@@ -109,34 +110,8 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
- if (result.getDependencies() != null) {
- for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
- if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
- && "maven-compat".equals(dependency.getArtifact().getArtifactId())
- && !JavaScopes.TEST.equals(dependency.getScope())) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
- }
- }
-
- Set<String> mavenArtifacts = result.getDependencies().stream()
- .filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
- .map(org.eclipse.aether.graph.Dependency::getArtifact)
- .filter(a -> "org.apache.maven".equals(a.getGroupId()))
- .filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
- a.getGroupId() + ":" + a.getArtifactId()))
- .filter(a -> a.getVersion().startsWith("3."))
- .map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
- .collect(Collectors.toSet());
-
- if (!mavenArtifacts.isEmpty()) {
- pluginValidationManager.reportPluginValidationIssue(
- session,
- pluginArtifact,
- "Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
- }
+ for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
+ dependenciesValidator.validate(session, pluginArtifact, result);
Review Comment:
@cstamas - ok, let's leave as is - we can improve later if needed
--
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 diff in pull request #1115: [MNG-7789] Dependency validation rules used wrong data
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #1115:
URL: https://github.com/apache/maven/pull/1115#discussion_r1200901478
##########
maven-core/src/main/java/org/apache/maven/plugin/internal/DefaultPluginValidationManager.java:
##########
@@ -46,6 +42,13 @@
@Singleton
@Named
public final class DefaultPluginValidationManager extends AbstractEventSpy implements PluginValidationManager {
+ /**
+ * The collection of "G:A" combinations that do NOT belong to Maven Core, hence, should be excluded from
+ * "expected in provided scope" type of checks.
+ */
+ static final Collection<String> EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA =
+ Collections.unmodifiableCollection(Arrays.asList(
+ "org.apache.maven:maven-archiver", "org.apache.maven:maven-jxr", "org.apache.maven:plexus-utils"));
Review Comment:
Yes, and this very artifact will not exist anymore in 4.0.0-m6, and that's why I asked the question. It does not exist in 3.9.x either. But afaik, you added a check on this artifact on https://github.com/apache/maven/commit/53b64732378fe27a91cb66dad5693d6d0c71628a#diff-95336d0fc4e39e625e90137247157a9d73ed8094dfb4e3431b589a9d94c3dedeR38
Do you recall for which use case you added this check ?
--
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