Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply license choices specified for a package defined in the repository configuration file #3713

Merged
merged 8 commits into from
Mar 22, 2021
54 changes: 54 additions & 0 deletions docs/config-file-ort-yml.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,3 +260,57 @@ resolutions:
reason: "INEFFECTIVE_VULNERABILITY"
comment: "CVE-9999-9999 is a false positive"
```

# License Choices

### When to Use License Choices

For multi-licensed dependencies a specific license can be selected.
The license choice can be applied to a package.
A choice is only valid for licenses combined with the SPDX operator `OR`.
The choices are applied in the evaluator, and the reporter to the effective license of a package, which is calculated
by the chosen [LicenseView](../model/src/main/kotlin/licenses/LicenseView.kt).

### License Choice by Package

To select a license from a multi-licensed dependency, specified by its `packageId`, an SPDX expression for a `choice`
must be provided.
The `choice` is either applied to the whole effective SPDX expression of the package or to an optional `given` SPDX
expression that can represent only a sub-expression of the whole effective SPDX expression.

e.g.
```yaml
license_choices:
package_license_choice:
- package_id: "Maven:com.example:first:0.0.1"
license_choices:
# The input of the calculated effective license would be: (A OR B) AND ((C OR D) AND E)
- given: A OR B
choice: A
# The result would be: A AND ((C OR D) AND E)
# The input of the current effective license would be: A AND ((C OR D) AND E)
- given: (C OR D) AND E
choice: C AND E
# The result would be: A AND C AND E
- package_id: "Maven:com.example:second:2.3.4"
license_choices:
# Without a 'given', the 'choice' is applied to the effective license expression if it is a valid choice.
# The input from the calculated effective license would be: (C OR D) AND E
- choice: C AND E
# The result would be: C AND E
```

---
**NOTE**

The choice will be applied to the WHOLE `given` license.
If the choice does not provide a valid result, an exception will be thrown upon deserialization.

e.g. invalid configuration:
```yaml
# This is invalid, as 'E' must be in the resulting license.
- given: (C OR D) AND E
choice: C
```

---
3 changes: 2 additions & 1 deletion evaluator/src/main/kotlin/PackageRule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ open class PackageRule(
* A DSL function to configure a [LicenseRule] and add it to this rule.
*/
fun licenseRule(name: String, licenseView: LicenseView, block: LicenseRule.() -> Unit) {
resolvedLicenseInfo.filter(licenseView, filterSources = true).forEach { resolvedLicense ->
resolvedLicenseInfo.filter(licenseView, filterSources = true)
.applyChoices(ruleSet.ortResult.getLicenseChoices(pkg.id)).forEach { resolvedLicense ->
resolvedLicense.sources.forEach { licenseSource ->
licenseRules += LicenseRule(name, resolvedLicense, licenseSource).apply(block)
}
Expand Down
21 changes: 21 additions & 0 deletions evaluator/src/test/kotlin/RuleSetTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import io.kotest.matchers.collections.haveSize
import io.kotest.matchers.should

import org.ossreviewtoolkit.model.licenses.LicenseView
import org.ossreviewtoolkit.spdx.toSpdx

class RuleSetTest : WordSpec() {
private val errorMessage = "error message"
Expand Down Expand Up @@ -120,6 +121,26 @@ class RuleSetTest : WordSpec() {

ruleSet.violations should haveSize(4)
}

"add no license errors if license is removed by license choice" {
val ruleSet = ruleSet(ortResult) {
dependencyRule("test") {
licenseRule("test", LicenseView.CONCLUDED_OR_DECLARED_AND_DETECTED) {
require {
object : RuleMatcher {
override val description = "containsLicense(license)"

override fun matches() = license == "LicenseRef-b".toSpdx()
}
}

error(errorMessage, howToFix)
}
}
}

ruleSet.violations should haveSize(5)
}
}
}
}
15 changes: 14 additions & 1 deletion evaluator/src/test/kotlin/TestData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,18 @@ import org.ossreviewtoolkit.model.TextLocation
import org.ossreviewtoolkit.model.VcsInfo
import org.ossreviewtoolkit.model.config.AnalyzerConfiguration
import org.ossreviewtoolkit.model.config.Excludes
import org.ossreviewtoolkit.model.config.LicenseChoices
import org.ossreviewtoolkit.model.config.PackageLicenseChoice
import org.ossreviewtoolkit.model.config.PathExclude
import org.ossreviewtoolkit.model.config.PathExcludeReason
import org.ossreviewtoolkit.model.config.RepositoryConfiguration
import org.ossreviewtoolkit.model.config.ScannerConfiguration
import org.ossreviewtoolkit.spdx.model.LicenseChoice
import org.ossreviewtoolkit.spdx.toSpdx
import org.ossreviewtoolkit.utils.DeclaredLicenseProcessor
import org.ossreviewtoolkit.utils.Environment

val concludedLicense = "LicenseRef-a AND LicenseRef-b".toSpdx()
val concludedLicense = "LicenseRef-a OR LicenseRef-b".toSpdx()
val declaredLicenses = sortedSetOf("Apache-2.0", "MIT")
val declaredLicensesProcessed = DeclaredLicenseProcessor.process(declaredLicenses)

Expand Down Expand Up @@ -162,6 +165,16 @@ val ortResult = OrtResult(
comment = "excluded"
)
)
),
licenseChoices = LicenseChoices(
packageLicenseChoices = listOf(
PackageLicenseChoice(
packageId = Identifier("Maven:org.ossreviewtoolkit:package-with-only-concluded-license:1.0"),
licenseChoices = listOf(
LicenseChoice("LicenseRef-a OR LicenseRef-b".toSpdx(), "LicenseRef-a".toSpdx())
)
)
)
)
)
),
Expand Down
2 changes: 2 additions & 0 deletions model/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ val hopliteVersion: String by project
val jacksonVersion: String by project
val postgresEmbeddedVersion: String by project
val postgresVersion: String by project
val mockkVersion: String by project
val semverVersion: String by project

plugins {
Expand Down Expand Up @@ -54,4 +55,5 @@ dependencies {
implementation("org.postgresql:postgresql:$postgresVersion")

testImplementation("com.opentable.components:otj-pg-embedded:$postgresEmbeddedVersion")
testImplementation("io.mockk:mockk:$mockkVersion")
}
7 changes: 7 additions & 0 deletions model/src/main/kotlin/OrtResult.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import org.ossreviewtoolkit.model.config.LicenseFindingCuration
import org.ossreviewtoolkit.model.config.RepositoryConfiguration
import org.ossreviewtoolkit.model.config.Resolutions
import org.ossreviewtoolkit.model.config.orEmpty
import org.ossreviewtoolkit.spdx.model.LicenseChoice
import org.ossreviewtoolkit.utils.log
import org.ossreviewtoolkit.utils.perf
import org.ossreviewtoolkit.utils.zipWithDefault
Expand Down Expand Up @@ -385,6 +386,12 @@ data class OrtResult(
!omitExcluded || !isExcluded(id)
}

/**
* Return all [LicenseChoice]s for the [Package] with [id].
*/
fun getLicenseChoices(id: Identifier): List<LicenseChoice> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed that in future we'll have license choices also defined elsewhere, so the license choices do not only come from the OrtResult. This is why I looked up how path excludes and license finding curations are retrieved.

I found that they get injected into DefaultLicenseInfoProvider to be used by the LicenseInfoResolver.
Have you considered processing license choices in a similar / analog way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be ok for you to refactor this when other license choice configurations are added?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd actually like to also have @mnonnenmacher opinion on that as he has written the parts relevant here before diving deeper into this. I fear that it could be quite a bit of tech debt, but let's see.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, the design idea is that the LicenseInfoResolver gets all required information from the LicenseInfo and therefore it would make sense to also add the choices there. But I would be fine with changing this separately, because I currently cannot decide which would be the best way to integrate the choices there.

repository.config.licenseChoices.packageLicenseChoices.find { it.packageId == id }?.licenseChoices.orEmpty()

/**
* Return the list of [AdvisorResult]s for the given [id].
*/
Expand Down
46 changes: 46 additions & 0 deletions model/src/main/kotlin/config/LicenseChoices.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (C) 2021 Bosch.IO GmbH
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
* License-Filename: LICENSE
*/

package org.ossreviewtoolkit.model.config

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

import org.ossreviewtoolkit.model.Identifier
import org.ossreviewtoolkit.spdx.model.LicenseChoice

/**
* The license choices configured for a repository.
*/
data class LicenseChoices(
@JsonInclude(JsonInclude.Include.NON_EMPTY)
val packageLicenseChoices: List<PackageLicenseChoice> = emptyList()
) {
@JsonIgnore
fun isEmpty() = packageLicenseChoices.isEmpty()
}

/**
* [LicenseChoice]s defined for an artifact.
*/
data class PackageLicenseChoice(
val packageId: Identifier,
@JsonInclude(JsonInclude.Include.NON_EMPTY)
val licenseChoices: List<LicenseChoice> = emptyList()
)
14 changes: 13 additions & 1 deletion model/src/main/kotlin/config/RepositoryConfiguration.kt
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ data class RepositoryConfiguration(
* Defines curations for artifacts contained in this repository.
*/
@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = CurationsFilter::class)
val curations: Curations = Curations()
val curations: Curations = Curations(),

/**
* Defines license choices within this repository.
*/
@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = LicenseChoiceFilter::class)
val licenseChoices: LicenseChoices = LicenseChoices()
)

@Suppress("EqualsOrHashCode", "EqualsWithHashCodeExist") // The class is not supposed to be used with hashing.
Expand All @@ -68,3 +74,9 @@ private class CurationsFilter {
override fun equals(other: Any?): Boolean =
if (other is Curations) other.licenseFindings.isEmpty() else false
}

@Suppress("EqualsOrHashCode", "EqualsWithHashCodeExist") // The class is not supposed to be used with hashing.
private class LicenseChoiceFilter {
override fun equals(other: Any?): Boolean =
MarcelBochtler marked this conversation as resolved.
Show resolved Hide resolved
other is LicenseChoices && other.isEmpty()
}
27 changes: 27 additions & 0 deletions model/src/main/kotlin/licenses/ResolvedLicenseInfo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import org.ossreviewtoolkit.model.config.LicenseFindingCuration
import org.ossreviewtoolkit.model.config.PathExclude
import org.ossreviewtoolkit.spdx.SpdxExpression
import org.ossreviewtoolkit.spdx.SpdxSingleLicenseExpression
import org.ossreviewtoolkit.spdx.model.LicenseChoice
import org.ossreviewtoolkit.utils.CopyrightStatementsProcessor
import org.ossreviewtoolkit.utils.DeclaredLicenseProcessor

Expand Down Expand Up @@ -65,6 +66,23 @@ data class ResolvedLicenseInfo(
) : Iterable<ResolvedLicense> by licenses {
operator fun get(license: SpdxSingleLicenseExpression): ResolvedLicense? = find { it.license == license }

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this resolved license info class has all configuration relevant for computing the needed information in its attributes. So, what's the reason for not adding an attribute for the applicable license choices to the data model?

For ResolvedLicenseLocation there is appliedCuration and matchingPathExcludes there is the curations and path excludes already applied, and additionally there is the properties for what was applied. The analog thing for choices would be that choices are already applied and there is an applied choices property?

Note: I'm haven't myself yet fully understood this area of the code, so I rather want to get the discussion going, but don't intent to say it's wrong as-is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we thought about this.
But the license choice should only be applied to the licenses used in the evaluator and the reporter, and therefore make use of the specified LicenseViews.

This implementation decision was a direct result of a discussion in the developers meeting on 2021-03-04, we developed it in pair programming with @mnonnenmacher.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, the problem is that the license choice can only be applied if the sub-expression matches, and if it does can depend on the used LicenseView. So the available license choices could be provided by the LicenseInfo as mentioned in another comment, but they can only be applied in combination with a LicenseView, for example in the evaluator rules.

* Return the effective [SpdxExpression] of this [ResolvedLicenseInfo] based on their [licenses] filtered by the
* [licenseView] and the applied [licenseChoices]. Effective, in this context, refers to an [SpdxExpression] that
* can be used as a final license of this [ResolvedLicenseInfo].
*/
fun effectiveLicense(licenseView: LicenseView, licenseChoices: List<LicenseChoice> = emptyList()): SpdxExpression? {
val resolvedLicenseInfo = filter(licenseView, filterSources = true)

return resolvedLicenseInfo.licenses.flatMap { it.originalExpressions.values }
.flatten()
.toSet()
.reduceOrNull(SpdxExpression::and)
?.applyChoices(licenseChoices)
?.validChoices()
?.reduceOrNull(SpdxExpression::or)
}

/**
* Return all copyright statements associated to this license info. Copyright findings that are excluded by
* [PathExclude]s are [omitted][omitExcluded] by default. The copyrights are [processed][process] by default
Expand Down Expand Up @@ -96,6 +114,15 @@ data class ResolvedLicenseInfo(
}
}

/**
* Apply [licenseChoices] on the effective license of [LicenseView.ALL].
*/
fun applyChoices(licenseChoices: List<LicenseChoice>): ResolvedLicenseInfo {
val licenses = effectiveLicense(LicenseView.ALL, licenseChoices)?.decompose().orEmpty()

return this.copy(licenses = this.licenses.filter { it.license in licenses })
}

/**
* Filter all excluded licenses and copyrights. Licenses are removed if they are only
* [detected][LicenseSource.DETECTED] and all [locations][ResolvedLicense.locations] have
Expand Down
24 changes: 24 additions & 0 deletions model/src/test/kotlin/config/RepositoryConfigurationTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import io.kotest.matchers.collections.haveSize
import io.kotest.matchers.should
import io.kotest.matchers.shouldBe

import org.ossreviewtoolkit.model.Identifier
import org.ossreviewtoolkit.model.yamlMapper
import org.ossreviewtoolkit.spdx.toSpdx

class RepositoryConfigurationTest : WordSpec({
"RepositoryConfiguration" should {
Expand Down Expand Up @@ -67,6 +69,13 @@ class RepositoryConfigurationTest : WordSpec({
- id: "vulnerability id"
reason: "INEFFECTIVE_VULNERABILITY"
comment: "vulnerability comment"
license_choices:
package_license_choices:
fviernau marked this conversation as resolved.
Show resolved Hide resolved
- package_id: "Maven:com.example:lib:0.0.1"
license_choices:
- given: MPL-2.0 or EPL-1.0
choice: MPL-2.0
- choice: MPL-2.0 AND MIT
""".trimIndent()

val repositoryConfiguration = yamlMapper.readValue<RepositoryConfiguration>(configuration)
Expand Down Expand Up @@ -110,6 +119,21 @@ class RepositoryConfigurationTest : WordSpec({
reason shouldBe VulnerabilityResolutionReason.INEFFECTIVE_VULNERABILITY
comment shouldBe "vulnerability comment"
}

val packageLicenseChoices = repositoryConfiguration.licenseChoices.packageLicenseChoices
packageLicenseChoices should haveSize(1)
with(packageLicenseChoices.first()) {
packageId shouldBe Identifier("Maven:com.example:lib:0.0.1")
with(licenseChoices.first()) {
given shouldBe "MPL-2.0 or EPL-1.0".toSpdx()
choice shouldBe "MPL-2.0".toSpdx()
}

with(licenseChoices[1]) {
given shouldBe null
choice shouldBe "MPL-2.0 AND MIT".toSpdx()
}
}
}
}
})
14 changes: 1 addition & 13 deletions model/src/test/kotlin/licenses/LicenseInfoResolverTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import org.ossreviewtoolkit.model.config.LicenseFindingCuration
import org.ossreviewtoolkit.model.config.LicenseFindingCurationReason
import org.ossreviewtoolkit.model.config.PathExclude
import org.ossreviewtoolkit.model.config.PathExcludeReason
import org.ossreviewtoolkit.model.licenses.TestUtils.containLicensesExactly
import org.ossreviewtoolkit.model.utils.FileArchiver
import org.ossreviewtoolkit.spdx.SpdxExpression
import org.ossreviewtoolkit.spdx.SpdxSingleLicenseExpression
Expand Down Expand Up @@ -763,19 +764,6 @@ fun containLicenseExpressionsExactlyBySource(
)
}

fun containLicensesExactly(vararg licenses: String): Matcher<Iterable<ResolvedLicense>?> =
neverNullMatcher { value ->
val expected = licenses.map { SpdxExpression.parse(it) as SpdxSingleLicenseExpression }.toSet()
val actual = value.map { it.license }.toSet()

MatcherResult(
expected == actual,
"ResolvedLicenseInfo should contain exactly licenses ${expected.show().value}, but has " +
actual.show().value,
"ResolvedLicenseInfo should not contain exactly ${expected.show().value}"
)
}

fun containNumberOfLocationsForLicense(license: String, count: Int): Matcher<ResolvedLicenseInfo?> =
neverNullMatcher { value ->
val actualCount = value[SpdxSingleLicenseExpression.parse(license)]?.locations?.size ?: 0
Expand Down
Loading