mthmulders commented on code in PR #1571: URL: https://github.com/apache/maven-dependency-plugin/pull/1571#discussion_r2661128678
########## src/main/java/org/apache/maven/plugins/dependency/fromDependencies/GraphRootMatcher.java: ########## @@ -0,0 +1,41 @@ +/* + * 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.dependency.fromDependencies; + +import org.apache.maven.model.Dependency; + +class GraphRootMatcher implements DependencyMatcher { + + private final GraphRoot graphRoot; + + GraphRootMatcher(GraphRoot graphRoot) { + this.graphRoot = graphRoot; + } + + @Override + public boolean matches(Dependency dependency) { + return matches(graphRoot.getGroupId(), dependency.getGroupId()) Review Comment: I think you could've used `Objects.equals()` here - it's available since Java 1.7, according to its Javadoc. ########## src/site/apt/examples/copying-project-dependencies.apt.vm: ########## @@ -96,4 +96,52 @@ Copying project dependencies </build> [...] </project> -+---+ \ No newline at end of file ++---+ + +* Subtrees + + By default the pom is considered the graphRoot, but it can be useful to get the jars of one dependency and all its transitive dependencies. Review Comment: ```suggestion By default, the pom is considered the root of the dependency tree, but it can be useful to get the jars of one dependency and all its transitive dependencies. ``` ########## src/main/java/org/apache/maven/plugins/dependency/fromDependencies/AbstractDependencyFilterMojo.java: ########## @@ -231,6 +237,16 @@ public abstract class AbstractDependencyFilterMojo extends AbstractDependencyMoj @Parameter(property = "mdep.prependGroupId", defaultValue = "false") protected boolean prependGroupId = false; + /** + * By default this matches the project itself is the root. + * With graphRoots you can choose for a subtree of dependencies. + * After that the general include/exclude filters can be applied. + * + * @since 3.10.0 + */ + @Parameter + private List<GraphRoot> graphRoots; Review Comment: Given that `graphRoots` is a list, wouldn't it make sense to have an IT that verifies the collection of dependencies with *multiple* `graphRoots` defined? ########## src/main/java/org/apache/maven/plugins/dependency/fromDependencies/GraphRoot.java: ########## @@ -0,0 +1,55 @@ +/* + * 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.dependency.fromDependencies; + +/** + * All values are static, no expression, so matcher can use equals Review Comment: But the matcher does not use `equals()`? ########## src/main/java/org/apache/maven/plugins/dependency/fromDependencies/AbstractDependencyFilterMojo.java: ########## @@ -231,6 +237,16 @@ public abstract class AbstractDependencyFilterMojo extends AbstractDependencyMoj @Parameter(property = "mdep.prependGroupId", defaultValue = "false") protected boolean prependGroupId = false; + /** + * By default this matches the project itself is the root. + * With graphRoots you can choose for a subtree of dependencies. + * After that the general include/exclude filters can be applied. + * + * @since 3.10.0 + */ Review Comment: ```suggestion /** * By default, this goal uses the project itself as the root of the dependency tree. * With graphRoots, you can select a subtree of dependencies. * After that, the general include/exclude filters can be applied. * * @since 3.10.0 */ ``` ########## src/main/java/org/apache/maven/plugins/dependency/fromDependencies/OrDependencyMatcher.java: ########## @@ -0,0 +1,43 @@ +/* + * 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.dependency.fromDependencies; + +import java.util.ArrayList; +import java.util.Collection; + +import org.apache.maven.model.Dependency; + +class OrDependencyMatcher implements DependencyMatcher { Review Comment: Personally, I would've added unit tests for this class. ########## src/it/projects/copy-dependencies-graphroots/pom.xml: ########## @@ -0,0 +1,103 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> + +<project xmlns="http://maven.apache.org/POM/4.0.0"; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + + <parent> + <groupId>org.apache</groupId> + <artifactId>apache</artifactId> + <version>5</version> + </parent> + + <groupId>org.apache.maven.its.dependency</groupId> + <artifactId>test</artifactId> + <version>1.0-SNAPSHOT</version> + + <name>Test</name> + <description> + Test dependency:copy-dependencies Review Comment: ```suggestion Test dependency:copy-dependencies using graphRoots ``` ########## src/main/java/org/apache/maven/plugins/dependency/fromDependencies/AbstractDependencyFilterMojo.java: ########## @@ -479,6 +502,38 @@ private Set<Artifact> resolve(Set<org.eclipse.aether.artifact.Artifact> artifact return resolvedArtifacts; } + private Set<Artifact> collectArtifacts(MavenProject project) throws DependencyResolutionException { + if (graphRoots == null || graphRoots.isEmpty()) { + return project.getArtifacts(); + } else { + List<DependencyMatcher> filterMatchers = + graphRoots.stream().map(GraphRootMatcher::new).collect(Collectors.toList()); + + DependencyMatcher subTreeMatcher = new OrDependencyMatcher(filterMatchers); + + Set<Artifact> artifacts = new HashSet<>(); + for (Dependency dep : project.getDependencies()) { + if (subTreeMatcher.matches(dep)) { + artifacts.addAll(resolveDependencyArtifacts(dep)); + } + } + return artifacts; + } + } + + private Set<Artifact> resolveDependencyArtifacts(Dependency root) throws DependencyResolutionException { + org.eclipse.aether.graph.Dependency dependency = RepositoryUtils.toDependency( + root, session.getRepositorySession().getArtifactTypeRegistry()); + + List<RemoteRepository> remoteRepositories = + RepositoryUtils.toRepos(session.getProjectBuildingRequest().getRemoteRepositories()); + + Collection<org.eclipse.aether.artifact.Artifact> depArtifacts = Review Comment: To clarify: we do full (transitive) dependency resolution only when the user specified one or more `graphRoot`s, but not when the project itself is the `graphRoot`. Does that make sense? ########## src/main/java/org/apache/maven/plugins/dependency/fromDependencies/GraphRootMatcher.java: ########## @@ -0,0 +1,41 @@ +/* + * 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.dependency.fromDependencies; + +import org.apache.maven.model.Dependency; + +class GraphRootMatcher implements DependencyMatcher { Review Comment: Personally, I would've added unit tests for this class. ########## src/main/java/org/apache/maven/plugins/dependency/fromDependencies/AbstractDependencyFilterMojo.java: ########## @@ -479,6 +502,38 @@ private Set<Artifact> resolve(Set<org.eclipse.aether.artifact.Artifact> artifact return resolvedArtifacts; } + private Set<Artifact> collectArtifacts(MavenProject project) throws DependencyResolutionException { + if (graphRoots == null || graphRoots.isEmpty()) { + return project.getArtifacts(); + } else { + List<DependencyMatcher> filterMatchers = + graphRoots.stream().map(GraphRootMatcher::new).collect(Collectors.toList()); + + DependencyMatcher subTreeMatcher = new OrDependencyMatcher(filterMatchers); + + Set<Artifact> artifacts = new HashSet<>(); + for (Dependency dep : project.getDependencies()) { + if (subTreeMatcher.matches(dep)) { + artifacts.addAll(resolveDependencyArtifacts(dep)); + } + } Review Comment: For consistent code style, did you consider using Streams here as well, just like a few lines above? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
