You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/12/16 13:48:26 UTC
[GitHub] [maven-enforcer] ajarmoniuk opened a new pull request, #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
ajarmoniuk opened a new pull request, #198:
URL: https://github.com/apache/maven-enforcer/pull/198
See summary; also: dependency:tree is no longer necessary to check parent information of dependencies which triggered validation failures -- path information is included.
@slawekjaranowski please review
--
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050886285
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BannedDependenciesBase.java:
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.plugins.enforcer;
+
+import com.google.common.base.Strings;
+import java.util.List;
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugins.enforcer.utils.ArtifactUtils;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.eclipse.aether.graph.DependencyNode;
+
+/**
+ * Abstract base class for rules which validate the transitive
+ * dependency tree by traversing all children and validating every
+ * dependency artifact.
+ */
+public abstract class BannedDependenciesBase extends AbstractNonCacheableEnforcerRule {
Review Comment:
Please make this class final (and probably also package protected) to prevent this from being extended by custom rules. I wouldn't necessarily expose further classes to custom rules.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056805792
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/DependencyConvergence.java:
##########
@@ -51,58 +46,41 @@ public class DependencyConvergence implements EnforcerRule {
private List<String> excludes;
+ private DependencyVersionMap dependencyVersionMap;
+
public void setUniqueVersions(boolean uniqueVersions) {
this.uniqueVersions = uniqueVersions;
}
- // CHECKSTYLE_OFF: LineLength
- /**
- * Uses the {@link EnforcerRuleHelper} to populate the values of the
- * {@link DependencyTreeBuilder#buildDependencyTree(MavenProject, ArtifactRepository, ArtifactFactory, ArtifactMetadataSource, ArtifactFilter, ArtifactCollector)}
- * factory method. <br/>
- * This method simply exists to hide all the ugly lookup that the {@link EnforcerRuleHelper} has to do.
- *
- * @param helper
- * @return a Dependency Node which is the root of the project's dependency tree
- * @throws EnforcerRuleException
- */
- // CHECKSTYLE_ON: LineLength
- private DependencyNode getNode(EnforcerRuleHelper helper) throws EnforcerRuleException {
- try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
- DependencyCollectorBuilder dependencyCollectorBuilder =
- helper.getComponent(DependencyCollectorBuilder.class);
- ArtifactRepository repository = (ArtifactRepository) helper.evaluate("${localRepository}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
- buildingRequest.setLocalRepository(repository);
- ArtifactFilter filter = (Artifact a) ->
- ("compile".equalsIgnoreCase(a.getScope()) || "runtime".equalsIgnoreCase(a.getScope()))
- && !a.isOptional();
-
- return dependencyCollectorBuilder.collectDependencyGraph(buildingRequest, filter);
- } catch (ExpressionEvaluationException | ComponentLookupException e) {
- throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
- } catch (DependencyCollectorBuilderException e) {
- throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
- }
- }
-
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
if (log == null) {
log = helper.getLog();
}
try {
- DependencyNode node = getNode(helper);
- DependencyVersionMap visitor = new DependencyVersionMap(log);
- visitor.setUniqueVersions(uniqueVersions);
- node.accept(visitor);
- List<CharSequence> errorMsgs = new ArrayList<>();
- errorMsgs.addAll(getConvergenceErrorMsgs(visitor.getConflictedVersionNumbers(includes, excludes)));
+ DependencyNode node = ArtifactUtils.resolveTransitiveDependencies(
+ helper,
+ // TODO: use a modified version of ExclusionDependencySelector to process excludes and includes
+ new DependencySelector() {
+ @Override
+ public boolean selectDependency(Dependency dependency) {
+ // regular OptionalDependencySelector only discriminates optional dependencies at level 2+
+ return !dependency.isOptional()
+ // regular ScopeDependencySelector is case-sensitive
+ && !dependency.getScope().equalsIgnoreCase(Artifact.SCOPE_TEST);
+ }
+
+ @Override
+ public DependencySelector deriveChildSelector(DependencyCollectionContext context) {
+ return this;
+ }
+ },
+ new ExclusionDependencySelector());
Review Comment:
the scope selector was not passing tests if the scope case was wrong; but I'll apply toLowerCase in ArtifactUtils.toArtifact(DependencyNode)
--
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-enforcer] pzygielo commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by "pzygielo (via GitHub)" <gi...@apache.org>.
pzygielo commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1416843773
Seems it might have caused MENFORCER-466, as 919fe02 is reported as first bad commit.
--
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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056257034
##########
maven-enforcer-plugin/src/it/projects/banned-dependencies-fail/invoker.properties:
##########
@@ -0,0 +1,18 @@
+# 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.
+
+invoker.buildResult = failure
Review Comment:
Please also add a verify - we should be sure why build fail
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056806120
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/DependencyConvergence.java:
##########
@@ -51,58 +46,41 @@ public class DependencyConvergence implements EnforcerRule {
private List<String> excludes;
+ private DependencyVersionMap dependencyVersionMap;
+
public void setUniqueVersions(boolean uniqueVersions) {
this.uniqueVersions = uniqueVersions;
}
- // CHECKSTYLE_OFF: LineLength
- /**
- * Uses the {@link EnforcerRuleHelper} to populate the values of the
- * {@link DependencyTreeBuilder#buildDependencyTree(MavenProject, ArtifactRepository, ArtifactFactory, ArtifactMetadataSource, ArtifactFilter, ArtifactCollector)}
- * factory method. <br/>
- * This method simply exists to hide all the ugly lookup that the {@link EnforcerRuleHelper} has to do.
- *
- * @param helper
- * @return a Dependency Node which is the root of the project's dependency tree
- * @throws EnforcerRuleException
- */
- // CHECKSTYLE_ON: LineLength
- private DependencyNode getNode(EnforcerRuleHelper helper) throws EnforcerRuleException {
- try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
- DependencyCollectorBuilder dependencyCollectorBuilder =
- helper.getComponent(DependencyCollectorBuilder.class);
- ArtifactRepository repository = (ArtifactRepository) helper.evaluate("${localRepository}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
- buildingRequest.setLocalRepository(repository);
- ArtifactFilter filter = (Artifact a) ->
- ("compile".equalsIgnoreCase(a.getScope()) || "runtime".equalsIgnoreCase(a.getScope()))
- && !a.isOptional();
-
- return dependencyCollectorBuilder.collectDependencyGraph(buildingRequest, filter);
- } catch (ExpressionEvaluationException | ComponentLookupException e) {
- throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
- } catch (DependencyCollectorBuilderException e) {
- throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
- }
- }
-
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
if (log == null) {
log = helper.getLog();
}
try {
- DependencyNode node = getNode(helper);
- DependencyVersionMap visitor = new DependencyVersionMap(log);
- visitor.setUniqueVersions(uniqueVersions);
- node.accept(visitor);
- List<CharSequence> errorMsgs = new ArrayList<>();
- errorMsgs.addAll(getConvergenceErrorMsgs(visitor.getConflictedVersionNumbers(includes, excludes)));
+ DependencyNode node = ArtifactUtils.resolveTransitiveDependencies(
+ helper,
+ // TODO: use a modified version of ExclusionDependencySelector to process excludes and includes
+ new DependencySelector() {
+ @Override
+ public boolean selectDependency(Dependency dependency) {
+ // regular OptionalDependencySelector only discriminates optional dependencies at level 2+
+ return !dependency.isOptional()
+ // regular ScopeDependencySelector is case-sensitive
+ && !dependency.getScope().equalsIgnoreCase(Artifact.SCOPE_TEST);
+ }
+
+ @Override
+ public DependencySelector deriveChildSelector(DependencyCollectionContext context) {
+ return this;
+ }
+ },
+ new ExclusionDependencySelector());
Review Comment:
Changed to:
new OptionalDependencySelector(),
new ScopeDependencySelector(SCOPE_TEST, SCOPE_PROVIDED),
new ExclusionDependencySelector());
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1356110388
I'd also gladly use a DependencySelector to exclude artifacts already at the dependency node retrieval level, but if you say everything is public API, I'm not sure whether I am allowed to modify any class. @slawekjaranowski could you please help out?
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1363744863
@slawekjaranowski your newest it failed. Checking.
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1364530592
It's been so long that I had forgotten the reasons why I did things like I did -- e.g. use bespoke dependency selectors instead of regular ones (the reason for that being that the out of the box selectors only work on transitive 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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050784182
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
I consider this file API (other rules might extend that) so it is backwards-incompatible to remove any non-private methods in there or change their signature. We may want to deprecate things though
--
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050813950
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
See also https://github.com/apache/maven-enforcer/pull/167.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050900528
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BannedDependenciesBase.java:
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.plugins.enforcer;
+
+import com.google.common.base.Strings;
+import java.util.List;
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugins.enforcer.utils.ArtifactUtils;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.eclipse.aether.graph.DependencyNode;
+
+/**
+ * Abstract base class for rules which validate the transitive
+ * dependency tree by traversing all children and validating every
+ * dependency artifact.
+ */
+public abstract class BannedDependenciesBase extends AbstractNonCacheableEnforcerRule {
Review Comment:
A final abstract 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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050880475
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
@slawekjaranowski Which classes do you consider so relevant that we need to ensure backwards-compatibility (again all of them are exposed to custom rules unfortunately)
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1354975012
I see. I was under the impression that the API for custom rule creators was the `enforcer-api` module. That was at least what I used when I created the custom enforcer rule in `versions-maven-plugin`. Didn't know `AbstractBanDependencies` was intended to be used as API for external rules. Perhaps it should be stated more clearly that it's also part of the API for custom rules.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050911744
##########
enforcer-rules/pom.xml:
##########
@@ -112,22 +112,6 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
Review Comment:
Ok, will restore; please keep the review comments coming, I'll process them later.
--
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050795396
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
API here just means it is exported via the class path to all custom rules and can be accessed and extended. It doesn't say anything about design....
Potentially that affects every rule which is not a final class (because there is no limited visibility) but I wouldn't go that far to say that it is strictly necessary to keep backwards compatibility but for abstract public classes I would say yes, we unfortunately need to be backwards compatible here (so first deprecate and only finally remove in m-enforcer-p 4.0.0)
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051432041
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,201 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
+ private static final Map<DependencyNode, Artifact> NODE_ARTIFACT_MAP = new HashMap<>();
+ private static final Map<Artifact, org.eclipse.aether.artifact.Artifact> AETHER_ARTIFACT_MAP = new HashMap<>();
+
+ /**
+ * Converts {@link DependencyNode} to {@link Artifact}; in comparison
+ * to {@linkpl RepositoryUtils#toArtifact(org.eclipse.aether.artifact.Artifact)}, this method
+ * assigns {@link Artifact#getScope()} and {@link Artifact#isOptional()} based on
+ * the dependency information from the node.
+ *
+ * @param node {@link DependencyNode} to convert to {@link Artifact}
+ * @return target artifact
+ */
+ public static Artifact toArtifact(DependencyNode node) {
+ Artifact result = NODE_ARTIFACT_MAP.computeIfAbsent(node, n -> RepositoryUtils.toArtifact(n.getArtifact()));
+ ofNullable(node.getDependency()).ifPresent(dependency -> {
+ ofNullable(dependency.getScope()).ifPresent(result::setScope);
+ result.setOptional(dependency.isOptional());
+ });
Review Comment:
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051430079
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
I donβt think this is explicit enough in the docs. I know of several custom rules which leverage or extend enforcer rules classes.
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1356057706
@olamy could you have a look as well? Thanks!
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051431973
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,201 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
+ private static final Map<DependencyNode, Artifact> NODE_ARTIFACT_MAP = new HashMap<>();
+ private static final Map<Artifact, org.eclipse.aether.artifact.Artifact> AETHER_ARTIFACT_MAP = new HashMap<>();
+
+ /**
+ * Converts {@link DependencyNode} to {@link Artifact}; in comparison
+ * to {@linkpl RepositoryUtils#toArtifact(org.eclipse.aether.artifact.Artifact)}, this method
+ * assigns {@link Artifact#getScope()} and {@link Artifact#isOptional()} based on
+ * the dependency information from the node.
+ *
+ * @param node {@link DependencyNode} to convert to {@link Artifact}
+ * @return target artifact
+ */
+ public static Artifact toArtifact(DependencyNode node) {
+ Artifact result = NODE_ARTIFACT_MAP.computeIfAbsent(node, n -> RepositoryUtils.toArtifact(n.getArtifact()));
+ ofNullable(node.getDependency()).ifPresent(dependency -> {
+ ofNullable(dependency.getScope()).ifPresent(result::setScope);
+ result.setOptional(dependency.isOptional());
+ });
+ AETHER_ARTIFACT_MAP.putIfAbsent(result, node.getArtifact());
+ return result;
+ }
+
+ /**
+ * Converts {@link Artifact} to {@link org.eclipse.aether.artifact.Artifact}
+ * @param artifact source artifact
+ * @return target artifact
+ */
+ public static org.eclipse.aether.artifact.Artifact toArtifact(Artifact artifact) {
+ return AETHER_ARTIFACT_MAP.computeIfAbsent(artifact, RepositoryUtils::toArtifact);
+ }
+
+ /**
+ * Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency
+ * for the current {@link MavenProject}.
+ *
+ * @param helper (may not be null) an instance of the {@link EnforcerRuleHelper} class
+ * @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
+ */
+ public static DependencyNode resolveTransitiveDependencies(
+ EnforcerRuleHelper helper, DependencySelector... selectors) throws EnforcerRuleException {
+ try {
+ RepositorySystem repositorySystem = helper.getComponent(RepositorySystem.class);
+ MavenSession session = (MavenSession) helper.evaluate("${session}");
+ MavenProject project = session.getCurrentProject();
+ ArtifactTypeRegistry artifactTypeRegistry =
+ session.getRepositorySession().getArtifactTypeRegistry();
- public static Set<Artifact> getAllDescendants(DependencyNode node) {
- Set<Artifact> children = null;
- if (node.getChildren() != null) {
- children = new HashSet<>();
- for (DependencyNode depNode : node.getChildren()) {
- children.add(depNode.getArtifact());
- Set<Artifact> subNodes = getAllDescendants(depNode);
- if (subNodes != null) {
- children.addAll(subNodes);
- }
+ DefaultRepositorySystemSession repositorySystemSession =
+ new DefaultRepositorySystemSession(session.getRepositorySession());
+ repositorySystemSession.setDependencyGraphTransformer(new ConflictResolver(
+ new NearestVersionSelector(),
+ new JavaScopeSelector(),
+ new SimpleOptionalitySelector(),
+ new JavaScopeDeriver()));
+ repositorySystemSession.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true);
+ repositorySystemSession.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true);
+ if (selectors.length > 0) {
+ repositorySystemSession.setDependencySelector(new AndDependencySelector(selectors));
}
+
+ CollectRequest collectRequest = new CollectRequest(
+ project.getDependencies().stream()
+ .map(d -> RepositoryUtils.toDependency(d, artifactTypeRegistry))
+ .collect(Collectors.toList()),
+ ofNullable(project.getDependencyManagement())
+ .map(DependencyManagement::getDependencies)
+ .map(list -> list.stream()
+ .map(d -> RepositoryUtils.toDependency(d, artifactTypeRegistry))
+ .collect(Collectors.toList()))
+ .orElse(null),
+ project.getRemoteProjectRepositories());
+ collectRequest.setRootArtifact(ArtifactUtils.toArtifact(project.getArtifact()));
+
+ return repositorySystem
+ .collectDependencies(repositorySystemSession, collectRequest)
+ .getRoot();
+ } catch (ExpressionEvaluationException | ComponentLookupException e) {
+ throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
+ } catch (DependencyCollectionException e) {
+ throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
}
- return children;
}
/**
- * Checks the set of dependencies against the list of patterns.
+ * <p>Retrieves all <em>child</em> dependency artifacts from the given {@link DependencyNode} and returns them
+ * as a set of {@link Artifact}.</p>
+ * <p><u>Note:</u> Thus, the result will not contain the root artifact.</p>
+ * @param node root node
+ * @return set of all <em>child</em> dependency artifacts
+ */
+ public static Set<Artifact> getDependencyArtifacts(DependencyNode node) {
+ return getDependencyArtifacts(node, new HashSet<>());
+ }
+
+ private static Set<Artifact> getDependencyArtifacts(DependencyNode node, Set<Artifact> set) {
+ node.getChildren().forEach(child -> {
+ set.add(toArtifact(child));
+ getDependencyArtifacts(child, set);
+ });
+ return set;
+ }
+
+ /**
+ * Returns a subset of dependency artifacts that match the given collection of patterns
*
- * @param thePatterns the patterns
- * @param dependencies the dependencies
+ * @param dependencies dependency artifacts to match against patterns
+ * @param patterns patterns to match against the artifacts
* @return a set containing artifacts matching one of the patterns or <code>null</code>
* @throws EnforcerRuleException the enforcer rule exception
*/
- public static Set<Artifact> checkDependencies(Set<Artifact> dependencies, List<String> thePatterns)
+ public static Set<Artifact> filterDependencyArtifacts(Set<Artifact> dependencies, Collection<String> patterns)
throws EnforcerRuleException {
- Set<Artifact> foundMatches = null;
-
- if (thePatterns != null && thePatterns.size() > 0) {
-
- for (String pattern : thePatterns) {
- String[] subStrings = pattern.split(":");
- subStrings = StringUtils.stripAll(subStrings);
- String resultPattern = StringUtils.join(subStrings, ":");
+ try {
+ return ofNullable(patterns)
+ .map(collection -> collection.parallelStream()
+ .map(p -> p.split(":"))
+ .map(StringUtils::stripAll)
+ .map(arr -> String.join(":", arr))
+ .flatMap(pattern -> dependencies.parallelStream()
+ .filter(artifact -> compareDependency(pattern, artifact)))
+ .collect(Collectors.toSet()))
Review Comment:
Yes...
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051434788
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,201 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
+ private static final Map<DependencyNode, Artifact> NODE_ARTIFACT_MAP = new HashMap<>();
+ private static final Map<Artifact, org.eclipse.aether.artifact.Artifact> AETHER_ARTIFACT_MAP = new HashMap<>();
+
+ /**
+ * Converts {@link DependencyNode} to {@link Artifact}; in comparison
+ * to {@linkpl RepositoryUtils#toArtifact(org.eclipse.aether.artifact.Artifact)}, this method
+ * assigns {@link Artifact#getScope()} and {@link Artifact#isOptional()} based on
+ * the dependency information from the node.
+ *
+ * @param node {@link DependencyNode} to convert to {@link Artifact}
+ * @return target artifact
+ */
+ public static Artifact toArtifact(DependencyNode node) {
+ Artifact result = NODE_ARTIFACT_MAP.computeIfAbsent(node, n -> RepositoryUtils.toArtifact(n.getArtifact()));
+ ofNullable(node.getDependency()).ifPresent(dependency -> {
+ ofNullable(dependency.getScope()).ifPresent(result::setScope);
+ result.setOptional(dependency.isOptional());
+ });
Review Comment:
Updated: caching the built objects.
--
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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051429294
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
Only classes from `enforcer-api` should be used to write own rules, like we have in docs:
https://maven.apache.org/enforcer/enforcer-api/writing-a-custom-rule.html
even more in `extra-enforcer-rules` internal `enforcer-rules` are baned:
https://github.com/mojohaus/extra-enforcer-rules/blob/master/pom.xml#L147-L152
--
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050795396
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
API here just means it is exported via the class path to all custom rules and can be accessed and extended. It doesn't say anything about design....
Potentially that affects every rule which is not a final class (because there is no limited visibility) but I wouldn't go that far to say that it is strictly necessary to keep backwards compatibility in those classes but at least for abstract public classes I would say we unfortunately need to be backwards compatible here (so first deprecate and only finally remove in m-enforcer-p 4.0.0)
--
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050887758
##########
enforcer-rules/pom.xml:
##########
@@ -112,22 +112,6 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
Review Comment:
you cannot remove that unfortunately without breaking backwards compatibility (maybe add a TODO already that this should be removed with m-enforcer 4.0.0)
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050790825
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
Really??? If so, I consider this a bad API. Especially because a collection of artifacts is unsuitable for a general purpose.
I might as well simply move the rules out of this 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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051432007
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractPropertyEnforcerRule.java:
##########
@@ -32,16 +32,16 @@ public abstract class AbstractPropertyEnforcerRule extends AbstractNonCacheableE
/**
* Match the property value to a given regular expression. Defaults to <code>null</code> (any value is ok).
*
- * @see {@link #setRegex(String)}
- * @see {@link #getRegex()}
+ * @see #setRegex(String)
+ * @see #getRegex()
*/
private String regex = null;
/**
* Specify a warning message if the regular expression is not matched.
*
- * @see {@link #setRegexMessage(String)}
- * @see {@link #getRegexMessage()}
+ * @see #setRegexMessage(String)
+ * @see #getRegexMessage()
Review Comment:
My linter complained about it, but can be done in a separate PR indeed.
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1355137055
Closed by mistake... π
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056198836
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BannedPlugins.java:
##########
@@ -29,12 +28,13 @@
*/
public class BannedPlugins extends BannedDependencies {
@Override
- protected Set<Artifact> getDependenciesToCheck(ProjectBuildingRequest buildingRequest) {
- return buildingRequest.getProject().getPluginArtifacts();
Review Comment:
Corrected that, and added a simple test on banned dependencies failure.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056275747
##########
maven-enforcer-plugin/src/it/projects/banned-dependencies-fail/invoker.properties:
##########
@@ -0,0 +1,18 @@
+# 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.
+
+invoker.buildResult = failure
Review Comment:
ok
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1356090919
Also resolves [MENFORCER-434]
--
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051430079
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
I donβt think this is explicit enough in the docs. I know of several custom rules which leverage or extend enforcer rules classes. See also https://mvnrepository.com/artifact/org.apache.maven.enforcer/enforcer-rules/usages
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051437004
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
Imho, the documentation is clear on how to create custom rules -- enforcer-api must be used. Is non-standard use supported? What if there are projects that depend on the current internal implementation? Should this also 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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051677093
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanTransitiveDependencies.java:
##########
@@ -127,55 +130,27 @@ private static boolean searchTree(DependencyNode node, int level, ArtifactMatche
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
- this.helper = helper;
-
- if (excludes == null) {
- excludes = Collections.emptyList();
- }
- if (includes == null) {
- includes = Collections.emptyList();
- }
-
- final ArtifactMatcher exclusions = new ArtifactMatcher(excludes, includes);
-
- DependencyNode rootNode = null;
-
+ MavenSession session;
try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
-
- rootNode = createDependencyGraphBuilder().buildDependencyGraph(buildingRequest, null);
- } catch (Exception e) {
- throw new EnforcerRuleException("Error: Could not construct dependency tree.", e);
- }
-
- String message = getMessage();
- StringBuilder generatedMessage = null;
- if (message == null) {
- generatedMessage = new StringBuilder();
+ session = (MavenSession) helper.evaluate("${session}");
+ } catch (ExpressionEvaluationException e) {
+ throw new RuntimeException(e);
}
-
+ ArtifactTypeRegistry artifactTypeRegistry =
+ session.getRepositorySession().getArtifactTypeRegistry();
+ ArtifactMatcher exclusions = new ArtifactMatcher(excludes, includes);
+ Set<Dependency> directDependencies = session.getCurrentProject().getDependencies().parallelStream()
+ .map(d -> RepositoryUtils.toDependency(d, artifactTypeRegistry))
+ .collect(Collectors.toSet());
Review Comment:
parallelStream π
I see other 7 places - are all thread safe?
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056805579
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/DependencyConvergence.java:
##########
@@ -51,58 +46,41 @@ public class DependencyConvergence implements EnforcerRule {
private List<String> excludes;
+ private DependencyVersionMap dependencyVersionMap;
+
public void setUniqueVersions(boolean uniqueVersions) {
this.uniqueVersions = uniqueVersions;
}
- // CHECKSTYLE_OFF: LineLength
- /**
- * Uses the {@link EnforcerRuleHelper} to populate the values of the
- * {@link DependencyTreeBuilder#buildDependencyTree(MavenProject, ArtifactRepository, ArtifactFactory, ArtifactMetadataSource, ArtifactFilter, ArtifactCollector)}
- * factory method. <br/>
- * This method simply exists to hide all the ugly lookup that the {@link EnforcerRuleHelper} has to do.
- *
- * @param helper
- * @return a Dependency Node which is the root of the project's dependency tree
- * @throws EnforcerRuleException
- */
- // CHECKSTYLE_ON: LineLength
- private DependencyNode getNode(EnforcerRuleHelper helper) throws EnforcerRuleException {
- try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
- DependencyCollectorBuilder dependencyCollectorBuilder =
- helper.getComponent(DependencyCollectorBuilder.class);
- ArtifactRepository repository = (ArtifactRepository) helper.evaluate("${localRepository}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
- buildingRequest.setLocalRepository(repository);
- ArtifactFilter filter = (Artifact a) ->
- ("compile".equalsIgnoreCase(a.getScope()) || "runtime".equalsIgnoreCase(a.getScope()))
- && !a.isOptional();
-
- return dependencyCollectorBuilder.collectDependencyGraph(buildingRequest, filter);
- } catch (ExpressionEvaluationException | ComponentLookupException e) {
- throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
- } catch (DependencyCollectorBuilderException e) {
- throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
- }
- }
-
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
if (log == null) {
log = helper.getLog();
}
try {
- DependencyNode node = getNode(helper);
- DependencyVersionMap visitor = new DependencyVersionMap(log);
- visitor.setUniqueVersions(uniqueVersions);
- node.accept(visitor);
- List<CharSequence> errorMsgs = new ArrayList<>();
- errorMsgs.addAll(getConvergenceErrorMsgs(visitor.getConflictedVersionNumbers(includes, excludes)));
+ DependencyNode node = ArtifactUtils.resolveTransitiveDependencies(
+ helper,
+ // TODO: use a modified version of ExclusionDependencySelector to process excludes and includes
+ new DependencySelector() {
+ @Override
+ public boolean selectDependency(Dependency dependency) {
+ // regular OptionalDependencySelector only discriminates optional dependencies at level 2+
+ return !dependency.isOptional()
+ // regular ScopeDependencySelector is case-sensitive
+ && !dependency.getScope().equalsIgnoreCase(Artifact.SCOPE_TEST);
+ }
+
+ @Override
+ public DependencySelector deriveChildSelector(DependencyCollectionContext context) {
+ return this;
+ }
+ },
+ new ExclusionDependencySelector());
Review Comment:
> Why we need here special selectors?
It's explained in the comment: // regular OptionalDependencySelector only discriminates optional dependencies at level 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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056822446
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/DependencyConvergence.java:
##########
@@ -51,58 +46,41 @@ public class DependencyConvergence implements EnforcerRule {
private List<String> excludes;
+ private DependencyVersionMap dependencyVersionMap;
+
public void setUniqueVersions(boolean uniqueVersions) {
this.uniqueVersions = uniqueVersions;
}
- // CHECKSTYLE_OFF: LineLength
- /**
- * Uses the {@link EnforcerRuleHelper} to populate the values of the
- * {@link DependencyTreeBuilder#buildDependencyTree(MavenProject, ArtifactRepository, ArtifactFactory, ArtifactMetadataSource, ArtifactFilter, ArtifactCollector)}
- * factory method. <br/>
- * This method simply exists to hide all the ugly lookup that the {@link EnforcerRuleHelper} has to do.
- *
- * @param helper
- * @return a Dependency Node which is the root of the project's dependency tree
- * @throws EnforcerRuleException
- */
- // CHECKSTYLE_ON: LineLength
- private DependencyNode getNode(EnforcerRuleHelper helper) throws EnforcerRuleException {
- try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
- DependencyCollectorBuilder dependencyCollectorBuilder =
- helper.getComponent(DependencyCollectorBuilder.class);
- ArtifactRepository repository = (ArtifactRepository) helper.evaluate("${localRepository}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
- buildingRequest.setLocalRepository(repository);
- ArtifactFilter filter = (Artifact a) ->
- ("compile".equalsIgnoreCase(a.getScope()) || "runtime".equalsIgnoreCase(a.getScope()))
- && !a.isOptional();
-
- return dependencyCollectorBuilder.collectDependencyGraph(buildingRequest, filter);
- } catch (ExpressionEvaluationException | ComponentLookupException e) {
- throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
- } catch (DependencyCollectorBuilderException e) {
- throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
- }
- }
-
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
if (log == null) {
log = helper.getLog();
}
try {
- DependencyNode node = getNode(helper);
- DependencyVersionMap visitor = new DependencyVersionMap(log);
- visitor.setUniqueVersions(uniqueVersions);
- node.accept(visitor);
- List<CharSequence> errorMsgs = new ArrayList<>();
- errorMsgs.addAll(getConvergenceErrorMsgs(visitor.getConflictedVersionNumbers(includes, excludes)));
+ DependencyNode node = ArtifactUtils.resolveTransitiveDependencies(
+ helper,
+ // TODO: use a modified version of ExclusionDependencySelector to process excludes and includes
+ new DependencySelector() {
+ @Override
+ public boolean selectDependency(Dependency dependency) {
+ // regular OptionalDependencySelector only discriminates optional dependencies at level 2+
+ return !dependency.isOptional()
+ // regular ScopeDependencySelector is case-sensitive
+ && !dependency.getScope().equalsIgnoreCase(Artifact.SCOPE_TEST);
+ }
+
+ @Override
+ public DependencySelector deriveChildSelector(DependencyCollectionContext context) {
+ return this;
+ }
+ },
+ new ExclusionDependencySelector());
Review Comment:
@slawekjaranowski unfortunately, out of the box selectors only work on transitive dependencies: direct dependencies are always included. That was the primary reason I used this, I'll put a comment about it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1364536078
I mean, it would be nice if there were also selectors working on direct dependencies, but that would require changes to Resolver π€‘
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056156097
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BannedPlugins.java:
##########
@@ -29,12 +28,13 @@
*/
public class BannedPlugins extends BannedDependencies {
@Override
- protected Set<Artifact> getDependenciesToCheck(ProjectBuildingRequest buildingRequest) {
- return buildingRequest.getProject().getPluginArtifacts();
Review Comment:
No, it doesn't work based on artifact sets anymore.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056805513
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,188 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
import java.util.HashSet;
-import java.util.List;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
- public static Set<Artifact> getAllDescendants(DependencyNode node) {
- Set<Artifact> children = null;
- if (node.getChildren() != null) {
- children = new HashSet<>();
- for (DependencyNode depNode : node.getChildren()) {
- children.add(depNode.getArtifact());
- Set<Artifact> subNodes = getAllDescendants(depNode);
- if (subNodes != null) {
- children.addAll(subNodes);
- }
+ /**
+ * Converts {@link DependencyNode} to {@link Artifact}; in comparison
+ * to {@link RepositoryUtils#toArtifact(org.eclipse.aether.artifact.Artifact)}, this method
+ * assigns {@link Artifact#getScope()} and {@link Artifact#isOptional()} based on
+ * the dependency information from the node.
+ *
+ * @param node {@link DependencyNode} to convert to {@link Artifact}
+ * @return target artifact
+ */
+ public static Artifact toArtifact(DependencyNode node) {
+ Artifact artifact = RepositoryUtils.toArtifact(node.getArtifact());
+ ofNullable(node.getDependency()).ifPresent(dependency -> {
+ ofNullable(dependency.getScope()).ifPresent(artifact::setScope);
+ artifact.setOptional(dependency.isOptional());
+ });
+ return artifact;
+ }
+
+ /**
+ * Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency
+ * for the current {@link MavenProject}.
+ *
+ * @param helper (may not be null) an instance of the {@link EnforcerRuleHelper} class
+ * @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
+ */
+ public static DependencyNode resolveTransitiveDependencies(
+ EnforcerRuleHelper helper, DependencySelector... selectors) throws EnforcerRuleException {
+ try {
+ RepositorySystem repositorySystem = helper.getComponent(RepositorySystem.class);
+ MavenSession session = (MavenSession) helper.evaluate("${session}");
+ MavenProject project = session.getCurrentProject();
+ ArtifactTypeRegistry artifactTypeRegistry =
+ session.getRepositorySession().getArtifactTypeRegistry();
+
+ DefaultRepositorySystemSession repositorySystemSession =
+ new DefaultRepositorySystemSession(session.getRepositorySession());
+ repositorySystemSession.setDependencyGraphTransformer(new ConflictResolver(
+ new NearestVersionSelector(),
+ new JavaScopeSelector(),
+ new SimpleOptionalitySelector(),
+ new JavaScopeDeriver()));
Review Comment:
Ok
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1363647871
Rebased and squashed.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1052618360
##########
maven-enforcer-extension/pom.xml:
##########
@@ -71,6 +71,7 @@
<artifactId>maven-invoker-plugin</artifactId>
<configuration>
<showErrors>true</showErrors>
+ <streamLogs>true</streamLogs>
Review Comment:
Not using streamLogs - as requested.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051437004
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
Imho, the documentation is clear on how to create custom rules -- enforcer-api must be supported. Is non-standard use supported? What if there are projects that depend on the current internal implementation? Should this also 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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051447866
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,201 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
+ private static final Map<DependencyNode, Artifact> NODE_ARTIFACT_MAP = new HashMap<>();
+ private static final Map<Artifact, org.eclipse.aether.artifact.Artifact> AETHER_ARTIFACT_MAP = new HashMap<>();
Review Comment:
Sorry for the confusion. Removed the caching altogether -- the gain is negligible and simplicity is much more important.
--
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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051445542
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
I must agree with Andrzej if someone use internal implementation, we should not stop our improvements in such case.
Documentation say that api should be used nothing else.
Yes we should think to hide internal classes when we can, but sometime it is not possible for technical reason.
--
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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051430883
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,201 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
+ private static final Map<DependencyNode, Artifact> NODE_ARTIFACT_MAP = new HashMap<>();
+ private static final Map<Artifact, org.eclipse.aether.artifact.Artifact> AETHER_ARTIFACT_MAP = new HashMap<>();
+
+ /**
+ * Converts {@link DependencyNode} to {@link Artifact}; in comparison
+ * to {@linkpl RepositoryUtils#toArtifact(org.eclipse.aether.artifact.Artifact)}, this method
+ * assigns {@link Artifact#getScope()} and {@link Artifact#isOptional()} based on
+ * the dependency information from the node.
+ *
+ * @param node {@link DependencyNode} to convert to {@link Artifact}
+ * @return target artifact
+ */
+ public static Artifact toArtifact(DependencyNode node) {
+ Artifact result = NODE_ARTIFACT_MAP.computeIfAbsent(node, n -> RepositoryUtils.toArtifact(n.getArtifact()));
+ ofNullable(node.getDependency()).ifPresent(dependency -> {
+ ofNullable(dependency.getScope()).ifPresent(result::setScope);
+ result.setOptional(dependency.isOptional());
+ });
+ AETHER_ARTIFACT_MAP.putIfAbsent(result, node.getArtifact());
+ return result;
+ }
+
+ /**
+ * Converts {@link Artifact} to {@link org.eclipse.aether.artifact.Artifact}
+ * @param artifact source artifact
+ * @return target artifact
+ */
+ public static org.eclipse.aether.artifact.Artifact toArtifact(Artifact artifact) {
+ return AETHER_ARTIFACT_MAP.computeIfAbsent(artifact, RepositoryUtils::toArtifact);
+ }
+
+ /**
+ * Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency
+ * for the current {@link MavenProject}.
+ *
+ * @param helper (may not be null) an instance of the {@link EnforcerRuleHelper} class
+ * @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
+ */
+ public static DependencyNode resolveTransitiveDependencies(
+ EnforcerRuleHelper helper, DependencySelector... selectors) throws EnforcerRuleException {
+ try {
+ RepositorySystem repositorySystem = helper.getComponent(RepositorySystem.class);
+ MavenSession session = (MavenSession) helper.evaluate("${session}");
+ MavenProject project = session.getCurrentProject();
+ ArtifactTypeRegistry artifactTypeRegistry =
+ session.getRepositorySession().getArtifactTypeRegistry();
- public static Set<Artifact> getAllDescendants(DependencyNode node) {
- Set<Artifact> children = null;
- if (node.getChildren() != null) {
- children = new HashSet<>();
- for (DependencyNode depNode : node.getChildren()) {
- children.add(depNode.getArtifact());
- Set<Artifact> subNodes = getAllDescendants(depNode);
- if (subNodes != null) {
- children.addAll(subNodes);
- }
+ DefaultRepositorySystemSession repositorySystemSession =
+ new DefaultRepositorySystemSession(session.getRepositorySession());
+ repositorySystemSession.setDependencyGraphTransformer(new ConflictResolver(
+ new NearestVersionSelector(),
+ new JavaScopeSelector(),
+ new SimpleOptionalitySelector(),
+ new JavaScopeDeriver()));
+ repositorySystemSession.setConfigProperty(ConflictResolver.CONFIG_PROP_VERBOSE, true);
+ repositorySystemSession.setConfigProperty(DependencyManagerUtils.CONFIG_PROP_VERBOSE, true);
+ if (selectors.length > 0) {
+ repositorySystemSession.setDependencySelector(new AndDependencySelector(selectors));
}
+
+ CollectRequest collectRequest = new CollectRequest(
+ project.getDependencies().stream()
+ .map(d -> RepositoryUtils.toDependency(d, artifactTypeRegistry))
+ .collect(Collectors.toList()),
+ ofNullable(project.getDependencyManagement())
+ .map(DependencyManagement::getDependencies)
+ .map(list -> list.stream()
+ .map(d -> RepositoryUtils.toDependency(d, artifactTypeRegistry))
+ .collect(Collectors.toList()))
+ .orElse(null),
+ project.getRemoteProjectRepositories());
+ collectRequest.setRootArtifact(ArtifactUtils.toArtifact(project.getArtifact()));
+
+ return repositorySystem
+ .collectDependencies(repositorySystemSession, collectRequest)
+ .getRoot();
+ } catch (ExpressionEvaluationException | ComponentLookupException e) {
+ throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
+ } catch (DependencyCollectionException e) {
+ throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
}
- return children;
}
/**
- * Checks the set of dependencies against the list of patterns.
+ * <p>Retrieves all <em>child</em> dependency artifacts from the given {@link DependencyNode} and returns them
+ * as a set of {@link Artifact}.</p>
+ * <p><u>Note:</u> Thus, the result will not contain the root artifact.</p>
+ * @param node root node
+ * @return set of all <em>child</em> dependency artifacts
+ */
+ public static Set<Artifact> getDependencyArtifacts(DependencyNode node) {
+ return getDependencyArtifacts(node, new HashSet<>());
+ }
+
+ private static Set<Artifact> getDependencyArtifacts(DependencyNode node, Set<Artifact> set) {
+ node.getChildren().forEach(child -> {
+ set.add(toArtifact(child));
+ getDependencyArtifacts(child, set);
+ });
+ return set;
+ }
+
+ /**
+ * Returns a subset of dependency artifacts that match the given collection of patterns
*
- * @param thePatterns the patterns
- * @param dependencies the dependencies
+ * @param dependencies dependency artifacts to match against patterns
+ * @param patterns patterns to match against the artifacts
* @return a set containing artifacts matching one of the patterns or <code>null</code>
* @throws EnforcerRuleException the enforcer rule exception
*/
- public static Set<Artifact> checkDependencies(Set<Artifact> dependencies, List<String> thePatterns)
+ public static Set<Artifact> filterDependencyArtifacts(Set<Artifact> dependencies, Collection<String> patterns)
throws EnforcerRuleException {
- Set<Artifact> foundMatches = null;
-
- if (thePatterns != null && thePatterns.size() > 0) {
-
- for (String pattern : thePatterns) {
- String[] subStrings = pattern.split(":");
- subStrings = StringUtils.stripAll(subStrings);
- String resultPattern = StringUtils.join(subStrings, ":");
+ try {
+ return ofNullable(patterns)
+ .map(collection -> collection.parallelStream()
+ .map(p -> p.split(":"))
+ .map(StringUtils::stripAll)
+ .map(arr -> String.join(":", arr))
+ .flatMap(pattern -> dependencies.parallelStream()
+ .filter(artifact -> compareDependency(pattern, artifact)))
+ .collect(Collectors.toSet()))
Review Comment:
`parallelStream` and not thread safe `Collectors.toSet()` π
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractPropertyEnforcerRule.java:
##########
@@ -32,16 +32,16 @@ public abstract class AbstractPropertyEnforcerRule extends AbstractNonCacheableE
/**
* Match the property value to a given regular expression. Defaults to <code>null</code> (any value is ok).
*
- * @see {@link #setRegex(String)}
- * @see {@link #getRegex()}
+ * @see #setRegex(String)
+ * @see #getRegex()
*/
private String regex = null;
/**
* Specify a warning message if the regular expression is not matched.
*
- * @see {@link #setRegexMessage(String)}
- * @see {@link #getRegexMessage()}
+ * @see #setRegexMessage(String)
+ * @see #getRegexMessage()
Review Comment:
Why such change ... it is connected somehow?
π
I would like to do such changes in separate PR if possible.
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,201 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
+ private static final Map<DependencyNode, Artifact> NODE_ARTIFACT_MAP = new HashMap<>();
+ private static final Map<Artifact, org.eclipse.aether.artifact.Artifact> AETHER_ARTIFACT_MAP = new HashMap<>();
+
+ /**
+ * Converts {@link DependencyNode} to {@link Artifact}; in comparison
+ * to {@linkpl RepositoryUtils#toArtifact(org.eclipse.aether.artifact.Artifact)}, this method
+ * assigns {@link Artifact#getScope()} and {@link Artifact#isOptional()} based on
+ * the dependency information from the node.
+ *
+ * @param node {@link DependencyNode} to convert to {@link Artifact}
+ * @return target artifact
+ */
+ public static Artifact toArtifact(DependencyNode node) {
+ Artifact result = NODE_ARTIFACT_MAP.computeIfAbsent(node, n -> RepositoryUtils.toArtifact(n.getArtifact()));
+ ofNullable(node.getDependency()).ifPresent(dependency -> {
+ ofNullable(dependency.getScope()).ifPresent(result::setScope);
+ result.setOptional(dependency.isOptional());
+ });
Review Comment:
This will be computed always and we change item which is in `NODE_ARTIFACT_MAP` ...
So result in multithread can be corrupted by other thread - we have the same instance.
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,201 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
+ private static final Map<DependencyNode, Artifact> NODE_ARTIFACT_MAP = new HashMap<>();
+ private static final Map<Artifact, org.eclipse.aether.artifact.Artifact> AETHER_ARTIFACT_MAP = new HashMap<>();
Review Comment:
Should be documented - why static is used here.
I guess that is for cache purpose ... but is it thread safe?
--
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-enforcer] ajarmoniuk commented on pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#issuecomment-1356410168
Looks like javadoc linter was upset with `BannedDependenciesBase` (which I'd rather not have and use the old `AbstractBannedDependencies` instead). Corrected.
--
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050910244
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BannedDependenciesBase.java:
##########
@@ -0,0 +1,195 @@
+/*
+ * 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.plugins.enforcer;
+
+import com.google.common.base.Strings;
+import java.util.List;
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugins.enforcer.utils.ArtifactUtils;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.eclipse.aether.graph.DependencyNode;
+
+/**
+ * Abstract base class for rules which validate the transitive
+ * dependency tree by traversing all children and validating every
+ * dependency artifact.
+ */
+public abstract class BannedDependenciesBase extends AbstractNonCacheableEnforcerRule {
Review Comment:
Sorry, just package protected obviously.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056159955
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BannedPlugins.java:
##########
@@ -29,12 +28,13 @@
*/
public class BannedPlugins extends BannedDependencies {
@Override
- protected Set<Artifact> getDependenciesToCheck(ProjectBuildingRequest buildingRequest) {
- return buildingRequest.getProject().getPluginArtifacts();
Review Comment:
Apologies for that, let me check why it's failing and I'll create more tests once done.
--
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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056796605
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/DependencyConvergence.java:
##########
@@ -51,58 +46,41 @@ public class DependencyConvergence implements EnforcerRule {
private List<String> excludes;
+ private DependencyVersionMap dependencyVersionMap;
+
public void setUniqueVersions(boolean uniqueVersions) {
this.uniqueVersions = uniqueVersions;
}
- // CHECKSTYLE_OFF: LineLength
- /**
- * Uses the {@link EnforcerRuleHelper} to populate the values of the
- * {@link DependencyTreeBuilder#buildDependencyTree(MavenProject, ArtifactRepository, ArtifactFactory, ArtifactMetadataSource, ArtifactFilter, ArtifactCollector)}
- * factory method. <br/>
- * This method simply exists to hide all the ugly lookup that the {@link EnforcerRuleHelper} has to do.
- *
- * @param helper
- * @return a Dependency Node which is the root of the project's dependency tree
- * @throws EnforcerRuleException
- */
- // CHECKSTYLE_ON: LineLength
- private DependencyNode getNode(EnforcerRuleHelper helper) throws EnforcerRuleException {
- try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
- DependencyCollectorBuilder dependencyCollectorBuilder =
- helper.getComponent(DependencyCollectorBuilder.class);
- ArtifactRepository repository = (ArtifactRepository) helper.evaluate("${localRepository}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
- buildingRequest.setLocalRepository(repository);
- ArtifactFilter filter = (Artifact a) ->
- ("compile".equalsIgnoreCase(a.getScope()) || "runtime".equalsIgnoreCase(a.getScope()))
- && !a.isOptional();
-
- return dependencyCollectorBuilder.collectDependencyGraph(buildingRequest, filter);
- } catch (ExpressionEvaluationException | ComponentLookupException e) {
- throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
- } catch (DependencyCollectorBuilderException e) {
- throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
- }
- }
-
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
if (log == null) {
log = helper.getLog();
}
try {
- DependencyNode node = getNode(helper);
- DependencyVersionMap visitor = new DependencyVersionMap(log);
- visitor.setUniqueVersions(uniqueVersions);
- node.accept(visitor);
- List<CharSequence> errorMsgs = new ArrayList<>();
- errorMsgs.addAll(getConvergenceErrorMsgs(visitor.getConflictedVersionNumbers(includes, excludes)));
+ DependencyNode node = ArtifactUtils.resolveTransitiveDependencies(
+ helper,
+ // TODO: use a modified version of ExclusionDependencySelector to process excludes and includes
+ new DependencySelector() {
+ @Override
+ public boolean selectDependency(Dependency dependency) {
+ // regular OptionalDependencySelector only discriminates optional dependencies at level 2+
+ return !dependency.isOptional()
+ // regular ScopeDependencySelector is case-sensitive
+ && !dependency.getScope().equalsIgnoreCase(Artifact.SCOPE_TEST);
+ }
+
+ @Override
+ public DependencySelector deriveChildSelector(DependencyCollectionContext context) {
+ return this;
+ }
+ },
+ new ExclusionDependencySelector());
Review Comment:
Why we need here special selectors?
provided scope is missing for exclusion - see MENFORCER-407
https://github.com/apache/maven/blob/master/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,188 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
import java.util.HashSet;
-import java.util.List;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
- public static Set<Artifact> getAllDescendants(DependencyNode node) {
- Set<Artifact> children = null;
- if (node.getChildren() != null) {
- children = new HashSet<>();
- for (DependencyNode depNode : node.getChildren()) {
- children.add(depNode.getArtifact());
- Set<Artifact> subNodes = getAllDescendants(depNode);
- if (subNodes != null) {
- children.addAll(subNodes);
- }
+ /**
+ * Converts {@link DependencyNode} to {@link Artifact}; in comparison
+ * to {@link RepositoryUtils#toArtifact(org.eclipse.aether.artifact.Artifact)}, this method
+ * assigns {@link Artifact#getScope()} and {@link Artifact#isOptional()} based on
+ * the dependency information from the node.
+ *
+ * @param node {@link DependencyNode} to convert to {@link Artifact}
+ * @return target artifact
+ */
+ public static Artifact toArtifact(DependencyNode node) {
+ Artifact artifact = RepositoryUtils.toArtifact(node.getArtifact());
+ ofNullable(node.getDependency()).ifPresent(dependency -> {
+ ofNullable(dependency.getScope()).ifPresent(artifact::setScope);
+ artifact.setOptional(dependency.isOptional());
+ });
+ return artifact;
+ }
+
+ /**
+ * Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency
+ * for the current {@link MavenProject}.
+ *
+ * @param helper (may not be null) an instance of the {@link EnforcerRuleHelper} class
+ * @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
+ */
+ public static DependencyNode resolveTransitiveDependencies(
+ EnforcerRuleHelper helper, DependencySelector... selectors) throws EnforcerRuleException {
+ try {
+ RepositorySystem repositorySystem = helper.getComponent(RepositorySystem.class);
+ MavenSession session = (MavenSession) helper.evaluate("${session}");
+ MavenProject project = session.getCurrentProject();
+ ArtifactTypeRegistry artifactTypeRegistry =
+ session.getRepositorySession().getArtifactTypeRegistry();
+
+ DefaultRepositorySystemSession repositorySystemSession =
+ new DefaultRepositorySystemSession(session.getRepositorySession());
+ repositorySystemSession.setDependencyGraphTransformer(new ConflictResolver(
+ new NearestVersionSelector(),
+ new JavaScopeSelector(),
+ new SimpleOptionalitySelector(),
+ new JavaScopeDeriver()));
Review Comment:
This is default settings provided by Maven - why we need to change?
https://github.com/apache/maven/blob/master/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/MavenRepositorySystemUtils.java
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056806899
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/DependencyConvergence.java:
##########
@@ -51,58 +46,41 @@ public class DependencyConvergence implements EnforcerRule {
private List<String> excludes;
+ private DependencyVersionMap dependencyVersionMap;
+
public void setUniqueVersions(boolean uniqueVersions) {
this.uniqueVersions = uniqueVersions;
}
- // CHECKSTYLE_OFF: LineLength
- /**
- * Uses the {@link EnforcerRuleHelper} to populate the values of the
- * {@link DependencyTreeBuilder#buildDependencyTree(MavenProject, ArtifactRepository, ArtifactFactory, ArtifactMetadataSource, ArtifactFilter, ArtifactCollector)}
- * factory method. <br/>
- * This method simply exists to hide all the ugly lookup that the {@link EnforcerRuleHelper} has to do.
- *
- * @param helper
- * @return a Dependency Node which is the root of the project's dependency tree
- * @throws EnforcerRuleException
- */
- // CHECKSTYLE_ON: LineLength
- private DependencyNode getNode(EnforcerRuleHelper helper) throws EnforcerRuleException {
- try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
- DependencyCollectorBuilder dependencyCollectorBuilder =
- helper.getComponent(DependencyCollectorBuilder.class);
- ArtifactRepository repository = (ArtifactRepository) helper.evaluate("${localRepository}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
- buildingRequest.setLocalRepository(repository);
- ArtifactFilter filter = (Artifact a) ->
- ("compile".equalsIgnoreCase(a.getScope()) || "runtime".equalsIgnoreCase(a.getScope()))
- && !a.isOptional();
-
- return dependencyCollectorBuilder.collectDependencyGraph(buildingRequest, filter);
- } catch (ExpressionEvaluationException | ComponentLookupException e) {
- throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
- } catch (DependencyCollectorBuilderException e) {
- throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
- }
- }
-
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
if (log == null) {
log = helper.getLog();
}
try {
- DependencyNode node = getNode(helper);
- DependencyVersionMap visitor = new DependencyVersionMap(log);
- visitor.setUniqueVersions(uniqueVersions);
- node.accept(visitor);
- List<CharSequence> errorMsgs = new ArrayList<>();
- errorMsgs.addAll(getConvergenceErrorMsgs(visitor.getConflictedVersionNumbers(includes, excludes)));
+ DependencyNode node = ArtifactUtils.resolveTransitiveDependencies(
+ helper,
+ // TODO: use a modified version of ExclusionDependencySelector to process excludes and includes
+ new DependencySelector() {
+ @Override
+ public boolean selectDependency(Dependency dependency) {
+ // regular OptionalDependencySelector only discriminates optional dependencies at level 2+
+ return !dependency.isOptional()
+ // regular ScopeDependencySelector is case-sensitive
+ && !dependency.getScope().equalsIgnoreCase(Artifact.SCOPE_TEST);
+ }
+
+ @Override
+ public DependencySelector deriveChildSelector(DependencyCollectionContext context) {
+ return this;
+ }
+ },
+ new ExclusionDependencySelector());
Review Comment:
Nope:
[INFO] The build exited with code 1. See /Users/ajarmoniuk/IdeaProjects/maven-enforcer/maven-enforcer-plugin/target/it/dependencies_not_converging_test_scope/build.log for details.
[INFO] dependencies_not_converging_test_scope/pom.xml ... FAILED (2.0 s)
--
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-enforcer] slawekjaranowski merged pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski merged PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056199938
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BannedPlugins.java:
##########
@@ -29,12 +28,13 @@
*/
public class BannedPlugins extends BannedDependencies {
@Override
- protected Set<Artifact> getDependenciesToCheck(ProjectBuildingRequest buildingRequest) {
- return buildingRequest.getProject().getPluginArtifacts();
Review Comment:
please approve the flow π€‘
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056807536
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/DependencyConvergence.java:
##########
@@ -51,58 +46,41 @@ public class DependencyConvergence implements EnforcerRule {
private List<String> excludes;
+ private DependencyVersionMap dependencyVersionMap;
+
public void setUniqueVersions(boolean uniqueVersions) {
this.uniqueVersions = uniqueVersions;
}
- // CHECKSTYLE_OFF: LineLength
- /**
- * Uses the {@link EnforcerRuleHelper} to populate the values of the
- * {@link DependencyTreeBuilder#buildDependencyTree(MavenProject, ArtifactRepository, ArtifactFactory, ArtifactMetadataSource, ArtifactFilter, ArtifactCollector)}
- * factory method. <br/>
- * This method simply exists to hide all the ugly lookup that the {@link EnforcerRuleHelper} has to do.
- *
- * @param helper
- * @return a Dependency Node which is the root of the project's dependency tree
- * @throws EnforcerRuleException
- */
- // CHECKSTYLE_ON: LineLength
- private DependencyNode getNode(EnforcerRuleHelper helper) throws EnforcerRuleException {
- try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
- DependencyCollectorBuilder dependencyCollectorBuilder =
- helper.getComponent(DependencyCollectorBuilder.class);
- ArtifactRepository repository = (ArtifactRepository) helper.evaluate("${localRepository}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
- buildingRequest.setLocalRepository(repository);
- ArtifactFilter filter = (Artifact a) ->
- ("compile".equalsIgnoreCase(a.getScope()) || "runtime".equalsIgnoreCase(a.getScope()))
- && !a.isOptional();
-
- return dependencyCollectorBuilder.collectDependencyGraph(buildingRequest, filter);
- } catch (ExpressionEvaluationException | ComponentLookupException e) {
- throw new EnforcerRuleException("Unable to lookup a component " + e.getLocalizedMessage(), e);
- } catch (DependencyCollectorBuilderException e) {
- throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
- }
- }
-
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
if (log == null) {
log = helper.getLog();
}
try {
- DependencyNode node = getNode(helper);
- DependencyVersionMap visitor = new DependencyVersionMap(log);
- visitor.setUniqueVersions(uniqueVersions);
- node.accept(visitor);
- List<CharSequence> errorMsgs = new ArrayList<>();
- errorMsgs.addAll(getConvergenceErrorMsgs(visitor.getConflictedVersionNumbers(includes, excludes)));
+ DependencyNode node = ArtifactUtils.resolveTransitiveDependencies(
+ helper,
+ // TODO: use a modified version of ExclusionDependencySelector to process excludes and includes
+ new DependencySelector() {
+ @Override
+ public boolean selectDependency(Dependency dependency) {
+ // regular OptionalDependencySelector only discriminates optional dependencies at level 2+
+ return !dependency.isOptional()
+ // regular ScopeDependencySelector is case-sensitive
+ && !dependency.getScope().equalsIgnoreCase(Artifact.SCOPE_TEST);
+ }
+
+ @Override
+ public DependencySelector deriveChildSelector(DependencyCollectionContext context) {
+ return this;
+ }
+ },
+ new ExclusionDependencySelector());
Review Comment:
will check why
--
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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1056152169
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BannedPlugins.java:
##########
@@ -29,12 +28,13 @@
*/
public class BannedPlugins extends BannedDependencies {
@Override
- protected Set<Artifact> getDependenciesToCheck(ProjectBuildingRequest buildingRequest) {
- return buildingRequest.getProject().getPluginArtifacts();
Review Comment:
This is important ... each extended rule must provided a dependency to check in BannedDependencies
--
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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1052585967
##########
maven-enforcer-extension/pom.xml:
##########
@@ -71,6 +71,7 @@
<artifactId>maven-invoker-plugin</artifactId>
<configuration>
<showErrors>true</showErrors>
+ <streamLogs>true</streamLogs>
Review Comment:
please don't use `streamLogs` - `streamLogsOnFailures` is configured in parent and should be enough in error case
--
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-enforcer] slawekjaranowski commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1052733966
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
@kwin I checked list from mvnrepository ... only few of project was released in this year. Another are very old ...
I think we can take a such risik and go forward.
By the way documentation should be more clear in this point - TODO in next PR.
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051434499
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/utils/ArtifactUtils.java:
##########
@@ -18,70 +18,201 @@
*/
package org.apache.maven.plugins.enforcer.utils;
+import static java.util.Optional.ofNullable;
+
+import java.util.Collection;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
+import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.RepositoryUtils;
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rule.api.EnforcerRuleHelper;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugins.enforcer.utils.ArtifactMatcher.Pattern;
-import org.apache.maven.shared.dependency.graph.DependencyNode;
+import org.apache.maven.project.MavenProject;
+import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluationException;
+import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.RepositorySystem;
+import org.eclipse.aether.artifact.ArtifactTypeRegistry;
+import org.eclipse.aether.collection.CollectRequest;
+import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.collection.DependencySelector;
+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.transformer.ConflictResolver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeDeriver;
+import org.eclipse.aether.util.graph.transformer.JavaScopeSelector;
+import org.eclipse.aether.util.graph.transformer.NearestVersionSelector;
+import org.eclipse.aether.util.graph.transformer.SimpleOptionalitySelector;
/**
*
* @author Robert Scholte
* @since 3.0.0
*/
public final class ArtifactUtils {
- private ArtifactUtils() {}
+ private static final Map<DependencyNode, Artifact> NODE_ARTIFACT_MAP = new HashMap<>();
+ private static final Map<Artifact, org.eclipse.aether.artifact.Artifact> AETHER_ARTIFACT_MAP = new HashMap<>();
Review Comment:
ConcurrentHashMap or should I rather move this to the instance?
--
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-enforcer] ajarmoniuk commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1051782468
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/BanTransitiveDependencies.java:
##########
@@ -127,55 +130,27 @@ private static boolean searchTree(DependencyNode node, int level, ArtifactMatche
@Override
public void execute(EnforcerRuleHelper helper) throws EnforcerRuleException {
- this.helper = helper;
-
- if (excludes == null) {
- excludes = Collections.emptyList();
- }
- if (includes == null) {
- includes = Collections.emptyList();
- }
-
- final ArtifactMatcher exclusions = new ArtifactMatcher(excludes, includes);
-
- DependencyNode rootNode = null;
-
+ MavenSession session;
try {
- MavenProject project = (MavenProject) helper.evaluate("${project}");
- MavenSession session = (MavenSession) helper.evaluate("${session}");
-
- ProjectBuildingRequest buildingRequest =
- new DefaultProjectBuildingRequest(session.getProjectBuildingRequest());
- buildingRequest.setProject(project);
-
- rootNode = createDependencyGraphBuilder().buildDependencyGraph(buildingRequest, null);
- } catch (Exception e) {
- throw new EnforcerRuleException("Error: Could not construct dependency tree.", e);
- }
-
- String message = getMessage();
- StringBuilder generatedMessage = null;
- if (message == null) {
- generatedMessage = new StringBuilder();
+ session = (MavenSession) helper.evaluate("${session}");
+ } catch (ExpressionEvaluationException e) {
+ throw new RuntimeException(e);
}
-
+ ArtifactTypeRegistry artifactTypeRegistry =
+ session.getRepositorySession().getArtifactTypeRegistry();
+ ArtifactMatcher exclusions = new ArtifactMatcher(excludes, includes);
+ Set<Dependency> directDependencies = session.getCurrentProject().getDependencies().parallelStream()
+ .map(d -> RepositoryUtils.toDependency(d, artifactTypeRegistry))
+ .collect(Collectors.toSet());
Review Comment:
Collectors are not. noneMatch, anyMatch should be safe, but I can replace those too just to be on the safe side.
--
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-enforcer] kwin commented on a diff in pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #198:
URL: https://github.com/apache/maven-enforcer/pull/198#discussion_r1050795396
##########
enforcer-rules/src/main/java/org/apache/maven/plugins/enforcer/AbstractBanDependencies.java:
##########
@@ -18,22 +18,15 @@
*/
Review Comment:
API here just means it is exported via the class path to all custom rules and can be accessed and extended. It doesn't say anything about design....
Potentially that affects every rule which is not a final class (because there is no limited visibility) but I wouldn't go that far to say that it is strictly necessary to keep backwards compatibility in those classes but at least for abstract public classes I would say we unfortunately need to be backwards compatible here (so first deprecate and only finally remove in m-enforcer-p 4.0.0)
Custom rules also transitively get the dependency towards `maven-dependency-tree` so we must not remove this either!
--
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-enforcer] ajarmoniuk closed pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
Posted by GitBox <gi...@apache.org>.
ajarmoniuk closed pull request #198: [MENFORCER-435] Replacing maven-compat and maven-dependency-tree usage with Resolver
URL: https://github.com/apache/maven-enforcer/pull/198
--
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