From 93e59f9cd75008533d7f1fb95532bbff8044bbab Mon Sep 17 00:00:00 2001 From: Oliver Heger Date: Thu, 15 Apr 2021 15:15:50 +0200 Subject: [PATCH] Maven: Switch to the optimized dependency graph format Rework the implementation to use DependencyGraphBuilder together with MavenDependencyHandler to produce the analyzer result. Adapt test classes as necessary. Rather than changing the expected test results, convert results to the classic format before they are compared with the expectations. This check is safer, as it makes sure that the content of the results has not changed, although they are now written in a different format. Add a helper function for this conversion to the test-utils module. Signed-off-by: Oliver Heger --- analyzer/src/funTest/kotlin/GitRepoFunTest.kt | 2 +- .../funTest/kotlin/managers/MavenFunTest.kt | 21 ++-- .../src/funTest/kotlin/managers/SbtFunTest.kt | 6 +- analyzer/src/main/kotlin/managers/Maven.kt | 111 +++++------------- 4 files changed, 47 insertions(+), 93 deletions(-) diff --git a/analyzer/src/funTest/kotlin/GitRepoFunTest.kt b/analyzer/src/funTest/kotlin/GitRepoFunTest.kt index 80621ca869482..9de75231214de 100644 --- a/analyzer/src/funTest/kotlin/GitRepoFunTest.kt +++ b/analyzer/src/funTest/kotlin/GitRepoFunTest.kt @@ -66,7 +66,7 @@ class GitRepoFunTest : StringSpec() { // Disabled on Azure Windows because it fails for unknown reasons. "Analyzer correctly reports VcsInfo for git-repo projects".config(enabled = !Ci.isAzureWindows) { val ortResult = Analyzer(DEFAULT_ANALYZER_CONFIGURATION).analyze(outputDir) - val actualResult = ortResult.toYaml() + val actualResult = ortResult.withResolvedScopes().toYaml() val expectedResult = patchExpectedResult( File("src/funTest/assets/projects/external/git-repo-expected-output.yml"), revision = REPO_REV, diff --git a/analyzer/src/funTest/kotlin/managers/MavenFunTest.kt b/analyzer/src/funTest/kotlin/managers/MavenFunTest.kt index b220c294e9a50..a8e1371a13ae1 100644 --- a/analyzer/src/funTest/kotlin/managers/MavenFunTest.kt +++ b/analyzer/src/funTest/kotlin/managers/MavenFunTest.kt @@ -49,7 +49,7 @@ class MavenFunTest : StringSpec() { val pomFile = projectDir.resolve("pom.xml") val expectedResult = projectDir.parentFile.resolve("jgnash-expected-output.yml").readText() - val result = createMaven().resolveSingleProject(pomFile) + val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true) result.toYaml() shouldBe expectedResult } @@ -65,12 +65,12 @@ class MavenFunTest : StringSpec() { // jgnash-core depends on jgnash-resources, so we also have to pass the pom.xml of jgnash-resources to // resolveDependencies so that it is available in the Maven.projectsByIdentifier cache. Otherwise resolution // of transitive dependencies would not work. - val result = createMaven().resolveDependencies(listOf(pomFileCore, pomFileResources)) - .projectResults[pomFileCore] + val managerResult = createMaven().resolveDependencies(listOf(pomFileCore, pomFileResources)) + val result = managerResult.projectResults[pomFileCore] result.shouldNotBeNull() result should haveSize(1) - result.single().toYaml() shouldBe expectedResult + managerResult.resolveScopes(result.single()).toYaml() shouldBe expectedResult } "Root project dependencies are detected correctly" { @@ -81,7 +81,7 @@ class MavenFunTest : StringSpec() { revision = vcsRevision ) - val result = createMaven().resolveSingleProject(pomFile) + val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true) result.toYaml() shouldBe expectedResult } @@ -98,11 +98,12 @@ class MavenFunTest : StringSpec() { // app depends on lib, so we also have to pass the pom.xml of lib to resolveDependencies so that it is // available in the Maven.projectsByIdentifier cache. Otherwise resolution of transitive dependencies would // not work. - val result = createMaven().resolveDependencies(listOf(pomFileApp, pomFileLib)).projectResults[pomFileApp] + val managerResult = createMaven().resolveDependencies(listOf(pomFileApp, pomFileLib)) + val result = managerResult.projectResults[pomFileApp] result.shouldNotBeNull() result should haveSize(1) - result.single().toYaml() shouldBe expectedResult + managerResult.resolveScopes(result.single()).toYaml() shouldBe expectedResult } "External dependencies are detected correctly" { @@ -113,7 +114,7 @@ class MavenFunTest : StringSpec() { revision = vcsRevision ) - val result = createMaven().resolveSingleProject(pomFile) + val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true) result.toYaml() shouldBe expectedResult } @@ -132,7 +133,7 @@ class MavenFunTest : StringSpec() { revision = vcsRevision ) - val result = createMaven().resolveSingleProject(pomFile) + val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true) result.toYaml() shouldBe expectedResult } @@ -146,7 +147,7 @@ class MavenFunTest : StringSpec() { revision = vcsRevision ) - val result = createMaven().resolveSingleProject(pomFile) + val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true) patchActualResult(result.toYaml(), patchStartAndEndTime = true) shouldBe expectedResult } diff --git a/analyzer/src/funTest/kotlin/managers/SbtFunTest.kt b/analyzer/src/funTest/kotlin/managers/SbtFunTest.kt index dedbcd9b4d12a..4acda93822b07 100644 --- a/analyzer/src/funTest/kotlin/managers/SbtFunTest.kt +++ b/analyzer/src/funTest/kotlin/managers/SbtFunTest.kt @@ -46,7 +46,7 @@ class SbtFunTest : StringSpec({ val ortResult = Analyzer(DEFAULT_ANALYZER_CONFIGURATION).analyze(projectDir, listOf(Sbt.Factory())) - val actualResult = ortResult.toYaml() + val actualResult = ortResult.withResolvedScopes().toYaml() val expectedResult = patchExpectedResult(expectedOutputFile) patchActualResult(actualResult, patchStartAndEndTime = true) shouldBe expectedResult @@ -64,7 +64,7 @@ class SbtFunTest : StringSpec({ val ortResult = Analyzer(DEFAULT_ANALYZER_CONFIGURATION).analyze(projectDir, listOf(Sbt.Factory())) - val actualResult = ortResult.toYaml() + val actualResult = ortResult.withResolvedScopes().toYaml() val expectedResult = patchExpectedResult(expectedOutputFile) patchActualResult(actualResult, patchStartAndEndTime = true) shouldBe expectedResult @@ -85,7 +85,7 @@ class SbtFunTest : StringSpec({ val ortResult = Analyzer(DEFAULT_ANALYZER_CONFIGURATION).analyze(projectDir, listOf(Sbt.Factory())) - val actualResult = ortResult.toYaml() + val actualResult = ortResult.withResolvedScopes().toYaml() val expectedResult = patchExpectedResult( expectedOutputFile, url = vcsUrl, diff --git a/analyzer/src/main/kotlin/managers/Maven.kt b/analyzer/src/main/kotlin/managers/Maven.kt index bf50f7df74ca8..c49354345e31a 100644 --- a/analyzer/src/main/kotlin/managers/Maven.kt +++ b/analyzer/src/main/kotlin/managers/Maven.kt @@ -21,7 +21,6 @@ package org.ossreviewtoolkit.analyzer.managers import java.io.File -import org.apache.maven.project.ProjectBuildingException import org.apache.maven.project.ProjectBuildingResult import org.eclipse.aether.artifact.Artifact @@ -31,24 +30,20 @@ import org.eclipse.aether.repository.WorkspaceRepository import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory import org.ossreviewtoolkit.analyzer.PackageManager +import org.ossreviewtoolkit.analyzer.PackageManagerResult +import org.ossreviewtoolkit.analyzer.managers.utils.DependencyGraphBuilder import org.ossreviewtoolkit.analyzer.managers.utils.MavenSupport import org.ossreviewtoolkit.analyzer.managers.utils.identifier import org.ossreviewtoolkit.downloader.VersionControlSystem +import org.ossreviewtoolkit.model.DependencyGraph import org.ossreviewtoolkit.model.Identifier -import org.ossreviewtoolkit.model.Package -import org.ossreviewtoolkit.model.PackageLinkage -import org.ossreviewtoolkit.model.PackageReference import org.ossreviewtoolkit.model.Project import org.ossreviewtoolkit.model.ProjectAnalyzerResult -import org.ossreviewtoolkit.model.Scope import org.ossreviewtoolkit.model.Severity import org.ossreviewtoolkit.model.config.AnalyzerConfiguration import org.ossreviewtoolkit.model.config.RepositoryConfiguration import org.ossreviewtoolkit.model.createAndLogIssue -import org.ossreviewtoolkit.utils.collectMessagesAsString -import org.ossreviewtoolkit.utils.log import org.ossreviewtoolkit.utils.searchUpwardsForSubdirectory -import org.ossreviewtoolkit.utils.showStackTrace /** * The [Maven](https://maven.apache.org/) package manager for Java. @@ -88,6 +83,9 @@ class Maven( private val localProjectBuildingResults = mutableMapOf() + /** The builder for the shared dependency graph. */ + private lateinit var graphBuilder: DependencyGraphBuilder + private var sbtMode = false /** @@ -97,22 +95,29 @@ class Maven( override fun beforeResolution(definitionFiles: List) { localProjectBuildingResults += mvn.prepareMavenProjects(definitionFiles) + + val localProjects = localProjectBuildingResults.mapValues { it.value.project } + val dependencyHandler = MavenDependencyHandler(managerName, mvn, localProjects, sbtMode) + graphBuilder = DependencyGraphBuilder(dependencyHandler) } + override fun createPackageManagerResult(projectResults: Map>): + PackageManagerResult = + PackageManagerResult(projectResults, graphBuilder.build(), graphBuilder.packages().toSortedSet()) + override fun resolveDependencies(definitionFile: File): List { val workingDir = definitionFile.parentFile val projectBuildingResult = mvn.buildMavenProject(definitionFile) val mavenProject = projectBuildingResult.project - val packagesById = mutableMapOf() - val scopesByName = mutableMapOf() - - projectBuildingResult.dependencyResolutionResult.dependencyGraph.children.forEach { node -> - val scopeName = node.dependency.scope - val scope = scopesByName.getOrPut(scopeName) { - Scope(scopeName, sortedSetOf()) - } + val projectId = Identifier( + type = managerName, + namespace = mavenProject.groupId, + name = mavenProject.artifactId, + version = mavenProject.version + ) - scope.dependencies += parseDependency(node, packagesById) + projectBuildingResult.dependencies.forEach { node -> + graphBuilder.addDependency(DependencyGraph.qualifyScope(projectId, node.dependency.scope), node) } val declaredLicenses = MavenSupport.parseLicenses(mavenProject) @@ -133,12 +138,7 @@ class Maven( val vcsFallbackUrls = listOfNotNull(browsableScmUrl, homepageUrl).toTypedArray() val project = Project( - id = Identifier( - type = managerName, - namespace = mavenProject.groupId, - name = mavenProject.artifactId, - version = mavenProject.version - ), + id = projectId, definitionFilePath = VersionControlSystem.getPathInfo(definitionFile).path, authors = MavenSupport.parseAuthors(mavenProject), declaredLicenses = declaredLicenses, @@ -146,10 +146,10 @@ class Maven( vcs = vcsFromPackage, vcsProcessed = processProjectVcs(projectDir, vcsFromPackage, *vcsFallbackUrls), homepageUrl = homepageUrl.orEmpty(), - scopeDependencies = scopesByName.values.toSortedSet() + scopeNames = projectBuildingResult.dependencies.mapTo(sortedSetOf()) { it.dependency.scope } ) - val packages = packagesById.values.toSortedSet() + val packages = graphBuilder.packages().toSortedSet() val issues = packages.mapNotNull { pkg -> if (pkg.description == "POM was created by Sonatype Nexus") { createAndLogIssue( @@ -162,59 +162,12 @@ class Maven( } } - return listOf(ProjectAnalyzerResult(project, packages, issues)) - } - - private fun parseDependency(node: DependencyNode, packages: MutableMap): PackageReference { - val identifier = node.artifact.identifier() - - try { - val dependencies = node.children.mapNotNull { child -> - val toolsJarCoordinates = listOf("com.sun:tools:", "jdk.tools:jdk.tools:") - if (toolsJarCoordinates.any { child.artifact.identifier().startsWith(it) }) { - log.info { "Omitting the Java < 1.9 system dependency on 'tools.jar'." } - null - } else { - parseDependency(child, packages) - } - }.toSortedSet() - - val localProjects = localProjectBuildingResults.mapValues { it.value.project } - - return if (localProjects.contains(identifier)) { - val id = Identifier( - type = managerName, - namespace = node.artifact.groupId, - name = node.artifact.artifactId, - version = node.artifact.version - ) - - log.info { "Dependency '${id.toCoordinates()}' refers to a local project." } - - PackageReference(id, PackageLinkage.PROJECT_DYNAMIC, dependencies) - } else { - val pkg = packages.getOrPut(identifier) { - // TODO: Omit the "localProjects" argument here once SBT is implemented independently of Maven as at - // this point we know already that "identifier" is not a local project. - mvn.parsePackage(node.artifact, node.repositories, localProjects, sbtMode) - } - - pkg.toReference(dependencies = dependencies) - } - } catch (e: ProjectBuildingException) { - e.showStackTrace() - - return PackageReference( - Identifier(managerName, node.artifact.groupId, node.artifact.artifactId, node.artifact.version), - dependencies = sortedSetOf(), - issues = listOf( - createAndLogIssue( - source = managerName, - message = "Could not get package information for dependency '$identifier': " + - e.collectMessagesAsString() - ) - ) - ) - } + return listOf(ProjectAnalyzerResult(project, sortedSetOf(), issues)) } } + +/** + * Convenience extension property to obtain all the [DependencyNode]s from this [ProjectBuildingResult]. + */ +private val ProjectBuildingResult.dependencies: List + get() = dependencyResolutionResult.dependencyGraph.children