From 5997772a6dd500c42002e83d9b1c52bdac23ab46 Mon Sep 17 00:00:00 2001 From: Onur Demirci Date: Tue, 28 Nov 2023 09:46:31 +0100 Subject: [PATCH 01/10] feat(CompareCommand): Add a comparison for the `Repository`s This commit adds new data classes to track the differences of the two ORT results and especially of two repositories. Signed-off-by: Onur Demirci --- .../compare/src/main/kotlin/CompareCommand.kt | 275 ++++++++++++++++-- 1 file changed, 246 insertions(+), 29 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index c39311424aa46..efe68d85c814f 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -34,7 +34,6 @@ import com.github.ajalt.clikt.parameters.options.flag import com.github.ajalt.clikt.parameters.options.option import com.github.ajalt.clikt.parameters.types.enum import com.github.ajalt.clikt.parameters.types.file -import com.github.ajalt.clikt.parameters.types.int import com.github.ajalt.mordant.rendering.Theme import com.github.difflib.DiffUtils import com.github.difflib.UnifiedDiffUtils @@ -42,18 +41,35 @@ import com.github.difflib.UnifiedDiffUtils import java.time.Instant import org.ossreviewtoolkit.model.OrtResult +import org.ossreviewtoolkit.model.PackageCuration +import org.ossreviewtoolkit.model.Repository +import org.ossreviewtoolkit.model.VcsInfo +import org.ossreviewtoolkit.model.config.Curations +import org.ossreviewtoolkit.model.config.Excludes +import org.ossreviewtoolkit.model.config.IssueResolution +import org.ossreviewtoolkit.model.config.LicenseChoices +import org.ossreviewtoolkit.model.config.LicenseFindingCuration +import org.ossreviewtoolkit.model.config.PackageConfiguration +import org.ossreviewtoolkit.model.config.PackageLicenseChoice +import org.ossreviewtoolkit.model.config.PackageManagerConfiguration +import org.ossreviewtoolkit.model.config.PathExclude +import org.ossreviewtoolkit.model.config.RepositoryAnalyzerConfiguration +import org.ossreviewtoolkit.model.config.RepositoryConfiguration +import org.ossreviewtoolkit.model.config.Resolutions +import org.ossreviewtoolkit.model.config.RuleViolationResolution +import org.ossreviewtoolkit.model.config.ScopeExclude +import org.ossreviewtoolkit.model.config.VulnerabilityResolution import org.ossreviewtoolkit.model.mapper import org.ossreviewtoolkit.plugins.commands.api.OrtCommand import org.ossreviewtoolkit.utils.common.expandTilde import org.ossreviewtoolkit.utils.common.getCommonParentFile import org.ossreviewtoolkit.utils.ort.Environment +import org.ossreviewtoolkit.utils.spdx.model.SpdxLicenseChoice class CompareCommand : OrtCommand( name = "compare", help = "Compare two ORT results with various methods." ) { - private enum class CompareMethod { SEMANTIC_DIFF, TEXT_DIFF } - private val fileA by argument(help = "The first ORT result file to compare.") .convert { it.expandTilde() } .file(mustExist = true, canBeFile = true, canBeDir = false, mustBeWritable = false, mustBeReadable = true) @@ -71,13 +87,6 @@ class CompareCommand : OrtCommand( ).enum() .default(CompareMethod.TEXT_DIFF) - private val contextSize by option( - "--context-size", "-C", - help = "The number of unmodified lines to display in the context of a modified line. Only applies to unified " + - "diff output." - ).int() - .default(7) - private val ignoreTime by option( "--ignore-time", "-t", help = "Ignore time differences." @@ -101,20 +110,19 @@ class CompareCommand : OrtCommand( if (fileA.extension != fileB.extension) { echo("The file arguments need to be of the same type.") - throw ProgramResult(1) + throw ProgramResult(2) } - // Arbitrarily determine the mapper from the first file as the file extensions are ensured to be the same. - val mapper = fileA.mapper().registerModule( + val deserializer = fileA.mapper().registerModule( SimpleModule().apply { - // TODO: Find a way to also ignore temporary directories (when diffing semantically). + // TODO: Find a way to also ignore temporary directories. if (ignoreTime) addDeserializer(Instant::class.java, EpochInstantDeserializer()) if (ignoreEnvironment) addDeserializer(Environment::class.java, DefaultEnvironmentDeserializer()) } ) - val resultA = mapper.readValue(fileA) - val resultB = mapper.readValue(fileB) + val resultA = deserializer.readValue(fileA) + val resultB = deserializer.readValue(fileB) when (method) { CompareMethod.SEMANTIC_DIFF -> { @@ -130,12 +138,16 @@ class CompareCommand : OrtCommand( throw ProgramResult(0) } - throw ProgramResult(2) + val diff = resultA.diff(resultB) + + echo(deserializer.writeValueAsString(diff)) + + throw ProgramResult(1) } CompareMethod.TEXT_DIFF -> { - val textA = mapper.writeValueAsString(resultA) - val textB = mapper.writeValueAsString(resultB) + val textA = deserializer.writeValueAsString(resultA) + val textB = deserializer.writeValueAsString(resultB) // Apply data type independent replacements in the texts. val replacements = buildMap { @@ -156,7 +168,7 @@ class CompareCommand : OrtCommand( "b/${fileB.relativeTo(commonParent).invariantSeparatorsPath}", linesA, DiffUtils.diff(linesA, linesB), - contextSize + /* contextSize = */ 7 ) if (diff.isEmpty()) { @@ -170,29 +182,234 @@ class CompareCommand : OrtCommand( echo(it) } - throw ProgramResult(2) + throw ProgramResult(1) } } } } +private fun OrtResult.diff(other: OrtResult) = + OrtResultDiff( + repositoryDiff = repository.diff(other.repository) + ) + +private fun Repository.diff(other: Repository): RepositoryDiff? = + if (this == other) { + null + } else { + RepositoryDiff( + vcsA = vcs.takeIf { it != other.vcs }, + vcsB = other.vcs.takeIf { it != vcs }, + vcsProcessedA = vcsProcessed.takeIf { it != other.vcsProcessed }, + vcsProcessedB = other.vcsProcessed.takeIf { it != vcsProcessed }, + nestedRepositoriesA = nestedRepositories.takeIf { it != other.nestedRepositories }, + nestedRepositoriesB = other.nestedRepositories.takeIf { it != nestedRepositories }, + configDiff = config.diff(other.config) + ) + } + +private fun RepositoryConfiguration.diff(other: RepositoryConfiguration): RepositoryConfigurationDiff? = + if (this == other) { + null + } else { + RepositoryConfigurationDiff( + analyzerConfigDiff = analyzer.diff(other.analyzer), + excludeDiff = excludes.diff(other.excludes), + resolutionsDiff = resolutions.diff(other.resolutions), + curationsDiff = curations.diff(other.curations), + packageConfigurationsA = (packageConfigurations - other.packageConfigurations.toSet()) + .takeUnless { it.isEmpty() }, + packageConfigurationsB = (other.packageConfigurations - packageConfigurations.toSet()) + .takeUnless { it.isEmpty() }, + licenseChoicesDiff = licenseChoices.diff(other.licenseChoices) + ) + } + +private fun Excludes.diff(other: Excludes): ExcludesDiff? = + if (this == other) { + null + } else { + ExcludesDiff( + pathsA = (paths - other.paths.toSet()).takeUnless { it.isEmpty() }, + pathsB = (other.paths - paths.toSet()).takeUnless { it.isEmpty() }, + scopesA = (scopes - other.scopes.toSet()).takeUnless { it.isEmpty() }, + scopesB = (other.scopes - scopes.toSet()).takeUnless { it.isEmpty() } + ) + } + +private fun Resolutions?.diff(other: Resolutions?): ResolutionsDiff? { + if (this == other) return null + + return if (this == null) { + ResolutionsDiff( + issuesB = other?.issues, + ruleViolationsB = other?.ruleViolations, + vulnerabilitiesB = other?.vulnerabilities + ) + } else if (other == null) { + ResolutionsDiff( + issuesA = issues, + ruleViolationsA = ruleViolations, + vulnerabilitiesA = vulnerabilities + ) + } else { + ResolutionsDiff( + issuesA = (issues - other.issues.toSet()).takeUnless { it.isEmpty() }, + issuesB = (other.issues - issues.toSet()).takeUnless { it.isEmpty() }, + ruleViolationsA = (ruleViolations - other.ruleViolations.toSet()).takeUnless { it.isEmpty() }, + ruleViolationsB = (other.ruleViolations - ruleViolations.toSet()).takeUnless { it.isEmpty() }, + vulnerabilitiesA = (vulnerabilities - other.vulnerabilities.toSet()).takeUnless { it.isEmpty() }, + vulnerabilitiesB = (other.vulnerabilities - vulnerabilities.toSet()).takeUnless { it.isEmpty() } + ) + } +} + +private fun Curations.diff(other: Curations): CurationsDiff? = + if (this == other) { + null + } else { + CurationsDiff( + packagesA = (packages - other.packages.toSet()).takeUnless { it.isEmpty() }, + packagesB = (other.packages - packages.toSet()).takeUnless { it.isEmpty() }, + licenseFindingsA = (licenseFindings - other.licenseFindings.toSet()).takeUnless { it.isEmpty() }, + licenseFindingsB = (other.licenseFindings - licenseFindings.toSet()).takeUnless { it.isEmpty() } + ) + } + +private fun LicenseChoices.diff(other: LicenseChoices): LicenseChoicesDiff? = + if (this == other) { + null + } else { + LicenseChoicesDiff( + repositoryLicenseChoicesA = (repositoryLicenseChoices - other.repositoryLicenseChoices.toSet()) + .takeUnless { it.isEmpty() }, + repositoryLicenseChoicesB = (other.repositoryLicenseChoices - repositoryLicenseChoices.toSet()) + .takeUnless { it.isEmpty() }, + packageLicenseChoicesA = (packageLicenseChoices - other.packageLicenseChoices.toSet()) + .takeUnless { it.isEmpty() }, + packageLicenseChoicesB = (other.packageLicenseChoices - packageLicenseChoices.toSet()) + .takeUnless { it.isEmpty() } + ) + } + +private fun RepositoryAnalyzerConfiguration?.diff(other: RepositoryAnalyzerConfiguration?): AnalyzerConfigurationDiff? { + if (this == other) return null + + return if (this == null) { + AnalyzerConfigurationDiff( + allowDynamicVersionsB = other?.allowDynamicVersions, + enabledPackageManagersB = other?.enabledPackageManagers, + disabledPackageManagersB = other?.disabledPackageManagers, + packageManagersB = other?.packageManagers, + skipExcludedB = other?.skipExcluded + ) + } else if (other == null) { + AnalyzerConfigurationDiff( + allowDynamicVersionsA = allowDynamicVersions, + enabledPackageManagersA = enabledPackageManagers, + disabledPackageManagersA = disabledPackageManagers, + packageManagersA = packageManagers, + skipExcludedA = skipExcluded + ) + } else { + AnalyzerConfigurationDiff( + allowDynamicVersionsA = allowDynamicVersions.takeIf { it != other.allowDynamicVersions }, + allowDynamicVersionsB = other.allowDynamicVersions.takeIf { it != allowDynamicVersions }, + enabledPackageManagersA = enabledPackageManagers.takeIf { it != other.enabledPackageManagers }, + enabledPackageManagersB = other.enabledPackageManagers.takeIf { it != enabledPackageManagers }, + disabledPackageManagersA = disabledPackageManagers.takeIf { it != other.disabledPackageManagers }, + disabledPackageManagersB = other.disabledPackageManagers.takeIf { it != disabledPackageManagers }, + packageManagersA = packageManagers.takeIf { it != other.packageManagers }, + packageManagersB = other.packageManagers.takeIf { it != packageManagers }, + skipExcludedA = skipExcluded.takeIf { it != other.skipExcluded }, + skipExcludedB = other.skipExcluded.takeIf { it != skipExcluded } + ) + } +} + +private enum class CompareMethod { + SEMANTIC_DIFF, + TEXT_DIFF +} + private class EpochInstantDeserializer : StdDeserializer(Instant::class.java) { override fun deserialize(parser: JsonParser, context: DeserializationContext): Instant = - Instant.EPOCH.also { - // Just consume the JSON text node without actually using it. - parser.codec.readTree(parser) - } + Instant.EPOCH.also { parser.codec.readTree(parser) } } private class DefaultEnvironmentDeserializer : StdDeserializer(Environment::class.java) { override fun deserialize(parser: JsonParser, context: DeserializationContext): Environment = - Environment().also { - // Just consume the JSON object node without actually using it. - parser.codec.readTree(parser) - } + Environment().also { parser.codec.readTree(parser) } } private fun Map.replaceIn(text: String) = entries.fold(text) { currentText, (from, to) -> currentText.replace(from, to) } + +private data class OrtResultDiff( + val repositoryDiff: RepositoryDiff? = null +) + +private data class RepositoryDiff( + val vcsA: VcsInfo? = null, + val vcsB: VcsInfo? = null, + val vcsProcessedA: VcsInfo? = null, + val vcsProcessedB: VcsInfo? = null, + val nestedRepositoriesA: Map? = null, + val nestedRepositoriesB: Map? = null, + val configDiff: RepositoryConfigurationDiff? = null +) + +private data class RepositoryConfigurationDiff( + val analyzerConfigDiff: AnalyzerConfigurationDiff? = null, + val excludeDiff: ExcludesDiff? = null, + val resolutionsDiff: ResolutionsDiff? = null, + val curationsDiff: CurationsDiff? = null, + val packageConfigurationsA: List? = null, + val packageConfigurationsB: List? = null, + val licenseChoicesDiff: LicenseChoicesDiff? = null +) + +private data class ExcludesDiff( + val pathsA: List? = null, + val pathsB: List? = null, + val scopesA: List? = null, + val scopesB: List? = null +) + +private data class ResolutionsDiff( + val issuesA: List? = null, + val issuesB: List? = null, + val ruleViolationsA: List? = null, + val ruleViolationsB: List? = null, + val vulnerabilitiesA: List? = null, + val vulnerabilitiesB: List? = null +) + +private data class CurationsDiff( + val packagesA: List? = null, + val packagesB: List? = null, + val licenseFindingsA: List? = null, + val licenseFindingsB: List? = null +) + +private data class LicenseChoicesDiff( + val repositoryLicenseChoicesA: List? = null, + val repositoryLicenseChoicesB: List? = null, + val packageLicenseChoicesA: List? = null, + val packageLicenseChoicesB: List? = null +) + +private data class AnalyzerConfigurationDiff( + val allowDynamicVersionsA: Boolean? = null, + val allowDynamicVersionsB: Boolean? = null, + val enabledPackageManagersA: List? = null, + val enabledPackageManagersB: List? = null, + val disabledPackageManagersA: List? = null, + val disabledPackageManagersB: List? = null, + val packageManagersA: Map? = null, + val packageManagersB: Map? = null, + val skipExcludedA: Boolean? = false, + val skipExcludedB: Boolean? = false +) From 7896c4fb97f53cafe91de1423c90105ed91eb76f Mon Sep 17 00:00:00 2001 From: Onur Demirci Date: Tue, 28 Nov 2023 09:46:31 +0100 Subject: [PATCH 02/10] feat(CompareCommand): Add a comparison for the `AnalyzerRun`s This commit includes the data classes to track the differences of the `AnalyzerRun`s and adds it to the `OrtResultDiff` class. Signed-off-by: Onur Demirci --- .../compare/src/main/kotlin/CompareCommand.kt | 136 +++++++++++++++++- 1 file changed, 134 insertions(+), 2 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index efe68d85c814f..d664ec5521316 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -17,6 +17,8 @@ * License-Filename: LICENSE */ +@file:Suppress("TooManyFunctions") + package org.ossreviewtoolkit.plugins.commands.compare import com.fasterxml.jackson.core.JsonParser @@ -40,10 +42,18 @@ import com.github.difflib.UnifiedDiffUtils import java.time.Instant +import org.ossreviewtoolkit.model.AnalyzerResult +import org.ossreviewtoolkit.model.AnalyzerRun +import org.ossreviewtoolkit.model.DependencyGraph +import org.ossreviewtoolkit.model.Identifier +import org.ossreviewtoolkit.model.Issue import org.ossreviewtoolkit.model.OrtResult +import org.ossreviewtoolkit.model.Package import org.ossreviewtoolkit.model.PackageCuration +import org.ossreviewtoolkit.model.Project import org.ossreviewtoolkit.model.Repository import org.ossreviewtoolkit.model.VcsInfo +import org.ossreviewtoolkit.model.config.AnalyzerConfiguration import org.ossreviewtoolkit.model.config.Curations import org.ossreviewtoolkit.model.config.Excludes import org.ossreviewtoolkit.model.config.IssueResolution @@ -190,7 +200,8 @@ class CompareCommand : OrtCommand( private fun OrtResult.diff(other: OrtResult) = OrtResultDiff( - repositoryDiff = repository.diff(other.repository) + repositoryDiff = repository.diff(other.repository), + analyzerRunDiff = analyzer.diff(other.analyzer) ) private fun Repository.diff(other: Repository): RepositoryDiff? = @@ -327,6 +338,105 @@ private fun RepositoryAnalyzerConfiguration?.diff(other: RepositoryAnalyzerConfi } } +private fun AnalyzerRun?.diff(other: AnalyzerRun?): AnalyzerRunDiff? { + if (this == other) return null + + return if (this == null) { + AnalyzerRunDiff( + startTimeB = other?.startTime, + endTimeB = other?.endTime, + environmentB = other?.environment, + configDiff = diff(other?.config), + resultDiff = diff(other?.result) + ) + } else if (other == null) { + AnalyzerRunDiff( + startTimeA = startTime, + endTimeA = endTime, + environmentA = environment, + configDiff = config.diff(null), + resultDiff = result.diff(null) + ) + } else { + AnalyzerRunDiff( + startTimeA = startTime.takeIf { it != other.startTime }, + startTimeB = other.startTime.takeIf { it != startTime }, + endTimeA = endTime.takeIf { it != other.endTime }, + endTimeB = other.endTime.takeIf { it != endTime }, + environmentA = environment.takeIf { it != other.environment }, + environmentB = other.environment.takeIf { it != environment }, + configDiff = config.diff(other.config), + resultDiff = result.diff(other.result) + ) + } +} + +private fun AnalyzerConfiguration?.diff(other: AnalyzerConfiguration?): AnalyzerConfigurationDiff? { + if (this == other) return null + + return if (this == null) { + AnalyzerConfigurationDiff( + allowDynamicVersionsB = other?.allowDynamicVersions, + enabledPackageManagersB = other?.enabledPackageManagers, + disabledPackageManagersB = other?.disabledPackageManagers, + packageManagersB = other?.packageManagers, + skipExcludedB = other?.skipExcluded + ) + } else if (other == null) { + AnalyzerConfigurationDiff( + allowDynamicVersionsA = allowDynamicVersions, + enabledPackageManagersA = enabledPackageManagers, + disabledPackageManagersA = disabledPackageManagers, + packageManagersA = packageManagers, + skipExcludedA = skipExcluded + ) + } else { + AnalyzerConfigurationDiff( + allowDynamicVersionsA = allowDynamicVersions.takeIf { it != other.allowDynamicVersions }, + allowDynamicVersionsB = other.allowDynamicVersions.takeIf { it != allowDynamicVersions }, + enabledPackageManagersA = enabledPackageManagers.takeIf { it != other.enabledPackageManagers }, + enabledPackageManagersB = other.enabledPackageManagers.takeIf { it != enabledPackageManagers }, + disabledPackageManagersA = disabledPackageManagers.takeIf { it != other.disabledPackageManagers }, + disabledPackageManagersB = other.disabledPackageManagers.takeIf { it != disabledPackageManagers }, + packageManagersA = packageManagers.takeIf { it != other.packageManagers }, + packageManagersB = other.packageManagers.takeIf { it != packageManagers }, + skipExcludedA = skipExcluded.takeIf { it != other.skipExcluded }, + skipExcludedB = other.skipExcluded.takeIf { it != skipExcluded } + ) + } +} + +private fun AnalyzerResult?.diff(other: AnalyzerResult?): AnalyzerResultDiff? { + if (this == other) return null + + return if (this == null) { + AnalyzerResultDiff( + projectsB = other?.projects, + packagesB = other?.packages, + issuesB = other?.issues, + dependencyGraphB = other?.dependencyGraphs + ) + } else if (other == null) { + AnalyzerResultDiff( + projectsA = projects, + packagesA = packages, + issuesA = issues, + dependencyGraphA = dependencyGraphs + ) + } else { + AnalyzerResultDiff( + projectsA = (projects - other.projects).takeUnless { it.isEmpty() }, + projectsB = (other.projects - projects).takeUnless { it.isEmpty() }, + packagesA = (packages - other.packages).takeUnless { it.isEmpty() }, + packagesB = (other.packages - packages).takeUnless { it.isEmpty() }, + issuesA = issues.takeIf { it != other.issues }, + issuesB = other.issues.takeIf { it != issues }, + dependencyGraphA = dependencyGraphs.takeIf { it != other.dependencyGraphs }, + dependencyGraphB = other.dependencyGraphs.takeIf { it != dependencyGraphs } + ) + } +} + private enum class CompareMethod { SEMANTIC_DIFF, TEXT_DIFF @@ -348,7 +458,8 @@ private fun Map.replaceIn(text: String) = } private data class OrtResultDiff( - val repositoryDiff: RepositoryDiff? = null + val repositoryDiff: RepositoryDiff? = null, + val analyzerRunDiff: AnalyzerRunDiff? = null ) private data class RepositoryDiff( @@ -413,3 +524,24 @@ private data class AnalyzerConfigurationDiff( val skipExcludedA: Boolean? = false, val skipExcludedB: Boolean? = false ) +private data class AnalyzerRunDiff( + val startTimeA: Instant? = null, + val startTimeB: Instant? = null, + val endTimeA: Instant? = null, + val endTimeB: Instant? = null, + val environmentA: Environment? = null, + val environmentB: Environment? = null, + val configDiff: AnalyzerConfigurationDiff? = null, + val resultDiff: AnalyzerResultDiff? = null +) + +private data class AnalyzerResultDiff( + val projectsA: Set? = null, + val projectsB: Set? = null, + val packagesA: Set? = null, + val packagesB: Set? = null, + val issuesA: Map>? = null, + val issuesB: Map>? = null, + val dependencyGraphA: Map? = null, + val dependencyGraphB: Map? = null +) From 8802dee9a21957bc7d010eb036c2e15e219c09e9 Mon Sep 17 00:00:00 2001 From: Onur Demirci Date: Tue, 28 Nov 2023 09:46:31 +0100 Subject: [PATCH 03/10] feat(CompareCommand): Add a comparison for the `ScannerRun`s Signed-off-by: Onur Demirci --- .../compare/src/main/kotlin/CompareCommand.kt | 168 +++++++++++++++++- 1 file changed, 166 insertions(+), 2 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index d664ec5521316..136659c7f7a68 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -45,17 +45,23 @@ import java.time.Instant import org.ossreviewtoolkit.model.AnalyzerResult import org.ossreviewtoolkit.model.AnalyzerRun import org.ossreviewtoolkit.model.DependencyGraph +import org.ossreviewtoolkit.model.FileList import org.ossreviewtoolkit.model.Identifier import org.ossreviewtoolkit.model.Issue import org.ossreviewtoolkit.model.OrtResult import org.ossreviewtoolkit.model.Package import org.ossreviewtoolkit.model.PackageCuration import org.ossreviewtoolkit.model.Project +import org.ossreviewtoolkit.model.ProvenanceResolutionResult import org.ossreviewtoolkit.model.Repository +import org.ossreviewtoolkit.model.ScanResult +import org.ossreviewtoolkit.model.ScannerRun import org.ossreviewtoolkit.model.VcsInfo import org.ossreviewtoolkit.model.config.AnalyzerConfiguration import org.ossreviewtoolkit.model.config.Curations import org.ossreviewtoolkit.model.config.Excludes +import org.ossreviewtoolkit.model.config.FileArchiverConfiguration +import org.ossreviewtoolkit.model.config.FileListStorageConfiguration import org.ossreviewtoolkit.model.config.IssueResolution import org.ossreviewtoolkit.model.config.LicenseChoices import org.ossreviewtoolkit.model.config.LicenseFindingCuration @@ -63,10 +69,14 @@ import org.ossreviewtoolkit.model.config.PackageConfiguration import org.ossreviewtoolkit.model.config.PackageLicenseChoice import org.ossreviewtoolkit.model.config.PackageManagerConfiguration import org.ossreviewtoolkit.model.config.PathExclude +import org.ossreviewtoolkit.model.config.PluginConfiguration +import org.ossreviewtoolkit.model.config.ProvenanceStorageConfiguration import org.ossreviewtoolkit.model.config.RepositoryAnalyzerConfiguration import org.ossreviewtoolkit.model.config.RepositoryConfiguration import org.ossreviewtoolkit.model.config.Resolutions import org.ossreviewtoolkit.model.config.RuleViolationResolution +import org.ossreviewtoolkit.model.config.ScanStorageConfiguration +import org.ossreviewtoolkit.model.config.ScannerConfiguration import org.ossreviewtoolkit.model.config.ScopeExclude import org.ossreviewtoolkit.model.config.VulnerabilityResolution import org.ossreviewtoolkit.model.mapper @@ -201,7 +211,8 @@ class CompareCommand : OrtCommand( private fun OrtResult.diff(other: OrtResult) = OrtResultDiff( repositoryDiff = repository.diff(other.repository), - analyzerRunDiff = analyzer.diff(other.analyzer) + analyzerRunDiff = analyzer.diff(other.analyzer), + scannerRunDiff = scanner.diff(other.scanner) ) private fun Repository.diff(other: Repository): RepositoryDiff? = @@ -437,6 +448,115 @@ private fun AnalyzerResult?.diff(other: AnalyzerResult?): AnalyzerResultDiff? { } } +private fun ScannerRun?.diff(other: ScannerRun?): ScannerRunDiff? { + if (this == other) return null + + return if (this == null) { + ScannerRunDiff( + startTimeB = other?.startTime, + endTimeB = other?.endTime, + environmentB = other?.environment, + config = diff(other?.config), + provenancesB = other?.provenances, + scanResultsB = other?.scanResults, + scannersB = other?.scanners, + filesB = other?.files + ) + } else if (other == null) { + ScannerRunDiff( + startTimeA = startTime, + endTimeA = endTime, + environmentA = environment, + config = config.diff(null), + provenancesA = provenances, + scanResultsA = scanResults, + scannersA = scanners, + filesA = files + ) + } else { + ScannerRunDiff( + startTimeA = startTime.takeIf { it != other.startTime }, + startTimeB = other.startTime.takeIf { it != startTime }, + endTimeA = endTime.takeIf { it != other.endTime }, + endTimeB = other.endTime.takeIf { it != endTime }, + environmentA = environment.takeIf { it != other.environment }, + environmentB = other.environment.takeIf { it != environment }, + config = config.diff(other.config), + provenancesA = (provenances - other.provenances).takeUnless { it.isEmpty() }, + provenancesB = (other.provenances - provenances).takeUnless { it.isEmpty() }, + scanResultsA = (scanResults - other.scanResults).takeUnless { it.isEmpty() }, + scanResultsB = (other.scanResults - scanResults).takeUnless { it.isEmpty() }, + scannersA = scanners.takeIf { it != other.scanners }, + scannersB = other.scanners.takeIf { it != scanners }, + filesA = (files - other.files).takeUnless { it.isEmpty() }, + filesB = (other.files - files).takeUnless { it.isEmpty() } + ) + } +} + +private fun ScannerConfiguration?.diff(other: ScannerConfiguration?): ScannerConfigurationDiff? { + if (this == other) return null + + return if (this == null) { + ScannerConfigurationDiff( + skipConcludedB = other?.skipConcluded, + archiveB = other?.archive, + createMissingArchivesB = other?.createMissingArchives, + detectedLicenseMappingB = other?.detectedLicenseMapping, + fileListStorageB = other?.fileListStorage, + configB = other?.config, + storagesB = other?.storages, + storageReadersB = other?.storageReaders, + storageWritersB = other?.storageWriters, + ignorePatternsB = other?.ignorePatterns, + provenanceStorageB = other?.provenanceStorage + ) + } else if (other == null) { + ScannerConfigurationDiff( + skipConcludedA = skipConcluded, + archiveA = archive, + createMissingArchivesA = createMissingArchives, + detectedLicenseMappingA = detectedLicenseMapping, + fileListStorageA = fileListStorage, + configA = config, + storagesA = storages, + storageReadersA = storageReaders, + storageWritersA = storageWriters, + ignorePatternsA = ignorePatterns, + provenanceStorageA = provenanceStorage + ) + } else { + ScannerConfigurationDiff( + skipConcludedA = skipConcluded.takeIf { it != other.skipConcluded }, + skipConcludedB = other.skipConcluded.takeIf { it != skipConcluded }, + archiveA = archive.takeIf { it != other.archive }, + archiveB = other.archive.takeIf { it != archive }, + createMissingArchivesA = createMissingArchives.takeIf { it != other.createMissingArchives }, + createMissingArchivesB = other.createMissingArchives.takeIf { it != createMissingArchives }, + detectedLicenseMappingA = detectedLicenseMapping.takeIf { it != other.detectedLicenseMapping }, + detectedLicenseMappingB = other.detectedLicenseMapping.takeIf { it != detectedLicenseMapping }, + fileListStorageA = fileListStorage.takeIf { it != other.fileListStorage }, + fileListStorageB = other.fileListStorage.takeIf { it != fileListStorage }, + configA = config.takeIf { it != other.config }, + configB = other.config.takeIf { it != config }, + storagesA = storages.takeIf { it != other.storages }, + storagesB = other.storages.takeIf { it != storages }, + storageReadersA = (storageReaders.orEmpty() - other.storageReaders.orEmpty().toSet()) + .takeUnless { it.isEmpty() }, + storageReadersB = (other.storageReaders.orEmpty() - storageReaders.orEmpty().toSet()) + .takeUnless { it.isEmpty() }, + storageWritersA = (storageWriters.orEmpty() - other.storageWriters.orEmpty().toSet()) + .takeUnless { it.isEmpty() }, + storageWritersB = (other.storageWriters.orEmpty() - storageWriters.orEmpty().toSet()) + .takeUnless { it.isEmpty() }, + ignorePatternsA = (ignorePatterns - other.ignorePatterns.toSet()).takeUnless { it.isEmpty() }, + ignorePatternsB = (other.ignorePatterns - ignorePatterns.toSet()).takeUnless { it.isEmpty() }, + provenanceStorageA = provenanceStorage.takeIf { it != other.provenanceStorage }, + provenanceStorageB = other.provenanceStorage.takeIf { it != provenanceStorage } + ) + } +} + private enum class CompareMethod { SEMANTIC_DIFF, TEXT_DIFF @@ -459,7 +579,8 @@ private fun Map.replaceIn(text: String) = private data class OrtResultDiff( val repositoryDiff: RepositoryDiff? = null, - val analyzerRunDiff: AnalyzerRunDiff? = null + val analyzerRunDiff: AnalyzerRunDiff? = null, + val scannerRunDiff: ScannerRunDiff? = null ) private data class RepositoryDiff( @@ -545,3 +666,46 @@ private data class AnalyzerResultDiff( val dependencyGraphA: Map? = null, val dependencyGraphB: Map? = null ) + +private data class ScannerRunDiff( + val startTimeA: Instant? = null, + val startTimeB: Instant? = null, + val endTimeA: Instant? = null, + val endTimeB: Instant? = null, + val environmentA: Environment? = null, + val environmentB: Environment? = null, + val config: ScannerConfigurationDiff? = null, + val provenancesA: Set? = null, + val provenancesB: Set? = null, + val scanResultsA: Set? = null, + val scanResultsB: Set? = null, + val scannersA: Map>? = null, + val scannersB: Map>? = null, + val filesA: Set? = null, + val filesB: Set? = null +) + +private data class ScannerConfigurationDiff( + val skipConcludedA: Boolean? = null, + val skipConcludedB: Boolean? = null, + val archiveA: FileArchiverConfiguration? = null, + val archiveB: FileArchiverConfiguration? = null, + val createMissingArchivesA: Boolean? = null, + val createMissingArchivesB: Boolean? = null, + val detectedLicenseMappingA: Map? = null, + val detectedLicenseMappingB: Map? = null, + val fileListStorageA: FileListStorageConfiguration? = null, + val fileListStorageB: FileListStorageConfiguration? = null, + val configA: Map? = null, + val configB: Map? = null, + val storagesA: Map? = null, + val storagesB: Map? = null, + val storageReadersA: List? = null, + val storageReadersB: List? = null, + val storageWritersA: List? = null, + val storageWritersB: List? = null, + val ignorePatternsA: List? = null, + val ignorePatternsB: List? = null, + val provenanceStorageA: ProvenanceStorageConfiguration? = null, + val provenanceStorageB: ProvenanceStorageConfiguration? = null +) From d15adc5c8ff70ef8134a663736d33c63f3c91e14 Mon Sep 17 00:00:00 2001 From: Onur Demirci Date: Tue, 28 Nov 2023 09:46:31 +0100 Subject: [PATCH 04/10] feat(CompareCommand): Add a comparison for the `AdvisorRun`s Signed-off-by: Onur Demirci --- .../compare/src/main/kotlin/CompareCommand.kt | 57 ++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index 136659c7f7a68..af574efca8c04 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -42,6 +42,8 @@ import com.github.difflib.UnifiedDiffUtils import java.time.Instant +import org.ossreviewtoolkit.model.AdvisorRecord +import org.ossreviewtoolkit.model.AdvisorRun import org.ossreviewtoolkit.model.AnalyzerResult import org.ossreviewtoolkit.model.AnalyzerRun import org.ossreviewtoolkit.model.DependencyGraph @@ -57,6 +59,7 @@ import org.ossreviewtoolkit.model.Repository import org.ossreviewtoolkit.model.ScanResult import org.ossreviewtoolkit.model.ScannerRun import org.ossreviewtoolkit.model.VcsInfo +import org.ossreviewtoolkit.model.config.AdvisorConfiguration import org.ossreviewtoolkit.model.config.AnalyzerConfiguration import org.ossreviewtoolkit.model.config.Curations import org.ossreviewtoolkit.model.config.Excludes @@ -212,7 +215,8 @@ private fun OrtResult.diff(other: OrtResult) = OrtResultDiff( repositoryDiff = repository.diff(other.repository), analyzerRunDiff = analyzer.diff(other.analyzer), - scannerRunDiff = scanner.diff(other.scanner) + scannerRunDiff = scanner.diff(other.scanner), + advisorRunDiff = advisor.diff(other.advisor) ) private fun Repository.diff(other: Repository): RepositoryDiff? = @@ -557,6 +561,41 @@ private fun ScannerConfiguration?.diff(other: ScannerConfiguration?): ScannerCon } } +private fun AdvisorRun?.diff(other: AdvisorRun?): AdvisorRunDiff? { + if (this == other) return null + + return if (this == null) { + AdvisorRunDiff( + startTimeB = other?.startTime, + endTimeB = other?.endTime, + environmentB = other?.environment, + configB = other?.config, + resultsB = other?.results + ) + } else if (other == null) { + AdvisorRunDiff( + startTimeA = startTime, + endTimeA = endTime, + environmentA = environment, + configA = config, + resultsA = results + ) + } else { + AdvisorRunDiff( + startTimeA = startTime.takeIf { it != other.startTime }, + startTimeB = other.startTime.takeIf { it != startTime }, + endTimeA = endTime.takeIf { it != other.endTime }, + endTimeB = other.endTime.takeIf { it != endTime }, + environmentA = environment.takeIf { it != other.environment }, + environmentB = other.environment.takeIf { it != environment }, + configA = config.takeIf { it != other.config }, + configB = other.config.takeIf { it != config }, + resultsA = results.takeIf { it != other.results }, + resultsB = other.results.takeIf { it != results } + ) + } +} + private enum class CompareMethod { SEMANTIC_DIFF, TEXT_DIFF @@ -580,7 +619,8 @@ private fun Map.replaceIn(text: String) = private data class OrtResultDiff( val repositoryDiff: RepositoryDiff? = null, val analyzerRunDiff: AnalyzerRunDiff? = null, - val scannerRunDiff: ScannerRunDiff? = null + val scannerRunDiff: ScannerRunDiff? = null, + val advisorRunDiff: AdvisorRunDiff? = null ) private data class RepositoryDiff( @@ -709,3 +749,16 @@ private data class ScannerConfigurationDiff( val provenanceStorageA: ProvenanceStorageConfiguration? = null, val provenanceStorageB: ProvenanceStorageConfiguration? = null ) + +private data class AdvisorRunDiff( + val startTimeA: Instant? = null, + val startTimeB: Instant? = null, + val endTimeA: Instant? = null, + val endTimeB: Instant? = null, + val environmentA: Environment? = null, + val environmentB: Environment? = null, + val configA: AdvisorConfiguration? = null, + val configB: AdvisorConfiguration? = null, + val resultsA: AdvisorRecord? = null, + val resultsB: AdvisorRecord? = null +) From 1617a1267d51d63666545ea8c6ed753515decc5a Mon Sep 17 00:00:00 2001 From: Onur Demirci Date: Tue, 28 Nov 2023 09:46:31 +0100 Subject: [PATCH 05/10] feat(CompareCommand): Add a comparison for the `EvaluatorRun`s Signed-off-by: Onur Demirci --- .../compare/src/main/kotlin/CompareCommand.kt | 44 ++++++++++++++++++- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index af574efca8c04..33d52b812ba52 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -47,6 +47,7 @@ import org.ossreviewtoolkit.model.AdvisorRun import org.ossreviewtoolkit.model.AnalyzerResult import org.ossreviewtoolkit.model.AnalyzerRun import org.ossreviewtoolkit.model.DependencyGraph +import org.ossreviewtoolkit.model.EvaluatorRun import org.ossreviewtoolkit.model.FileList import org.ossreviewtoolkit.model.Identifier import org.ossreviewtoolkit.model.Issue @@ -56,6 +57,7 @@ import org.ossreviewtoolkit.model.PackageCuration import org.ossreviewtoolkit.model.Project import org.ossreviewtoolkit.model.ProvenanceResolutionResult import org.ossreviewtoolkit.model.Repository +import org.ossreviewtoolkit.model.RuleViolation import org.ossreviewtoolkit.model.ScanResult import org.ossreviewtoolkit.model.ScannerRun import org.ossreviewtoolkit.model.VcsInfo @@ -216,7 +218,8 @@ private fun OrtResult.diff(other: OrtResult) = repositoryDiff = repository.diff(other.repository), analyzerRunDiff = analyzer.diff(other.analyzer), scannerRunDiff = scanner.diff(other.scanner), - advisorRunDiff = advisor.diff(other.advisor) + advisorRunDiff = advisor.diff(other.advisor), + evaluatorRunDiff = evaluator.diff(other.evaluator) ) private fun Repository.diff(other: Repository): RepositoryDiff? = @@ -561,6 +564,33 @@ private fun ScannerConfiguration?.diff(other: ScannerConfiguration?): ScannerCon } } +private fun EvaluatorRun?.diff(other: EvaluatorRun?): EvaluatorRunDiff? { + if (this == other) return null + + return if (this == null) { + EvaluatorRunDiff( + startTimeB = other?.startTime, + endTimeB = other?.endTime, + violationsB = other?.violations + ) + } else if (other == null) { + EvaluatorRunDiff( + startTimeA = startTime, + endTimeA = endTime, + violationsA = violations + ) + } else { + EvaluatorRunDiff( + startTimeA = startTime.takeIf { it != other.startTime }, + startTimeB = other.startTime.takeIf { it != startTime }, + endTimeA = endTime.takeIf { it != other.endTime }, + endTimeB = other.endTime.takeIf { it != endTime }, + violationsA = (violations - other.violations.toSet()).takeUnless { it.isEmpty() }, + violationsB = (other.violations - violations.toSet()).takeUnless { it.isEmpty() } + ) + } +} + private fun AdvisorRun?.diff(other: AdvisorRun?): AdvisorRunDiff? { if (this == other) return null @@ -620,7 +650,8 @@ private data class OrtResultDiff( val repositoryDiff: RepositoryDiff? = null, val analyzerRunDiff: AnalyzerRunDiff? = null, val scannerRunDiff: ScannerRunDiff? = null, - val advisorRunDiff: AdvisorRunDiff? = null + val advisorRunDiff: AdvisorRunDiff? = null, + val evaluatorRunDiff: EvaluatorRunDiff? = null ) private data class RepositoryDiff( @@ -762,3 +793,12 @@ private data class AdvisorRunDiff( val resultsA: AdvisorRecord? = null, val resultsB: AdvisorRecord? = null ) + +private data class EvaluatorRunDiff( + val startTimeA: Instant? = null, + val startTimeB: Instant? = null, + val endTimeA: Instant? = null, + val endTimeB: Instant? = null, + val violationsA: List? = null, + val violationsB: List? = null +) From 1814c02021d0b3c10abc32539448a119d57bfe1c Mon Sep 17 00:00:00 2001 From: Onur Demirci Date: Tue, 28 Nov 2023 09:46:31 +0100 Subject: [PATCH 06/10] feat(CompareCommand): Add a comparison for the `ResolvedConfiguration`s Signed-off-by: Onur Demirci --- .../compare/src/main/kotlin/CompareCommand.kt | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index 33d52b812ba52..a1b323c97f27e 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -57,6 +57,8 @@ import org.ossreviewtoolkit.model.PackageCuration import org.ossreviewtoolkit.model.Project import org.ossreviewtoolkit.model.ProvenanceResolutionResult import org.ossreviewtoolkit.model.Repository +import org.ossreviewtoolkit.model.ResolvedConfiguration +import org.ossreviewtoolkit.model.ResolvedPackageCurations import org.ossreviewtoolkit.model.RuleViolation import org.ossreviewtoolkit.model.ScanResult import org.ossreviewtoolkit.model.ScannerRun @@ -219,7 +221,8 @@ private fun OrtResult.diff(other: OrtResult) = analyzerRunDiff = analyzer.diff(other.analyzer), scannerRunDiff = scanner.diff(other.scanner), advisorRunDiff = advisor.diff(other.advisor), - evaluatorRunDiff = evaluator.diff(other.evaluator) + evaluatorRunDiff = evaluator.diff(other.evaluator), + resolvedConfigurationDiff = resolvedConfiguration.diff(other.resolvedConfiguration) ) private fun Repository.diff(other: Repository): RepositoryDiff? = @@ -626,6 +629,21 @@ private fun AdvisorRun?.diff(other: AdvisorRun?): AdvisorRunDiff? { } } +private fun ResolvedConfiguration.diff(other: ResolvedConfiguration): ResolvedConfigurationDiff? = + if (this == other) { + null + } else { + ResolvedConfigurationDiff( + packageConfigurationsA = (packageConfigurations.orEmpty() - other.packageConfigurations.orEmpty().toSet()) + .takeUnless { it.isEmpty() }, + packageConfigurationsB = (other.packageConfigurations.orEmpty() - packageConfigurations.orEmpty().toSet()) + .takeUnless { it.isEmpty() }, + packageCurationsA = (packageCurations - other.packageCurations.toSet()).takeUnless { it.isEmpty() }, + packageCurationsB = (other.packageCurations - packageCurations.toSet()).takeUnless { it.isEmpty() }, + resolutionsDiff = resolutions?.diff(other.resolutions) + ) + } + private enum class CompareMethod { SEMANTIC_DIFF, TEXT_DIFF @@ -651,7 +669,8 @@ private data class OrtResultDiff( val analyzerRunDiff: AnalyzerRunDiff? = null, val scannerRunDiff: ScannerRunDiff? = null, val advisorRunDiff: AdvisorRunDiff? = null, - val evaluatorRunDiff: EvaluatorRunDiff? = null + val evaluatorRunDiff: EvaluatorRunDiff? = null, + val resolvedConfigurationDiff: ResolvedConfigurationDiff? = null ) private data class RepositoryDiff( @@ -802,3 +821,11 @@ private data class EvaluatorRunDiff( val violationsA: List? = null, val violationsB: List? = null ) + +private data class ResolvedConfigurationDiff( + val packageConfigurationsA: List? = null, + val packageConfigurationsB: List? = null, + val packageCurationsA: List? = null, + val packageCurationsB: List? = null, + val resolutionsDiff: ResolutionsDiff? = null +) From 4c86abe9370c62937be6ced732808f3768721b6f Mon Sep 17 00:00:00 2001 From: Oliver Heger Date: Mon, 4 Dec 2023 15:16:33 +0100 Subject: [PATCH 07/10] feat(cli): Reduce delta of semantic diff method in CompareCommand Allow for some lenient comparisons and print only differences in license and snippet findings. Signed-off-by: Oliver Heger --- .../compare/src/main/kotlin/CompareCommand.kt | 251 +++++++++++++++--- 1 file changed, 220 insertions(+), 31 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index a1b323c97f27e..1bcef150a3b27 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -46,22 +46,28 @@ import org.ossreviewtoolkit.model.AdvisorRecord import org.ossreviewtoolkit.model.AdvisorRun import org.ossreviewtoolkit.model.AnalyzerResult import org.ossreviewtoolkit.model.AnalyzerRun +import org.ossreviewtoolkit.model.CopyrightFinding import org.ossreviewtoolkit.model.DependencyGraph import org.ossreviewtoolkit.model.EvaluatorRun import org.ossreviewtoolkit.model.FileList import org.ossreviewtoolkit.model.Identifier import org.ossreviewtoolkit.model.Issue +import org.ossreviewtoolkit.model.LicenseFinding import org.ossreviewtoolkit.model.OrtResult import org.ossreviewtoolkit.model.Package import org.ossreviewtoolkit.model.PackageCuration import org.ossreviewtoolkit.model.Project +import org.ossreviewtoolkit.model.Provenance import org.ossreviewtoolkit.model.ProvenanceResolutionResult import org.ossreviewtoolkit.model.Repository import org.ossreviewtoolkit.model.ResolvedConfiguration import org.ossreviewtoolkit.model.ResolvedPackageCurations import org.ossreviewtoolkit.model.RuleViolation import org.ossreviewtoolkit.model.ScanResult +import org.ossreviewtoolkit.model.ScanSummary +import org.ossreviewtoolkit.model.ScannerDetails import org.ossreviewtoolkit.model.ScannerRun +import org.ossreviewtoolkit.model.SnippetFinding import org.ossreviewtoolkit.model.VcsInfo import org.ossreviewtoolkit.model.config.AdvisorConfiguration import org.ossreviewtoolkit.model.config.AnalyzerConfiguration @@ -129,6 +135,16 @@ class CompareCommand : OrtCommand( help = "Ignore temporary directory differences." ).flag() + private val ignoreConfig by option( + "--ignore-config", "-c", + help = "Ignore configuration options." + ).flag() + + private val ignoreResultsFromDifferentScannerVersions by option( + "--ignore-version-mismatch", "-v", + help = "Ignore scan results if they have been created by different scanner versions." + ).flag() + override fun run() { if (fileA == fileB) { echo("The arguments point to the same file.") @@ -165,7 +181,14 @@ class CompareCommand : OrtCommand( throw ProgramResult(0) } - val diff = resultA.diff(resultB) + val context = SemanticDiffContext( + ignoreConfig = ignoreConfig, + ignoreResultsFromDifferentScannerVersions = ignoreResultsFromDifferentScannerVersions, + packageConfigMapper = ::lenientPackageConfigurationMapper, + scanResultMapper = ::lenientScanResultMapper + ) + + val diff = resultA.diff(resultB, context) echo(deserializer.writeValueAsString(diff)) @@ -215,17 +238,17 @@ class CompareCommand : OrtCommand( } } -private fun OrtResult.diff(other: OrtResult) = +private fun OrtResult.diff(other: OrtResult, context: SemanticDiffContext) = OrtResultDiff( - repositoryDiff = repository.diff(other.repository), + repositoryDiff = repository.diff(other.repository, context), analyzerRunDiff = analyzer.diff(other.analyzer), - scannerRunDiff = scanner.diff(other.scanner), + scannerRunDiff = scanner.diff(other.scanner, context), advisorRunDiff = advisor.diff(other.advisor), evaluatorRunDiff = evaluator.diff(other.evaluator), resolvedConfigurationDiff = resolvedConfiguration.diff(other.resolvedConfiguration) ) -private fun Repository.diff(other: Repository): RepositoryDiff? = +private fun Repository.diff(other: Repository, context: SemanticDiffContext): RepositoryDiff? = if (this == other) { null } else { @@ -236,11 +259,14 @@ private fun Repository.diff(other: Repository): RepositoryDiff? = vcsProcessedB = other.vcsProcessed.takeIf { it != vcsProcessed }, nestedRepositoriesA = nestedRepositories.takeIf { it != other.nestedRepositories }, nestedRepositoriesB = other.nestedRepositories.takeIf { it != nestedRepositories }, - configDiff = config.diff(other.config) + configDiff = config.diff(other.config, context) ) } -private fun RepositoryConfiguration.diff(other: RepositoryConfiguration): RepositoryConfigurationDiff? = +private fun RepositoryConfiguration.diff( + other: RepositoryConfiguration, + context: SemanticDiffContext +): RepositoryConfigurationDiff? = if (this == other) { null } else { @@ -249,10 +275,16 @@ private fun RepositoryConfiguration.diff(other: RepositoryConfiguration): Reposi excludeDiff = excludes.diff(other.excludes), resolutionsDiff = resolutions.diff(other.resolutions), curationsDiff = curations.diff(other.curations), - packageConfigurationsA = (packageConfigurations - other.packageConfigurations.toSet()) - .takeUnless { it.isEmpty() }, - packageConfigurationsB = (other.packageConfigurations - packageConfigurations.toSet()) - .takeUnless { it.isEmpty() }, + packageConfigurationsA = mappedDiff( + packageConfigurations, + other.packageConfigurations, + context.packageConfigMapper + ), + packageConfigurationsB = mappedDiff( + other.packageConfigurations, + packageConfigurations.toSet(), + context.packageConfigMapper + ), licenseChoicesDiff = licenseChoices.diff(other.licenseChoices) ) } @@ -458,7 +490,7 @@ private fun AnalyzerResult?.diff(other: AnalyzerResult?): AnalyzerResultDiff? { } } -private fun ScannerRun?.diff(other: ScannerRun?): ScannerRunDiff? { +private fun ScannerRun?.diff(other: ScannerRun?, context: SemanticDiffContext): ScannerRunDiff? { if (this == other) return null return if (this == null) { @@ -468,9 +500,9 @@ private fun ScannerRun?.diff(other: ScannerRun?): ScannerRunDiff? { environmentB = other?.environment, config = diff(other?.config), provenancesB = other?.provenances, - scanResultsB = other?.scanResults, scannersB = other?.scanners, - filesB = other?.files + filesB = other?.files, + scanResultDiff = other?.scanResults?.mapNotNullTo(mutableSetOf()) { null.diff(it, context) } ) } else if (other == null) { ScannerRunDiff( @@ -479,11 +511,22 @@ private fun ScannerRun?.diff(other: ScannerRun?): ScannerRunDiff? { environmentA = environment, config = config.diff(null), provenancesA = provenances, - scanResultsA = scanResults, scannersA = scanners, - filesA = files + filesA = files, + scanResultDiff = scanResults.mapNotNullTo(mutableSetOf()) { it.diff(null, context) } ) } else { + val scanResultsA = scanResults.map(context.scanResultMapper) + val scanResultsAMapping = scanResultsA.associateBy { result -> (result.provenance to result.scanner.name) } + val scanResultsB = other.scanResults.map(context.scanResultMapper) + val scanResultsBMapping = scanResultsB.associateBy { result -> (result.provenance to result.scanner.name) } + + val differentResults = scanResultsA.mapNotNullTo(mutableSetOf()) { + it.diff(scanResultsBMapping[it.provenance to it.scanner.name], context) + } + scanResultsB.mapNotNullTo(mutableSetOf()) { + scanResultsAMapping[it.provenance to it.scanner.name].diff(it, context) + } + ScannerRunDiff( startTimeA = startTime.takeIf { it != other.startTime }, startTimeB = other.startTime.takeIf { it != startTime }, @@ -491,15 +534,14 @@ private fun ScannerRun?.diff(other: ScannerRun?): ScannerRunDiff? { endTimeB = other.endTime.takeIf { it != endTime }, environmentA = environment.takeIf { it != other.environment }, environmentB = other.environment.takeIf { it != environment }, - config = config.diff(other.config), + config = if (context.ignoreConfig) null else config.diff(other.config), provenancesA = (provenances - other.provenances).takeUnless { it.isEmpty() }, provenancesB = (other.provenances - provenances).takeUnless { it.isEmpty() }, - scanResultsA = (scanResults - other.scanResults).takeUnless { it.isEmpty() }, - scanResultsB = (other.scanResults - scanResults).takeUnless { it.isEmpty() }, scannersA = scanners.takeIf { it != other.scanners }, scannersB = other.scanners.takeIf { it != scanners }, filesA = (files - other.files).takeUnless { it.isEmpty() }, - filesB = (other.files - files).takeUnless { it.isEmpty() } + filesB = (other.files - files).takeUnless { it.isEmpty() }, + scanResultDiff = differentResults.takeUnless { it.isEmpty() } ) } } @@ -567,6 +609,91 @@ private fun ScannerConfiguration?.diff(other: ScannerConfiguration?): ScannerCon } } +private fun ScanResult?.diff(other: ScanResult?, context: SemanticDiffContext): ScanResultDiff? { + val thisMapped = this?.let { context.scanResultMapper(it) } + val otherMapped = other?.let { context.scanResultMapper(it) } + if (thisMapped == otherMapped) return null + + return if (thisMapped == null) { + ScanResultDiff( + provenance = other?.provenance, + scanner = other?.scanner, + summaryDiff = null.diff(other?.summary) + ) + } else if (otherMapped == null) { + ScanResultDiff( + provenance = thisMapped.provenance, + scanner = thisMapped.scanner, + summaryDiff = thisMapped.summary.diff(null) + ) + } else { + if (context.ignoreResultsFromDifferentScannerVersions && + thisMapped.scanner.version != otherMapped.scanner.version + ) { + null + } else { + ScanResultDiff( + provenance = thisMapped.provenance, + scanner = thisMapped.scanner, + summaryDiff = thisMapped.summary.diff(otherMapped.summary) + ) + } + } +} + +private fun ScanSummary?.diff(other: ScanSummary?): ScanSummaryDiff? { + if (this == other) return null + + return if (this == null) { + ScanSummaryDiff( + licenseFindingsB = other?.licenseFindings, + copyrightFindingsB = other?.copyrightFindings, + snippetFindingsB = other?.snippetFindings, + issuesB = other?.issues?.toSet() + ) + } else if (other == null) { + ScanSummaryDiff( + licenseFindingsA = licenseFindings, + copyrightFindingsA = copyrightFindings, + snippetFindingsA = snippetFindings, + issuesA = issues.toSet() + ) + } else { + val lenientSnippets = snippetFindings.map(::lenientSnippetFindingMapper) + val otherLenientSnippets = other.snippetFindings.map(::lenientSnippetFindingMapper) + val snippetFindingsA = mappedDiff(lenientSnippets, otherLenientSnippets, identityMapper()) + val reducedSnippetFindingsA = snippetFindingsA?.mapTo(mutableSetOf()) { + reduceSnippedFindingDelta(it, otherLenientSnippets) + } + + val snippetFindingsB = mappedDiff(otherLenientSnippets, lenientSnippets, identityMapper()) + val reducedSnippedFindingsB = snippetFindingsB?.mapTo(mutableSetOf()) { + reduceSnippedFindingDelta(it, lenientSnippets) + } + + ScanSummaryDiff( + licenseFindingsA = mappedDiff(licenseFindings, other.licenseFindings, identityMapper()), + licenseFindingsB = mappedDiff(other.licenseFindings, licenseFindings, identityMapper()), + copyrightFindingsA = mappedDiff(copyrightFindings, other.copyrightFindings, identityMapper()), + copyrightFindingsB = mappedDiff(other.copyrightFindings, copyrightFindings, identityMapper()), + snippetFindingsA = reducedSnippetFindingsA, + snippetFindingsB = reducedSnippedFindingsB, + issuesA = mappedDiff(issues, other.issues, identityMapper()), + issuesB = mappedDiff(other.issues, issues, identityMapper()) + ) + } +} + +private fun reduceSnippedFindingDelta( + finding: SnippetFinding, + otherFindings: Collection +): SnippetFinding = + otherFindings.filter { it.sourceLocation == finding.sourceLocation } + .map { it to it.snippets.subtract(finding.snippets) } + .minByOrNull { it.second.size }?.first?.let { other -> + finding.copy(snippets = finding.snippets - other.snippets) + } ?: finding + private fun EvaluatorRun?.diff(other: EvaluatorRun?): EvaluatorRunDiff? { if (this == other) return null @@ -588,8 +715,8 @@ private fun EvaluatorRun?.diff(other: EvaluatorRun?): EvaluatorRunDiff? { startTimeB = other.startTime.takeIf { it != startTime }, endTimeA = endTime.takeIf { it != other.endTime }, endTimeB = other.endTime.takeIf { it != endTime }, - violationsA = (violations - other.violations.toSet()).takeUnless { it.isEmpty() }, - violationsB = (other.violations - violations.toSet()).takeUnless { it.isEmpty() } + violationsA = mappedDiff(violations, other.violations.toSet(), ::lenientViolationMapper)?.toList(), + violationsB = mappedDiff(other.violations, violations.toSet(), ::lenientViolationMapper)?.toList() ) } } @@ -634,10 +761,16 @@ private fun ResolvedConfiguration.diff(other: ResolvedConfiguration): ResolvedCo null } else { ResolvedConfigurationDiff( - packageConfigurationsA = (packageConfigurations.orEmpty() - other.packageConfigurations.orEmpty().toSet()) - .takeUnless { it.isEmpty() }, - packageConfigurationsB = (other.packageConfigurations.orEmpty() - packageConfigurations.orEmpty().toSet()) - .takeUnless { it.isEmpty() }, + packageConfigurationsA = mappedDiff( + packageConfigurations.orEmpty(), + other.packageConfigurations.orEmpty(), + ::lenientPackageConfigurationMapper + )?.toList(), + packageConfigurationsB = mappedDiff( + other.packageConfigurations.orEmpty(), + packageConfigurations.orEmpty(), + ::lenientPackageConfigurationMapper + )?.toList(), packageCurationsA = (packageCurations - other.packageCurations.toSet()).takeUnless { it.isEmpty() }, packageCurationsB = (other.packageCurations - packageCurations.toSet()).takeUnless { it.isEmpty() }, resolutionsDiff = resolutions?.diff(other.resolutions) @@ -664,6 +797,45 @@ private fun Map.replaceIn(text: String) = currentText.replace(from, to) } +typealias DiffMapper = (T) -> T + +private fun lenientPackageConfigurationMapper(configuration: PackageConfiguration): PackageConfiguration = + configuration.copy( + pathExcludes = configuration.pathExcludes.distinct().sortedBy { it.pattern }, + licenseFindingCurations = configuration.licenseFindingCurations.distinct().sortedBy { it.path } + ) + +private fun lenientScanResultMapper(scanResult: ScanResult): ScanResult = + scanResult.copy( + scanner = scanResult.scanner.copy(configuration = "") + ) + +private fun lenientViolationMapper(violation: RuleViolation): RuleViolation = + violation.copy( + message = violation.message.substringBefore('\n') + ) + +private fun lenientSnippetFindingMapper(snippetFinding: SnippetFinding): SnippetFinding { + val lenientSnippets = snippetFinding.snippets.mapTo(mutableSetOf()) { it.copy(additionalData = emptyMap()) } + return snippetFinding.copy(snippets = lenientSnippets) +} + +private fun identityMapper(): DiffMapper = { it } + +private fun mappedDiff(itemsA: Collection, itemsB: Collection, mapper: DiffMapper): Set? { + val mappedA = itemsA.mapTo(mutableSetOf(), mapper) + val mappedB = itemsB.mapTo(mutableSetOf(), mapper) + + return (mappedA - mappedB).takeUnless { it.isEmpty() } +} + +private data class SemanticDiffContext( + val ignoreConfig: Boolean = false, + val ignoreResultsFromDifferentScannerVersions: Boolean = false, + val packageConfigMapper: DiffMapper = identityMapper(), + val scanResultMapper: DiffMapper = identityMapper() +) + private data class OrtResultDiff( val repositoryDiff: RepositoryDiff? = null, val analyzerRunDiff: AnalyzerRunDiff? = null, @@ -688,8 +860,8 @@ private data class RepositoryConfigurationDiff( val excludeDiff: ExcludesDiff? = null, val resolutionsDiff: ResolutionsDiff? = null, val curationsDiff: CurationsDiff? = null, - val packageConfigurationsA: List? = null, - val packageConfigurationsB: List? = null, + val packageConfigurationsA: Set? = null, + val packageConfigurationsB: Set? = null, val licenseChoicesDiff: LicenseChoicesDiff? = null ) @@ -735,6 +907,7 @@ private data class AnalyzerConfigurationDiff( val skipExcludedA: Boolean? = false, val skipExcludedB: Boolean? = false ) + private data class AnalyzerRunDiff( val startTimeA: Instant? = null, val startTimeB: Instant? = null, @@ -767,12 +940,11 @@ private data class ScannerRunDiff( val config: ScannerConfigurationDiff? = null, val provenancesA: Set? = null, val provenancesB: Set? = null, - val scanResultsA: Set? = null, - val scanResultsB: Set? = null, val scannersA: Map>? = null, val scannersB: Map>? = null, val filesA: Set? = null, - val filesB: Set? = null + val filesB: Set? = null, + val scanResultDiff: Set? = null ) private data class ScannerConfigurationDiff( @@ -800,6 +972,23 @@ private data class ScannerConfigurationDiff( val provenanceStorageB: ProvenanceStorageConfiguration? = null ) +private data class ScanResultDiff( + val provenance: Provenance? = null, + val scanner: ScannerDetails? = null, + val summaryDiff: ScanSummaryDiff? = null +) + +private data class ScanSummaryDiff( + val licenseFindingsA: Set? = null, + val licenseFindingsB: Set? = null, + val copyrightFindingsA: Set? = null, + val copyrightFindingsB: Set? = null, + val snippetFindingsA: Set? = null, + val snippetFindingsB: Set? = null, + val issuesA: Set? = null, + val issuesB: Set? = null +) + private data class AdvisorRunDiff( val startTimeA: Instant? = null, val startTimeB: Instant? = null, From f00cecebbb236911f78b83baba743a7c889cc22e Mon Sep 17 00:00:00 2001 From: Martin Nonnenmacher Date: Thu, 7 Dec 2023 18:04:38 +0100 Subject: [PATCH 08/10] feat(cli): Improve handling of resolved package curations Improve the handling of resolved package curations in the `CompareCommand` by building a diff of the curations for each provider. Signed-off-by: Martin Nonnenmacher --- .../compare/src/main/kotlin/CompareCommand.kt | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index 1bcef150a3b27..634d542a0b318 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -771,8 +771,7 @@ private fun ResolvedConfiguration.diff(other: ResolvedConfiguration): ResolvedCo packageConfigurations.orEmpty(), ::lenientPackageConfigurationMapper )?.toList(), - packageCurationsA = (packageCurations - other.packageCurations.toSet()).takeUnless { it.isEmpty() }, - packageCurationsB = (other.packageCurations - packageCurations.toSet()).takeUnless { it.isEmpty() }, + packageCurations = packageCurations.diff(other.packageCurations), resolutionsDiff = resolutions?.diff(other.resolutions) ) } @@ -782,6 +781,22 @@ private enum class CompareMethod { TEXT_DIFF } +private fun List.diff(other: List): Map? { + val providers = (map { it.provider.id } + other.map { it.provider.id }).toSortedSet() + + return providers.associateWith { provider -> + val curationsA = find { it.provider.id == provider }?.curations.orEmpty() + val curationsB = other.find { it.provider.id == provider }?.curations.orEmpty() + + PackageCurationDiff( + packageCurationsA = (curationsA - curationsB).sortedBy { it.id }, + packageCurationsB = (curationsB - curationsA).sortedBy { it.id } + ) + }.takeIf { it.isNotEmpty() }?.filter { + it.value.packageCurationsA?.isNotEmpty() == true || it.value.packageCurationsB?.isNotEmpty() == true + } +} + private class EpochInstantDeserializer : StdDeserializer(Instant::class.java) { override fun deserialize(parser: JsonParser, context: DeserializationContext): Instant = Instant.EPOCH.also { parser.codec.readTree(parser) } @@ -1014,7 +1029,11 @@ private data class EvaluatorRunDiff( private data class ResolvedConfigurationDiff( val packageConfigurationsA: List? = null, val packageConfigurationsB: List? = null, - val packageCurationsA: List? = null, - val packageCurationsB: List? = null, + val packageCurations: Map? = null, val resolutionsDiff: ResolutionsDiff? = null ) + +private data class PackageCurationDiff( + val packageCurationsA: List? = null, + val packageCurationsB: List? = null +) From 99472dc688b0e6808d8438043dd47dca0f8a9580 Mon Sep 17 00:00:00 2001 From: Martin Nonnenmacher Date: Fri, 8 Dec 2023 08:37:41 +0100 Subject: [PATCH 09/10] feat(cli): Add option to ignore file lists to `CompareCommand` Signed-off-by: Martin Nonnenmacher --- .../compare/src/main/kotlin/CompareCommand.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index 634d542a0b318..d6b3e16c9d32c 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -130,6 +130,11 @@ class CompareCommand : OrtCommand( help = "Ignore environment differences." ).flag() + private val ignoreFileList by option( + "--ignore-file-list", "-f", + help = "Ignore environment differences." + ).flag() + private val ignoreTmpDir by option( "--ignore-tmp-dir", "-d", help = "Ignore temporary directory differences." @@ -183,6 +188,7 @@ class CompareCommand : OrtCommand( val context = SemanticDiffContext( ignoreConfig = ignoreConfig, + ignoreFileList = ignoreFileList, ignoreResultsFromDifferentScannerVersions = ignoreResultsFromDifferentScannerVersions, packageConfigMapper = ::lenientPackageConfigurationMapper, scanResultMapper = ::lenientScanResultMapper @@ -539,8 +545,8 @@ private fun ScannerRun?.diff(other: ScannerRun?, context: SemanticDiffContext): provenancesB = (other.provenances - provenances).takeUnless { it.isEmpty() }, scannersA = scanners.takeIf { it != other.scanners }, scannersB = other.scanners.takeIf { it != scanners }, - filesA = (files - other.files).takeUnless { it.isEmpty() }, - filesB = (other.files - files).takeUnless { it.isEmpty() }, + filesA = if (context.ignoreFileList) null else (files - other.files).takeUnless { it.isEmpty() }, + filesB = if (context.ignoreFileList) null else (other.files - files).takeUnless { it.isEmpty() }, scanResultDiff = differentResults.takeUnless { it.isEmpty() } ) } @@ -846,6 +852,7 @@ private fun mappedDiff(itemsA: Collection, itemsB: Collection, mapper: private data class SemanticDiffContext( val ignoreConfig: Boolean = false, + val ignoreFileList: Boolean = false, val ignoreResultsFromDifferentScannerVersions: Boolean = false, val packageConfigMapper: DiffMapper = identityMapper(), val scanResultMapper: DiffMapper = identityMapper() From 47c1c0c4da465d3f5cc1d6a9d09c6d771ea98e7c Mon Sep 17 00:00:00 2001 From: Martin Nonnenmacher Date: Fri, 8 Dec 2023 12:23:02 +0100 Subject: [PATCH 10/10] feat(cli): Improve scanner comparison in `CompareCommand` If the scanners used for the packages are different, only show the entries that actually are different, not the whole lists. Signed-off-by: Martin Nonnenmacher --- .../commands/compare/src/main/kotlin/CompareCommand.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt index d6b3e16c9d32c..6d7fb8c5b59c4 100644 --- a/plugins/commands/compare/src/main/kotlin/CompareCommand.kt +++ b/plugins/commands/compare/src/main/kotlin/CompareCommand.kt @@ -543,8 +543,8 @@ private fun ScannerRun?.diff(other: ScannerRun?, context: SemanticDiffContext): config = if (context.ignoreConfig) null else config.diff(other.config), provenancesA = (provenances - other.provenances).takeUnless { it.isEmpty() }, provenancesB = (other.provenances - provenances).takeUnless { it.isEmpty() }, - scannersA = scanners.takeIf { it != other.scanners }, - scannersB = other.scanners.takeIf { it != scanners }, + scannersA = scanners.diff(other.scanners).takeUnless { it.isEmpty() }, + scannersB = other.scanners.diff(scanners).takeUnless { it.isEmpty() }, filesA = if (context.ignoreFileList) null else (files - other.files).takeUnless { it.isEmpty() }, filesB = if (context.ignoreFileList) null else (other.files - files).takeUnless { it.isEmpty() }, scanResultDiff = differentResults.takeUnless { it.isEmpty() } @@ -552,6 +552,10 @@ private fun ScannerRun?.diff(other: ScannerRun?, context: SemanticDiffContext): } } +private fun Map>.diff(other: Map>): Map> { + return filter { (id, values) -> other[id] != values } +} + private fun ScannerConfiguration?.diff(other: ScannerConfiguration?): ScannerConfigurationDiff? { if (this == other) return null