slachiewicz commented on code in PR #566:
URL: https://github.com/apache/felix-dev/pull/566#discussion_r4194571724
##########
tools/maven-bundle-plugin/src/main/java/org/apache/felix/bundleplugin/BundlePlugin.java:
##########
@@ -956,6 +954,37 @@ public boolean updateExcludesInDeps( MavenProject project,
List<Dependency> depe
}
+ /**
+ * Collects the dependency graph of the project after conflict resolution:
the root stands for the project, its
+ * children for the direct dependencies, and so on. Only POMs are
downloaded, no artifact files. This is what
+ * maven-dependency-tree's {@code DependencyGraphBuilder} did on top of
the same Maven API.
+ */
+ protected DependencyNode collectDependencyGraph( MavenProject project )
throws DependencyResolutionException
+ {
+ DependencyResolutionRequest request =
+ new DefaultDependencyResolutionRequest( project,
session.getRepositorySession() );
+ request.setResolutionFilter( ( node, parents ) -> false );
+ return projectDependenciesResolver.resolve( request
).getDependencyGraph();
+ }
+
+ /**
+ * The artifact of a node of {@link
#collectDependencyGraph(MavenProject)}, with the scope and optional flag of its
+ * dependency; for the root, which has no dependency, the artifact of the
project.
+ */
+ protected static Artifact toArtifact( DependencyNode node, MavenProject
project )
+ {
+ org.eclipse.aether.graph.Dependency dependency = node.getDependency();
+ if ( dependency == null )
+ {
+ return project.getArtifact();
Review Comment:
Good catch on the aliasing, though it isn't new: maven-dependency-tree 3.3.0
also used the live object for the root. `DefaultDependencyGraphBuilder` calls
`buildDependencyNode(null, graph, project.getArtifact(), filter)` and
`DefaultDependencyNode` stores that artifact as is, so the old
`node.getArtifact().setFile(...)` already pointed the project's main artifact
at the repository copy.
Rather than copying the root, 49d33b2 removes that `setFile`: nothing reads
the node's file afterwards (child projects are built from the resolved
`artifact`, and `bundleRoot` skips the project the goal runs on), and
`resolveArtifact` only reads its argument. So `bundleall` no longer touches the
project's artifact at all. ITs pass, and `bundleall` output still matches 6.2.0.
##########
tools/maven-bundle-plugin/src/main/java/org/apache/felix/bundleplugin/BundleAllPlugin.java:
##########
@@ -229,16 +225,16 @@ protected BundleInfo bundleAll( MavenProject project, int
maxDepth ) throws Mojo
continue;
}
- throw new MojoExecutionException( "Artifact was not found in
the repo" + node.getArtifact(), e );
+ throw new MojoExecutionException( "Artifact was not found in
the repo" + nodeArtifact, e );
}
- node.getArtifact().setFile( artifact.getFile() );
+ nodeArtifact.setFile( artifact.getFile() );
Review Comment:
Agreed, it was a dead store, and for the root the aliasing above; removed in
49d33b2.
--
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]
