From 79db9a3babf2353ce91cf63d467082f06d918252 Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Sat, 11 Mar 2023 20:54:02 +0100 Subject: [PATCH] [MENFORCER-469] Fix banTransitiveDependencies and transitive dependencies with another version than the resolved one - introduce methods: - resolveTransitiveDependenciesVerbose resolve with full tree contains all dependencies - also conflicted - resolveTransitiveDependencies resolve final tree contains dependencies after conflict resolved --- .../dependency/BannedDependenciesBase.java | 2 +- .../dependency/DependencyConvergence.java | 2 +- .../dependency/RequireUpperBoundDeps.java | 2 +- .../rules/dependency/ResolveUtil.java | 49 ++++++++---- .../dependency/BannedDependenciesTest.java | 4 +- .../dependency/RequireReleaseDepsTest.java | 17 ++-- .../dependency/RequireUpperBoundDepsTest.java | 2 +- .../pom.xml | 70 ++++++++++++++++ .../pom.xml | 69 ++++++++++++++++ .../pom.xml | 79 +++++++++++++++++++ 10 files changed, 268 insertions(+), 28 deletions(-) create mode 100644 maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep1/pom.xml create mode 100644 maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep2/pom.xml create mode 100644 maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep3/pom.xml diff --git a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependenciesBase.java b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependenciesBase.java index 4ee240e5..90691c29 100644 --- a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependenciesBase.java +++ b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BannedDependenciesBase.java @@ -102,7 +102,7 @@ public void execute() throws EnforcerRuleException { } } else { StringBuilder messageBuilder = new StringBuilder(); - DependencyNode rootNode = resolveUtil.resolveTransitiveDependencies(); + DependencyNode rootNode = resolveUtil.resolveTransitiveDependenciesVerbose(); if (!validate(rootNode, 0, messageBuilder)) { String message = ""; if (getMessage() != null) { diff --git a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/DependencyConvergence.java b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/DependencyConvergence.java index 4c6fbddb..b235b66a 100644 --- a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/DependencyConvergence.java +++ b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/DependencyConvergence.java @@ -63,7 +63,7 @@ public DependencyConvergence(ResolveUtil resolveUtil) { @Override public void execute() throws EnforcerRuleException { - DependencyNode node = resolveUtil.resolveTransitiveDependencies( + DependencyNode node = resolveUtil.resolveTransitiveDependenciesVerbose( // TODO: use a modified version of ExclusionDependencySelector to process excludes and includes new DependencySelector() { @Override diff --git a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDeps.java b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDeps.java index 27f78899..0a26be3b 100644 --- a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDeps.java +++ b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDeps.java @@ -97,7 +97,7 @@ public void setIncludes(List includes) { @Override public void execute() throws EnforcerRuleException { - DependencyNode node = resolveUtil.resolveTransitiveDependencies(); + DependencyNode node = resolveUtil.resolveTransitiveDependenciesVerbose(); upperBoundDepsVisitor = new RequireUpperBoundDepsVisitor() .setUniqueVersions(uniqueVersions) .setIncludes(includes); diff --git a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/ResolveUtil.java b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/ResolveUtil.java index 1337fed3..19d4a204 100644 --- a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/ResolveUtil.java +++ b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/ResolveUtil.java @@ -39,14 +39,9 @@ import org.eclipse.aether.graph.DependencyNode; import org.eclipse.aether.util.graph.manager.DependencyManagerUtils; import org.eclipse.aether.util.graph.selector.AndDependencySelector; -import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector; -import org.eclipse.aether.util.graph.selector.OptionalDependencySelector; -import org.eclipse.aether.util.graph.selector.ScopeDependencySelector; import org.eclipse.aether.util.graph.transformer.ConflictResolver; import static java.util.Optional.ofNullable; -import static org.apache.maven.artifact.Artifact.SCOPE_PROVIDED; -import static org.apache.maven.artifact.Artifact.SCOPE_TEST; /** * Resolver helper class. @@ -55,6 +50,7 @@ class ResolveUtil { private final RepositorySystem repositorySystem; + private final MavenSession session; /** @@ -66,6 +62,24 @@ class ResolveUtil { this.session = Objects.requireNonNull(session); } + /** + * Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency + * for the current {@link MavenProject} in verbose mode. + *

+ * In verbose mode all nodes participating in a conflict are retained. + *

+ *

+ * Please consult {@link ConflictResolver} and {@link DependencyManagerUtils}> + *

+ * + * @param selectors zero or more {@link DependencySelector} instances + * @return a Dependency Node which is the root of the project's dependency tree + * @throws EnforcerRuleException thrown if the lookup fails + */ + DependencyNode resolveTransitiveDependenciesVerbose(DependencySelector... selectors) throws EnforcerRuleException { + return resolveTransitiveDependencies(true, selectors); + } + /** * Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency * for the current {@link MavenProject}. @@ -75,13 +89,12 @@ class ResolveUtil { * @throws EnforcerRuleException thrown if the lookup fails */ DependencyNode resolveTransitiveDependencies(DependencySelector... selectors) throws EnforcerRuleException { - if (selectors.length == 0) { - selectors = new DependencySelector[] { - new ScopeDependencySelector(SCOPE_TEST, SCOPE_PROVIDED), - new OptionalDependencySelector(), - new ExclusionDependencySelector() - }; - } + return resolveTransitiveDependencies(false, selectors); + } + + private DependencyNode resolveTransitiveDependencies(boolean verbose, DependencySelector... selectors) + throws EnforcerRuleException { + try { MavenProject project = session.getCurrentProject(); ArtifactTypeRegistry artifactTypeRegistry = @@ -89,9 +102,15 @@ DependencyNode resolveTransitiveDependencies(DependencySelector... selectors) th DefaultRepositorySystemSession repositorySystemSession = new DefaultRepositorySystemSession(session.getRepositorySession()); - repositorySystemSession.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true); - repositorySystemSession.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true); - repositorySystemSession.setDependencySelector(new AndDependencySelector(selectors)); + + if (selectors.length > 0) { + repositorySystemSession.setDependencySelector(new AndDependencySelector(selectors)); + } + + if (verbose) { + repositorySystemSession.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true); + repositorySystemSession.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true); + } CollectRequest collectRequest = new CollectRequest( project.getDependencies().stream() diff --git a/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/BannedDependenciesTest.java b/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/BannedDependenciesTest.java index 69ad445d..18883573 100644 --- a/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/BannedDependenciesTest.java +++ b/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/BannedDependenciesTest.java @@ -94,7 +94,7 @@ void excludesAndIncludesDoNotUseTransitiveDependencies() throws Exception { @Test void excludesUseTransitiveDependencies() throws Exception { - when(resolveUtil.resolveTransitiveDependencies()) + when(resolveUtil.resolveTransitiveDependenciesVerbose()) .thenReturn(new DependencyNodeBuilder() .withType(DependencyNodeBuilder.Type.POM) .withChildNode(new DependencyNodeBuilder() @@ -127,7 +127,7 @@ void excludesUseTransitiveDependencies() throws Exception { @Test void excludesAndIncludesUseTransitiveDependencies() throws Exception { - when(resolveUtil.resolveTransitiveDependencies()) + when(resolveUtil.resolveTransitiveDependenciesVerbose()) .thenReturn(new DependencyNodeBuilder() .withType(DependencyNodeBuilder.Type.POM) .withChildNode(new DependencyNodeBuilder() diff --git a/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/RequireReleaseDepsTest.java b/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/RequireReleaseDepsTest.java index b1409a6d..5a0b76f8 100644 --- a/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/RequireReleaseDepsTest.java +++ b/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/RequireReleaseDepsTest.java @@ -80,7 +80,7 @@ void testSearchNonTransitive() throws IOException { @Test void testSearchTransitiveMultipleFailures() throws Exception { - when(resolveUtil.resolveTransitiveDependencies()).thenReturn(getDependencyNodeWithMultipleSnapshots()); + when(resolveUtil.resolveTransitiveDependenciesVerbose()).thenReturn(getDependencyNodeWithMultipleSnapshots()); rule.setSearchTransitive(true); assertThatCode(rule::execute) @@ -94,7 +94,7 @@ void testSearchTransitiveMultipleFailures() throws Exception { @Test void testSearchTransitiveNoFailures() throws Exception { when(session.getCurrentProject()).thenReturn(project); - when(resolveUtil.resolveTransitiveDependencies()).thenReturn(new DependencyNodeBuilder().build()); + when(resolveUtil.resolveTransitiveDependenciesVerbose()).thenReturn(new DependencyNodeBuilder().build()); rule.setSearchTransitive(true); assertThatCode(rule::execute).doesNotThrowAnyException(); @@ -114,7 +114,8 @@ void testShouldFailOnlyWhenRelease() throws Exception { @Test void testWildcardExcludeTests() throws Exception { when(session.getCurrentProject()).thenReturn(project); - when(resolveUtil.resolveTransitiveDependencies()).thenReturn(getDependencyNodeWithMultipleTestSnapshots()); + when(resolveUtil.resolveTransitiveDependenciesVerbose()) + .thenReturn(getDependencyNodeWithMultipleTestSnapshots()); rule.setExcludes(Collections.singletonList("*:*:*:*:test")); rule.setSearchTransitive(true); @@ -125,7 +126,8 @@ void testWildcardExcludeTests() throws Exception { @Test void testWildcardExcludeAll() throws Exception { when(session.getCurrentProject()).thenReturn(project); - when(resolveUtil.resolveTransitiveDependencies()).thenReturn(getDependencyNodeWithMultipleTestSnapshots()); + when(resolveUtil.resolveTransitiveDependenciesVerbose()) + .thenReturn(getDependencyNodeWithMultipleTestSnapshots()); rule.setExcludes(Collections.singletonList("*")); rule.setSearchTransitive(true); @@ -135,7 +137,8 @@ void testWildcardExcludeAll() throws Exception { @Test void testExcludesAndIncludes() throws Exception { - when(resolveUtil.resolveTransitiveDependencies()).thenReturn(getDependencyNodeWithMultipleTestSnapshots()); + when(resolveUtil.resolveTransitiveDependenciesVerbose()) + .thenReturn(getDependencyNodeWithMultipleTestSnapshots()); rule.setExcludes(Collections.singletonList("*")); rule.setIncludes(Collections.singletonList("*:*:*:*:test")); @@ -161,7 +164,7 @@ void testId() { void testFailWhenParentIsSnapshot() throws Exception { when(session.getCurrentProject()).thenReturn(project); when(project.getParentArtifact()).thenReturn(ARTIFACT_STUB_FACTORY.getSnapshotArtifact()); - when(resolveUtil.resolveTransitiveDependencies()).thenReturn(new DependencyNodeBuilder().build()); + when(resolveUtil.resolveTransitiveDependenciesVerbose()).thenReturn(new DependencyNodeBuilder().build()); rule.setFailWhenParentIsSnapshot(true); @@ -174,7 +177,7 @@ void testFailWhenParentIsSnapshot() throws Exception { void parentShouldBeExcluded() throws Exception { when(session.getCurrentProject()).thenReturn(project); when(project.getParentArtifact()).thenReturn(ARTIFACT_STUB_FACTORY.getSnapshotArtifact()); - when(resolveUtil.resolveTransitiveDependencies()).thenReturn(new DependencyNodeBuilder().build()); + when(resolveUtil.resolveTransitiveDependenciesVerbose()).thenReturn(new DependencyNodeBuilder().build()); rule.setFailWhenParentIsSnapshot(true); rule.setExcludes(Collections.singletonList("testGroupId:*")); diff --git a/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDepsTest.java b/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDepsTest.java index 5cbbe30e..0cdd221a 100644 --- a/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDepsTest.java +++ b/enforcer-rules/src/test/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDepsTest.java @@ -41,7 +41,7 @@ class RequireUpperBoundDepsTest { @Test void testRule() throws Exception { - when(resolveUtil.resolveTransitiveDependencies()) + when(resolveUtil.resolveTransitiveDependenciesVerbose()) .thenReturn(new DependencyNodeBuilder() .withType(DependencyNodeBuilder.Type.POM) .withChildNode(new DependencyNodeBuilder() diff --git a/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep1/pom.xml b/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep1/pom.xml new file mode 100644 index 00000000..01a1d9bc --- /dev/null +++ b/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep1/pom.xml @@ -0,0 +1,70 @@ + + + + + + 4.0.0 + + org.apache.maven.its.enforcer + ban-transitive-test + 1.0 + + https://issues.apache.org/jira/browse/MENFORCER-469 + + + + + org.apache.maven.plugins + maven-enforcer-plugin + @project.version@ + + + test + + enforce + + + + + + + + + + + + + + + org.apache.maven.plugins.enforcer.its + menforcer128_classic + 0.9.9 + + + + + + org.apache.maven.plugins.enforcer.its + menforcer128_api + 1.5.0 + + + + diff --git a/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep2/pom.xml b/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep2/pom.xml new file mode 100644 index 00000000..ad69e5a8 --- /dev/null +++ b/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep2/pom.xml @@ -0,0 +1,69 @@ + + + + + + 4.0.0 + + org.apache.maven.its.enforcer + ban-transitive-test + 1.0 + + https://issues.apache.org/jira/browse/MENFORCER-469 + + + + + org.apache.maven.plugins + maven-enforcer-plugin + @project.version@ + + + test + + enforce + + + + + + + + + + + + + + + org.apache.maven.plugins.enforcer.its + menforcer128_classic + 0.9.9 + + + + + org.apache.maven.plugins.enforcer.its + menforcer128_api + 1.6.0 + + + + diff --git a/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep3/pom.xml b/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep3/pom.xml new file mode 100644 index 00000000..64dd5158 --- /dev/null +++ b/maven-enforcer-plugin/src/it/projects/ban-transitive-dependencies-direct-dep3/pom.xml @@ -0,0 +1,79 @@ + + + + + + 4.0.0 + + org.apache.maven.its.enforcer + ban-transitive-test + 1.0 + + https://issues.apache.org/jira/browse/MENFORCER-469 + + + + + org.apache.maven.plugins + maven-enforcer-plugin + @project.version@ + + + test + + enforce + + + + + + + + + + + + + + + + org.apache.maven.plugins.enforcer.its + menforcer128_api + 1.6.0 + + + + + + + org.apache.maven.plugins.enforcer.its + menforcer128_classic + 0.9.9 + + + + + + org.apache.maven.plugins.enforcer.its + menforcer128_api + + + +