From d86baf06b719f3c7eaaa2f1dc889dacaa36809b6 Mon Sep 17 00:00:00 2001 From: daz Date: Sun, 9 Jul 2023 21:31:56 -0600 Subject: [PATCH 1/2] Report dependencies by declaring project This change aims to reduce redundant dependency reporting in repository snapshots, with the aim to make the result work better with the GitHub Dependency Graph. Previously, a dependency was reported for every project that referenced it directly or transitively through a project dependency. This resulted in many dependency versions being reported many times within the snapshot, and these duplicates were then mirrored in the GitHub Dependency Graph and Dependabot Security Alerts. With this change, only external dependencies are reported, and only within the context of the project that declares those dependencies. This should help reduce the massive redundancy in the generated snapshot and make these files more useful. --- ...MultiProjectDependencyExtractorTest.groovy | 50 +++++-------------- .../PluginDependencyExtractorTest.groovy | 9 ++-- ...ingleProjectDependencyExtractorTest.groovy | 8 +-- .../dependencygraph/internal/BuildLayout.kt | 6 +-- .../internal/DependencyExtractor.kt | 36 ++++++++++--- .../GitHubRepositorySnapshotBuilder.kt | 26 +++++----- .../internal/model/ResolutionRoot.kt | 9 ++++ .../internal/model/ResolvedComponent.kt | 9 +++- .../internal/model/ResolvedConfiguration.kt | 7 +-- 9 files changed, 83 insertions(+), 77 deletions(-) create mode 100644 plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolutionRoot.kt diff --git a/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/MultiProjectDependencyExtractorTest.groovy b/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/MultiProjectDependencyExtractorTest.groovy index 5bab6ca9..c8b78c8c 100644 --- a/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/MultiProjectDependencyExtractorTest.groovy +++ b/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/MultiProjectDependencyExtractorTest.groovy @@ -69,7 +69,7 @@ class MultiProjectDependencyExtractorTest extends BaseExtractorTest { } } - def "extracts transitive project dependencies in multi-project build"() { + def "extracts transitive project dependencies in multi-project build with #description"() { given: settingsFile << "include 'a', 'b', 'c'" buildFile << """ @@ -89,15 +89,16 @@ class MultiProjectDependencyExtractorTest extends BaseExtractorTest { apply plugin: 'java' dependencies { implementation project(':b') + implementation 'org.test:bar:1.0' } } """ when: - run() + run(task) then: - manifestNames == ["project :a", "project :b", "project :c"] + manifestNames == ["project :a", "project :c"] def manifestA = gitHubManifest("project :a") manifestA.sourceFile == "a/build.gradle" @@ -105,37 +106,16 @@ class MultiProjectDependencyExtractorTest extends BaseExtractorTest { "org.test:foo:1.0": [package_url: purlFor(foo)] ]) - def manifestB = gitHubManifest("project :b") - manifestB.sourceFile == "b/build.gradle" - manifestB.assertResolved([ - "project :a" : [ - package_url : "pkg:maven/org.test/a@1.0", - dependencies: ["org.test:foo:1.0"] - ], - "org.test:foo:1.0": [ - package_url : purlFor(foo), - relationship: "indirect" - ] - ]) - def manifestC = gitHubManifest("project :c") manifestC.sourceFile == "c/build.gradle" manifestC.assertResolved([ - "project :b" : [ - package_url : "pkg:maven/org.test/b@1.0", - dependencies: ["project :a"] - ], - "project :a" : [ - package_url : "pkg:maven/org.test/a@1.0", - relationship: "indirect", - dependencies: ["org.test:foo:1.0"] - ], - "org.test:foo:1.0": [ - package_url : purlFor(foo), - relationship: "indirect" - ] - + "org.test:bar:1.0": [package_url: purlFor(bar)] ]) + + where: + task | description + "GitHubDependencyGraphPlugin_generateDependencyGraph" | "All dependencies resolved" + ":c:dependencies" | "One project resolved" } def "extracts dependencies from buildSrc project"() { @@ -200,6 +180,7 @@ class MultiProjectDependencyExtractorTest extends BaseExtractorTest { apply plugin: 'java' dependencies { implementation 'org.test.included:included-child' + implementation 'org.test:bar:1.0' } """ @@ -212,14 +193,7 @@ class MultiProjectDependencyExtractorTest extends BaseExtractorTest { def projectManifest = gitHubManifest("project :") projectManifest.sourceFile == "build.gradle" projectManifest.assertResolved([ - "project :included-child": [ - package_url : "pkg:maven/org.test.included/included-child@1.0", - dependencies: ["org.test:foo:1.0"] - ], - "org.test:foo:1.0" : [ - package_url : purlFor(foo), - relationship: "indirect" - ] + "org.test:bar:1.0": [package_url: purlFor(bar)] ]) def includedManifest = gitHubManifest("project :included-child") diff --git a/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/PluginDependencyExtractorTest.groovy b/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/PluginDependencyExtractorTest.groovy index 289577be..59b80189 100644 --- a/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/PluginDependencyExtractorTest.groovy +++ b/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/PluginDependencyExtractorTest.groovy @@ -54,9 +54,8 @@ class PluginDependencyExtractorTest extends BaseExtractorTest { run() then: - manifestNames == ["build :", "project :", "project :buildSrc", "project :buildSrc:a"] + manifestNames == ["build :", "project :buildSrc", "project :buildSrc:a"] manifestHasSettingsPlugin("build :") - manifestIsEmpty("project :") manifestHasPlugin1("project :buildSrc") manifestHasPlugin2("project :buildSrc:a") } @@ -72,9 +71,8 @@ class PluginDependencyExtractorTest extends BaseExtractorTest { run() then: - manifestNames == ["build :", "project :", "project :included-child", "project :included-child:a"] + manifestNames == ["build :", "project :included-child", "project :included-child:a"] manifestHasSettingsPlugin("build :") - manifestIsEmpty("project :") manifestHasPlugin1("project :included-child") manifestHasPlugin2("project :included-child:a") } @@ -94,9 +92,8 @@ class PluginDependencyExtractorTest extends BaseExtractorTest { run() then: - manifestNames == ["build :", "project :", "project :included-plugin", "project :included-plugin:a"] + manifestNames == ["build :", "project :included-plugin", "project :included-plugin:a"] manifestHasSettingsPlugin("build :") - manifestIsEmpty("project :") manifestHasPlugin1("project :included-plugin") manifestHasPlugin2("project :included-plugin:a") } diff --git a/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/SingleProjectDependencyExtractorTest.groovy b/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/SingleProjectDependencyExtractorTest.groovy index 2d957b1b..6b251c80 100644 --- a/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/SingleProjectDependencyExtractorTest.groovy +++ b/plugin-test/src/test/groovy/org/gradle/github/dependencygraph/SingleProjectDependencyExtractorTest.groovy @@ -329,7 +329,7 @@ class SingleProjectDependencyExtractorTest extends BaseExtractorTest { run() then: - manifestNames == ["build :", "project :"] + manifestNames == ["build :"] def settingsManifest = gitHubManifest("build :") settingsManifest.sourceFile == "build.gradle" @@ -412,7 +412,7 @@ class SingleProjectDependencyExtractorTest extends BaseExtractorTest { run() then: - manifestNames == ["build :", "project :"] + manifestNames == ["build :"] def buildManifest = gitHubManifest("build :") buildManifest.sourceFile == "build.gradle" buildManifest.assertResolved([ @@ -425,9 +425,5 @@ class SingleProjectDependencyExtractorTest extends BaseExtractorTest { relationship: "indirect" ] ]) - - def manifest = gitHubManifest("project :") - manifest.sourceFile == "build.gradle" - manifest.assertResolved([:]) } } diff --git a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/BuildLayout.kt b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/BuildLayout.kt index ad69b4fd..b7f8dea5 100644 --- a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/BuildLayout.kt +++ b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/BuildLayout.kt @@ -1,7 +1,7 @@ package org.gradle.github.dependencygraph.internal import org.gradle.github.dependencygraph.internal.json.GitHubManifestFile -import org.gradle.github.dependencygraph.internal.model.ResolvedConfiguration +import org.gradle.github.dependencygraph.internal.model.ResolutionRoot import java.nio.file.Path import java.nio.file.Paths import java.util.concurrent.ConcurrentHashMap @@ -22,7 +22,7 @@ class BuildLayout(val gitWorkspaceDirectory: Path) { } } - fun getBuildFile(resolvedConfiguration: ResolvedConfiguration): GitHubManifestFile? { - return buildFileForProject(resolvedConfiguration.identityPath) + fun getBuildFile(rootComponent: ResolutionRoot): GitHubManifestFile? { + return buildFileForProject(rootComponent.path) } } \ No newline at end of file diff --git a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/DependencyExtractor.kt b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/DependencyExtractor.kt index 908c63cf..43b99a4f 100644 --- a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/DependencyExtractor.kt +++ b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/DependencyExtractor.kt @@ -7,6 +7,7 @@ import org.gradle.api.internal.artifacts.DefaultProjectComponentIdentifier import org.gradle.api.internal.artifacts.configurations.ResolveConfigurationDependenciesBuildOperationType import org.gradle.github.GitHubDependencyGraphPlugin import org.gradle.github.dependencygraph.internal.model.ComponentCoordinates +import org.gradle.github.dependencygraph.internal.model.ResolutionRoot import org.gradle.github.dependencygraph.internal.model.ResolvedComponent import org.gradle.github.dependencygraph.internal.model.ResolvedConfiguration import org.gradle.internal.exceptions.DefaultMultiCauseException @@ -116,14 +117,21 @@ abstract class DependencyExtractor : // It is possible to do better. By tracking the current build operation context, we can assign more precisely. // See the Gradle Enterprise Build Scan Plugin: `ConfigurationResolutionCapturer_5_0` val rootPath = projectIdentityPath ?: details.buildPath - val rootId = if (projectIdentityPath == null) "build $rootPath" else "project $rootPath" - val resolvedConfiguration = ResolvedConfiguration(rootId, rootPath) + val rootId = if (projectIdentityPath == null) "build $rootPath" else componentId(rootComponent) + val resolutionRoot = ResolutionRoot(rootId, rootPath) + val resolvedConfiguration = ResolvedConfiguration(resolutionRoot) for (directDependency in getResolvedDependencies(rootComponent)) { - val directDep = createComponentNode(componentId(directDependency), true, directDependency, repositoryLookup) + val directDep = createComponentNode( + componentId(directDependency), + resolutionRoot, + true, + directDependency, + repositoryLookup + ) resolvedConfiguration.addDependency(directDep) - walkComponentDependencies(directDependency, repositoryLookup, resolvedConfiguration) + walkComponentDependencies(directDependency, resolutionRoot, repositoryLookup, resolvedConfiguration) } resolvedConfigurations.add(resolvedConfiguration) @@ -131,29 +139,41 @@ abstract class DependencyExtractor : private fun walkComponentDependencies( component: ResolvedComponentResult, + resolveContext: ResolutionRoot, repositoryLookup: RepositoryUrlLookup, resolvedConfiguration: ResolvedConfiguration ) { + val rootComponent = getResolutionRoot(component, resolveContext) + val direct = rootComponent != resolveContext + val dependencyComponents = getResolvedDependencies(component) for (dependencyComponent in dependencyComponents) { val dependencyId = componentId(dependencyComponent) if (!resolvedConfiguration.hasDependency(dependencyId)) { - val dependencyNode = createComponentNode(dependencyId, false, dependencyComponent, repositoryLookup) + val dependencyNode = createComponentNode(dependencyId, rootComponent, direct, dependencyComponent, repositoryLookup) resolvedConfiguration.addDependency(dependencyNode) - walkComponentDependencies(dependencyComponent, repositoryLookup, resolvedConfiguration) + walkComponentDependencies(dependencyComponent, rootComponent, repositoryLookup, resolvedConfiguration) } } } + private fun getResolutionRoot(component: ResolvedComponentResult, resolveContext: ResolutionRoot): ResolutionRoot { + val componentId = component.id + if (componentId is DefaultProjectComponentIdentifier) { + return ResolutionRoot(componentId(component), componentId.identityPath.path) + } + return resolveContext + } + private fun getResolvedDependencies(component: ResolvedComponentResult): List { return component.dependencies.filterIsInstance().map { it.selected }.filter { it != component } } - private fun createComponentNode(componentId: String, direct: Boolean, component: ResolvedComponentResult, repositoryLookup: RepositoryUrlLookup): ResolvedComponent { + private fun createComponentNode(componentId: String, rootComponent: ResolutionRoot, direct: Boolean, component: ResolvedComponentResult, repositoryLookup: RepositoryUrlLookup): ResolvedComponent { val componentDependencies = component.dependencies.filterIsInstance().map { componentId(it.selected) } val repositoryUrl = repositoryLookup.doLookup(component) - return ResolvedComponent(componentId, direct, coordinates(component), repositoryUrl, componentDependencies) + return ResolvedComponent(componentId, rootComponent, direct, coordinates(component), repositoryUrl, componentDependencies) } private fun componentId(component: ResolvedComponentResult): String { diff --git a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt index 598fea35..07df898a 100644 --- a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt +++ b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt @@ -4,6 +4,7 @@ import com.github.packageurl.PackageURLBuilder import org.gradle.github.dependencygraph.internal.model.ResolvedComponent import org.gradle.github.dependencygraph.internal.model.ResolvedConfiguration import org.gradle.github.dependencygraph.internal.json.* +import org.gradle.github.dependencygraph.internal.model.ResolutionRoot private const val DEFAULT_MAVEN_REPOSITORY_URL = "https://repo.maven.apache.org/maven2" @@ -25,25 +26,25 @@ class GitHubRepositorySnapshotBuilder( fun build(resolvedConfigurations: MutableList, buildLayout: BuildLayout): GitHubRepositorySnapshot { val manifestDependencies = mutableMapOf() - val manifestFiles = mutableMapOf() for (resolutionRoot in resolvedConfigurations) { - val manifestName = manifestName(resolutionRoot) - val dependencyCollector = manifestDependencies.getOrPut(manifestName) { DependencyCollector() } - for (component in resolutionRoot.allDependencies) { - dependencyCollector.addResolved(component) - } + for (dependency in resolutionRoot.allDependencies) { + // Ignore project dependencies (transitive deps of projects will be reported with project) + if (isProject(dependency)) continue - // If not assigned to a project, assume the root project for the assigned build. - manifestFiles.putIfAbsent(manifestName, buildLayout.getBuildFile(resolutionRoot)) + val rootComponent = dependency.rootComponent + val dependencyCollector = manifestDependencies.getOrPut(rootComponent.id) { DependencyCollector(rootComponent) } + dependencyCollector.addResolved(dependency) + } } val manifests = manifestDependencies.mapValues { (name, collector) -> + val manifestFile = buildLayout.getBuildFile(collector.rootComponent) GitHubManifest( name, collector.getDependencies(), - manifestFiles[name] + manifestFile ) } return GitHubRepositorySnapshot( @@ -55,11 +56,12 @@ class GitHubRepositorySnapshotBuilder( ) } - private fun manifestName(root: ResolvedConfiguration): String { - return root.id + // TODO:DAZ Model this better + private fun isProject(dependency: ResolvedComponent): Boolean { + return dependency.id.startsWith("project ") } - private class DependencyCollector() { + private class DependencyCollector(var rootComponent: ResolutionRoot) { private val dependencyBuilders: MutableMap = mutableMapOf() /** diff --git a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolutionRoot.kt b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolutionRoot.kt new file mode 100644 index 00000000..2f4feba4 --- /dev/null +++ b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolutionRoot.kt @@ -0,0 +1,9 @@ +package org.gradle.github.dependencygraph.internal.model + +/** + * A root component in dependency resolution, this represents a component that "owns" or declares + * dependencies within a given resolution. + * In most cases, this will be the project component that declares the dependency, but for certain resolutions + * this component will represent an overall build. + */ +data class ResolutionRoot(val id: String, val path: String) diff --git a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolvedComponent.kt b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolvedComponent.kt index 0d96b1b4..8acd6db0 100644 --- a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolvedComponent.kt +++ b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolvedComponent.kt @@ -1,3 +1,10 @@ package org.gradle.github.dependencygraph.internal.model -data class ResolvedComponent(val id: String, val direct: Boolean, val coordinates: ComponentCoordinates, val repositoryUrl: String?, val dependencies: List) +data class ResolvedComponent( + val id: String, + val rootComponent: ResolutionRoot, + val direct: Boolean, + val coordinates: ComponentCoordinates, + val repositoryUrl: String?, + val dependencies: List +) diff --git a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolvedConfiguration.kt b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolvedConfiguration.kt index 62ebec6f..92236de1 100644 --- a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolvedConfiguration.kt +++ b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/model/ResolvedConfiguration.kt @@ -1,8 +1,9 @@ package org.gradle.github.dependencygraph.internal.model -data class ResolvedConfiguration(val id: String, - val identityPath: String, - val allDependencies: MutableList = mutableListOf()) { +data class ResolvedConfiguration( + val rootComponent: ResolutionRoot, + val allDependencies: MutableList = mutableListOf() +) { fun addDependency(component: ResolvedComponent) { allDependencies.add(component) } From 73dfee450c45eab5ef5891f9bd2167b53a9c5d37 Mon Sep 17 00:00:00 2001 From: daz Date: Mon, 10 Jul 2023 05:28:28 -0600 Subject: [PATCH 2/2] var -> val --- .../dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt index 07df898a..d3770274 100644 --- a/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt +++ b/plugin/src/main/kotlin/org/gradle/github/dependencygraph/internal/GitHubRepositorySnapshotBuilder.kt @@ -61,7 +61,7 @@ class GitHubRepositorySnapshotBuilder( return dependency.id.startsWith("project ") } - private class DependencyCollector(var rootComponent: ResolutionRoot) { + private class DependencyCollector(val rootComponent: ResolutionRoot) { private val dependencyBuilders: MutableMap = mutableMapOf() /**