Skip to content

Commit

Permalink
Move concludedLicense from Package to CuratedPackage
Browse files Browse the repository at this point in the history 
Generally speaking, a concluded license is a property of a curated
package. Thus, move the corresponding property from `Package` to
`CuratedPackage`. This invalidates the historic note about ORT itself not
setting the `concludedLicense` field, and it is a step towards cleanly
separating (eventually corrected) technical package metadata from making
legal conclusions.

Signed-off-by: Sebastian Schuberth <[email protected]>
  • Loading branch information
@sschuberth
sschuberth committed Aug 31, 2022
1 parent 6e244b1 commit c9542b8
Show file tree
Hide file tree
Showing 21 changed files with 218 additions and 192 deletions.
45 changes: 27 additions & 18 deletions analyzer/src/main/kotlin/managers/SpdxDocumentFile.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.ossreviewtoolkit.analyzer.managers.utils.PackageManagerDependencyHand
import org.ossreviewtoolkit.analyzer.managers.utils.SpdxDocumentCache
import org.ossreviewtoolkit.analyzer.managers.utils.SpdxResolvedDocument
import org.ossreviewtoolkit.downloader.VersionControlSystem
import org.ossreviewtoolkit.model.CuratedPackage
import org.ossreviewtoolkit.model.Hash
import org.ossreviewtoolkit.model.Identifier
import org.ossreviewtoolkit.model.OrtIssue
Expand Down Expand Up @@ -292,7 +293,7 @@ class SpdxDocumentFile(
/**
* Create a [Package] out of this [SpdxPackage].
*/
private fun SpdxPackage.toPackage(definitionFile: File?, doc: SpdxResolvedDocument): Package {
private fun SpdxPackage.toPackage(definitionFile: File?, doc: SpdxResolvedDocument): CuratedPackage {
val packageDescription = description.takeUnless { it.isEmpty() } ?: summary

// If the VCS information cannot be determined from the VCS working tree itself, fall back to try getting it
Expand All @@ -310,18 +311,20 @@ class SpdxDocumentFile(
val id = toIdentifier()
val artifact = getRemoteArtifact()

return Package(
id = id,
purl = locateExternalReference(SpdxExternalReference.Type.Purl) ?: id.toPurl(),
cpe = locateCpe(),
authors = originator.wrapPresentInSortedSet(),
declaredLicenses = sortedSetOf(licenseDeclared),
concludedLicense = getConcludedLicense(),
description = packageDescription,
homepageUrl = homepage.mapNotPresentToEmpty(),
binaryArtifact = artifact.takeIf { isBinaryArtifact }.orEmpty(),
sourceArtifact = artifact.takeUnless { isBinaryArtifact }.orEmpty(),
vcs = vcs
return CuratedPackage(
pkg = Package(
id = id,
purl = locateExternalReference(SpdxExternalReference.Type.Purl) ?: id.toPurl(),
cpe = locateCpe(),
authors = originator.wrapPresentInSortedSet(),
declaredLicenses = sortedSetOf(licenseDeclared),
description = packageDescription,
homepageUrl = homepage.mapNotPresentToEmpty(),
binaryArtifact = artifact.takeIf { isBinaryArtifact }.orEmpty(),
sourceArtifact = artifact.takeUnless { isBinaryArtifact }.orEmpty(),
vcs = vcs
),
concludedLicense = getConcludedLicense()
)
}

Expand All @@ -333,7 +336,7 @@ class SpdxDocumentFile(
private fun getDependencies(
pkgId: String,
doc: SpdxResolvedDocument,
packages: MutableSet<Package>
packages: MutableSet<CuratedPackage>
): SortedSet<PackageReference> =
getDependencies(pkgId, doc, packages, SpdxRelationship.Type.DEPENDENCY_OF) { target ->
val issues = mutableListOf<OrtIssue>()
Expand Down Expand Up @@ -386,7 +389,7 @@ class SpdxDocumentFile(
private fun getDependencies(
pkgId: String,
doc: SpdxResolvedDocument,
packages: MutableSet<Package>,
packages: MutableSet<CuratedPackage>,
dependencyOfRelation: SpdxRelationship.Type,
dependsOnCase: (String) -> PackageReference? = { null }
): SortedSet<PackageReference> =
Expand Down Expand Up @@ -451,7 +454,7 @@ class SpdxDocumentFile(
spdxDocument: SpdxResolvedDocument,
projectPackageId: String,
relation: SpdxRelationship.Type,
packages: MutableSet<Package>
packages: MutableSet<CuratedPackage>
): Scope? =
getDependencies(projectPackageId, spdxDocument, packages, relation).takeUnless {
it.isEmpty()
Expand Down Expand Up @@ -486,7 +489,7 @@ class SpdxDocumentFile(

val spdxDocument = transitiveDocument.rootDocument.document

val packages = mutableSetOf<Package>()
val packages = mutableSetOf<CuratedPackage>()
val scopes = sortedSetOf<Scope>()

val projectPackage = if (!spdxDocument.isProject()) {
Expand Down Expand Up @@ -522,7 +525,13 @@ class SpdxDocumentFile(
scopeDependencies = scopes
)

return listOf(ProjectAnalyzerResult(project, packages.toSortedSet()))
return listOf(
ProjectAnalyzerResult(
project = project,
// TODO: How to handle concluded licenses from an SPDX package?
packages = packages.mapTo(sortedSetOf()) { it.pkg }
)
)
}

/**
Expand Down
15 changes: 7 additions & 8 deletions evaluator/src/test/kotlin/DependencyRuleTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,17 @@ import io.kotest.core.spec.style.WordSpec
import io.kotest.matchers.shouldBe

import org.ossreviewtoolkit.model.CuratedPackage
import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.model.PackageReference

class DependencyRuleTest : WordSpec() {
private val ruleSet = ruleSet(ortResult)

private fun createRule(pkg: Package, dependency: PackageReference) =
private fun createRule(pkg: CuratedPackage, dependency: PackageReference) =
DependencyRule(
ruleSet = ruleSet,
name = "test",
pkg = CuratedPackage(pkg),
resolvedLicenseInfo = ruleSet.licenseInfoResolver.resolveLicenseInfo(pkg.id),
pkg = pkg,
resolvedLicenseInfo = ruleSet.licenseInfoResolver.resolveLicenseInfo(pkg.pkg.id),
dependency = dependency,
ancestors = emptyList(),
level = 0,
Expand All @@ -45,14 +44,14 @@ class DependencyRuleTest : WordSpec() {
init {
"isAtTreeLevel()" should {
"return true if the dependency is at the expected tree level" {
val rule = createRule(packageWithoutLicense, packageWithoutLicense.toReference())
val rule = createRule(packageWithoutLicense, packageWithoutLicense.pkg.toReference())
val matcher = rule.isAtTreeLevel(0)

matcher.matches() shouldBe true
}

"return false if the dependency is not at the expected tree level" {
val rule = createRule(packageWithoutLicense, packageWithoutLicense.toReference())
val rule = createRule(packageWithoutLicense, packageWithoutLicense.pkg.toReference())
val matcher = rule.isAtTreeLevel(1)

matcher.matches() shouldBe false
Expand All @@ -61,14 +60,14 @@ class DependencyRuleTest : WordSpec() {

"isProjectFromOrg()" should {
"return true if the project is from org" {
val rule = createRule(packageWithoutLicense, packageWithoutLicense.toReference())
val rule = createRule(packageWithoutLicense, packageWithoutLicense.pkg.toReference())
val matcher = rule.isProjectFromOrg("ossreviewtoolkit")

matcher.matches() shouldBe true
}

"return false if the project is not from org" {
val rule = createRule(packageWithoutLicense, packageWithoutLicense.toReference())
val rule = createRule(packageWithoutLicense, packageWithoutLicense.pkg.toReference())
val matcher = rule.isProjectFromOrg("unknown")

matcher.matches() shouldBe false
Expand Down
10 changes: 5 additions & 5 deletions evaluator/src/test/kotlin/PackageRuleTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ import org.ossreviewtoolkit.utils.spdx.SpdxSingleLicenseExpression
class PackageRuleTest : WordSpec() {
private val ruleSet = ruleSet(ortResult)

private fun createPackageRule(pkg: Package) =
private fun createPackageRule(pkg: CuratedPackage) =
PackageRule(
ruleSet = ruleSet,
name = "test",
pkg = CuratedPackage(pkg),
resolvedLicenseInfo = ruleSet.licenseInfoResolver.resolveLicenseInfo(pkg.id)
pkg = pkg,
resolvedLicenseInfo = ruleSet.licenseInfoResolver.resolveLicenseInfo(pkg.pkg.id)
)

private fun PackageRule.createLicenseRule(license: SpdxSingleLicenseExpression, licenseSource: LicenseSource) =
Expand Down Expand Up @@ -91,7 +91,7 @@ class PackageRuleTest : WordSpec() {
}

"return false for non-existing packages" {
val rule = createPackageRule(Package.EMPTY)
val rule = createPackageRule(CuratedPackage(Package.EMPTY))
val matcher = rule.hasLicense()

matcher.matches() shouldBe false
Expand Down Expand Up @@ -148,7 +148,7 @@ class PackageRuleTest : WordSpec() {

"isProject()" should {
"return true for a project" {
val rule = createPackageRule(projectIncluded.toPackage())
val rule = createPackageRule(CuratedPackage(projectIncluded.toPackage()))
val matcher = rule.isProject()

matcher.matches() shouldBe true
Expand Down
94 changes: 51 additions & 43 deletions evaluator/src/test/kotlin/TestData.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,65 +68,73 @@ val concludedLicense = "LicenseRef-a OR LicenseRef-b OR LicenseRef-c or LicenseR
val declaredLicenses = sortedSetOf("Apache-2.0", "MIT")
val declaredLicensesProcessed = DeclaredLicenseProcessor.process(declaredLicenses)

val packageExcluded = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-excluded:1.0")
val packageExcluded = CuratedPackage(
pkg = Package.EMPTY.copy(id = Identifier("Maven:org.ossreviewtoolkit:package-excluded:1.0"))
)

val packageDynamicallyLinked = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-dynamically-linked:1.0")
val packageDynamicallyLinked = CuratedPackage(
pkg = Package.EMPTY.copy(id = Identifier("Maven:org.ossreviewtoolkit:package-dynamically-linked:1.0"))
)

val packageStaticallyLinked = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-statically-linked:1.0")
val packageStaticallyLinked = CuratedPackage(
pkg = Package.EMPTY.copy(id = Identifier("Maven:org.ossreviewtoolkit:package-statically-linked:1.0"))
)

val packageWithoutLicense = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-without-license:1.0")
val packageWithoutLicense = CuratedPackage(
pkg = Package.EMPTY.copy(id = Identifier("Maven:org.ossreviewtoolkit:package-without-license:1.0"))
)

val packageWithNotPresentLicense = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-with-not-present-license:1.0"),
val packageWithNotPresentLicense = CuratedPackage(
pkg = Package.EMPTY.copy(id = Identifier("Maven:org.ossreviewtoolkit:package-with-not-present-license:1.0")),
concludedLicense = "${SpdxConstants.NONE} OR ${SpdxConstants.NOASSERTION}".toSpdx()
)

val packageWithOnlyConcludedLicense = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-with-only-concluded-license:1.0"),
val packageWithOnlyConcludedLicense = CuratedPackage(
pkg = Package.EMPTY.copy(id = Identifier("Maven:org.ossreviewtoolkit:package-with-only-concluded-license:1.0")),
concludedLicense = concludedLicense
)

val packageWithOnlyDeclaredLicense = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-with-only-declared-license:1.0"),
declaredLicenses = declaredLicenses,
declaredLicensesProcessed = declaredLicensesProcessed
val packageWithOnlyDeclaredLicense = CuratedPackage(
pkg = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-with-only-declared-license:1.0"),
declaredLicenses = declaredLicenses,
declaredLicensesProcessed = declaredLicensesProcessed
)
)

val packageWithOnlyDetectedLicense = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-with-only-detected-license:1.0")
val packageWithOnlyDetectedLicense = CuratedPackage(
pkg = Package.EMPTY.copy(id = Identifier("Maven:org.ossreviewtoolkit:package-with-only-detected-license:1.0"))
// Detected license for this package is added in the ortResult.
)

val packageWithConcludedAndDeclaredLicense = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-with-concluded-and-declared-license:1.0"),
concludedLicense = concludedLicense,
declaredLicenses = declaredLicenses,
declaredLicensesProcessed = declaredLicensesProcessed
val packageWithConcludedAndDeclaredLicense = CuratedPackage(
pkg = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-with-concluded-and-declared-license:1.0"),
declaredLicenses = declaredLicenses,
declaredLicensesProcessed = declaredLicensesProcessed
),
concludedLicense = concludedLicense
)

val packageWithVulnerabilities = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-with-vulnerabilities:1.0")
val packageWithVulnerabilities = CuratedPackage(
pkg = Package.EMPTY.copy(id = Identifier("Maven:org.ossreviewtoolkit:package-with-vulnerabilities:1.0"))
)

val packageMetaDataOnly = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-metadata-only:1.0"),
isMetaDataOnly = true
val packageMetaDataOnly = CuratedPackage(
pkg = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:package-metadata-only:1.0"),
isMetaDataOnly = true
)
)

val packageDependency = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:common-lib:1.0"),
declaredLicenses = declaredLicenses
val packageDependency = CuratedPackage(
pkg = Package.EMPTY.copy(
id = Identifier("Maven:org.ossreviewtoolkit:common-lib:1.0"),
declaredLicenses = declaredLicenses
)
)

val allPackages = listOf(
val allPackages = sortedSetOf(
packageExcluded,
packageDynamicallyLinked,
packageStaticallyLinked,
Expand All @@ -142,7 +150,7 @@ val allPackages = listOf(
val scopeExcluded = Scope(
name = "compile",
dependencies = sortedSetOf(
packageExcluded.toReference()
packageExcluded.pkg.toReference()
)
)

Expand All @@ -152,20 +160,20 @@ val projectExcluded = Project.EMPTY.copy(
scopeDependencies = sortedSetOf(scopeExcluded)
)

val packageRefDynamicallyLinked = packageDynamicallyLinked.toReference(PackageLinkage.DYNAMIC)
val packageRefStaticallyLinked = packageStaticallyLinked.toReference(PackageLinkage.STATIC)
val packageRefDynamicallyLinked = packageDynamicallyLinked.pkg.toReference(PackageLinkage.DYNAMIC)
val packageRefStaticallyLinked = packageStaticallyLinked.pkg.toReference(PackageLinkage.STATIC)

val scopeIncluded = Scope(
name = "compile",
dependencies = sortedSetOf(
packageWithoutLicense.toReference(),
packageWithNotPresentLicense.toReference(),
packageWithOnlyConcludedLicense.toReference(),
packageWithOnlyDeclaredLicense.toReference(dependencies = sortedSetOf(packageDependency.toReference())),
packageWithConcludedAndDeclaredLicense.toReference(),
packageWithoutLicense.pkg.toReference(),
packageWithNotPresentLicense.pkg.toReference(),
packageWithOnlyConcludedLicense.pkg.toReference(),
packageWithOnlyDeclaredLicense.pkg.toReference(dependencies = sortedSetOf(packageDependency.pkg.toReference())),
packageWithConcludedAndDeclaredLicense.pkg.toReference(),
packageRefDynamicallyLinked,
packageRefStaticallyLinked,
packageMetaDataOnly.toReference()
packageMetaDataOnly.pkg.toReference()
)
)

Expand Down Expand Up @@ -219,7 +227,7 @@ val ortResult = OrtResult(
projectExcluded,
projectIncluded
),
packages = allPackages.mapTo(sortedSetOf()) { CuratedPackage(it) }
packages = allPackages
)
),
advisor = AdvisorRun(
Expand All @@ -229,7 +237,7 @@ val ortResult = OrtResult(
config = AdvisorConfiguration(),
results = AdvisorRecord(
advisorResults = sortedMapOf(
packageWithVulnerabilities.id to listOf(
packageWithVulnerabilities.pkg.id to listOf(
AdvisorResult(
advisor = AdvisorDetails.EMPTY,
summary = AdvisorSummary(startTime = Instant.EPOCH, endTime = Instant.EPOCH),
Expand Down
1 change: 0 additions & 1 deletion helper-cli/src/main/kotlin/commands/CreateAnalyzerResultCommand.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ private fun ScannedPackage.toPackage(): Package {
return Package(
id = id,
declaredLicenses = sortedSetOf(),
concludedLicense = null,
description = "",
homepageUrl = "",
binaryArtifact = RemoteArtifact.EMPTY,
Expand Down
16 changes: 9 additions & 7 deletions model/src/main/kotlin/CuratedPackage.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.ossreviewtoolkit.model

import com.fasterxml.jackson.annotation.JsonIgnore
import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.annotation.JsonProperty

import org.ossreviewtoolkit.utils.ort.DeclaredLicenseProcessor
Expand All @@ -36,6 +37,13 @@ data class CuratedPackage(
@JsonProperty("package")
val pkg: Package,

/**
* The concluded license as an [SpdxExpression]. It can be used to override the [declared][declaredLicenses] /
* [detected][LicenseFinding.license] licenses of a package.
*/
@JsonInclude(JsonInclude.Include.NON_NULL)
val concludedLicense: SpdxExpression? = null,

/**
* The curations in the order they were applied.
*/
Expand All @@ -54,13 +62,7 @@ data class CuratedPackage(
curation.base.apply(current)
}.pkg.copy(
// The declared license mapping cannot be reversed as it is additive.
declaredLicensesProcessed = DeclaredLicenseProcessor.process(pkg.declaredLicenses),

// It is not possible to derive the original concluded license value with the above reversed application
// of the base curations, even if the function Package.diff() was extended to handle the concluded license,
// see also https://github.com/oss-review-toolkit/ort/issues/5637. Until this is fixed just set the
// concluded license to null, because an un-curated package cannot have a non-null concluded license.
concludedLicense = null
declaredLicensesProcessed = DeclaredLicenseProcessor.process(pkg.declaredLicenses)
)

@JsonIgnore
Expand Down
Loading

0 comments on commit c9542b8

Please sign in to comment.