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

Add functionality to handle license choices #3396

Closed
MarcelBochtler opened this issue Nov 30, 2020 · 7 comments · Fixed by #3801
Closed

Add functionality to handle license choices #3396

MarcelBochtler opened this issue Nov 30, 2020 · 7 comments · Fixed by #3801
Assignees

Comments

@MarcelBochtler
Copy link
Member

It should be possible to select a license if a package is dual licensed.
This should be possible to do ORT instance globally (~/.ort/config/licenseChoices.yml) and project specific (.ort.yml).
Project specific configurations should override global configrations.

The logic how to select and replace SPDX OR clauses are already defined in these tests:

"disjunctiveNormalForm()" should {
"not change an expression already in DNF" {
val spdxExpression = "(a AND b) OR (c AND d)".toSpdx()
spdxExpression.disjunctiveNormalForm() shouldBe spdxExpression
}
"correctly convert an OR on the left side of an AND expression" {
"(a OR b) AND c".toSpdx().disjunctiveNormalForm() shouldBe "(a AND c) OR (b AND c)".toSpdx()
}
"correctly convert an OR on the right side of an AND expression" {
"a AND (b OR c)".toSpdx().disjunctiveNormalForm() shouldBe "(a AND b) OR (a AND c)".toSpdx()
}
"correctly convert ORs on both sides of an AND expression" {
"(a OR b) AND (c OR d)".toSpdx().disjunctiveNormalForm() shouldBe
"(a AND c) OR (a AND d) OR (b AND c) OR (b AND d)".toSpdx()
}
"correctly convert a complex expression" {
"(a OR b) AND c AND (d OR e)".toSpdx().disjunctiveNormalForm() shouldBe
"(a AND c AND d) OR (a AND c AND e) OR (b AND c AND d) OR (b AND c AND e)".toSpdx()
}
}
"validChoices()" should {
"list the valid choices for a complex expression" {
"(a OR b) AND c AND (d OR e)".toSpdx().validChoices() should containExactlyInAnyOrder(
"a AND c AND d".toSpdx(),
"a AND c AND e".toSpdx(),
"b AND c AND d".toSpdx(),
"b AND c AND e".toSpdx()
)
}
}
"offersChoice()" should {
"return true if the expression contains the OR operator" {
"a OR b".toSpdx().offersChoice() shouldBe true
"a AND b OR c".toSpdx().offersChoice() shouldBe true
"a OR b AND c".toSpdx().offersChoice() shouldBe true
"a AND b AND c OR d".toSpdx().offersChoice() shouldBe true
}
"return false if the expression does not contain the OR operator" {
"a".toSpdx().offersChoice() shouldBe false
"a AND b".toSpdx().offersChoice() shouldBe false
"a AND b AND c".toSpdx().offersChoice() shouldBe false
}
}
"isValidChoice()" should {
"return true if a choice is valid" {
val spdxExpression = "(a OR b) AND c AND (d OR e)".toSpdx()
spdxExpression.isValidChoice("a AND c AND d".toSpdx()) shouldBe true
spdxExpression.isValidChoice("a AND d AND c".toSpdx()) shouldBe true
spdxExpression.isValidChoice("c AND a AND d".toSpdx()) shouldBe true
spdxExpression.isValidChoice("c AND d AND a".toSpdx()) shouldBe true
spdxExpression.isValidChoice("d AND a AND c".toSpdx()) shouldBe true
spdxExpression.isValidChoice("d AND c AND a".toSpdx()) shouldBe true
spdxExpression.isValidChoice("a AND c AND e".toSpdx()) shouldBe true
spdxExpression.isValidChoice("b AND c AND d".toSpdx()) shouldBe true
spdxExpression.isValidChoice("b AND c AND e".toSpdx()) shouldBe true
}
"return false if a choice is invalid" {
val spdxExpression = "(a OR b) AND c AND (d OR e)".toSpdx()
spdxExpression.isValidChoice("a".toSpdx()) shouldBe false
spdxExpression.isValidChoice("a AND b".toSpdx()) shouldBe false
spdxExpression.isValidChoice("a AND c".toSpdx()) shouldBe false
spdxExpression.isValidChoice("a AND d".toSpdx()) shouldBe false
spdxExpression.isValidChoice("a AND e".toSpdx()) shouldBe false
spdxExpression.isValidChoice("a AND b AND c".toSpdx()) shouldBe false
spdxExpression.isValidChoice("a AND b AND d".toSpdx()) shouldBe false
spdxExpression.isValidChoice("a AND b AND c AND d".toSpdx()) shouldBe false
}
}

@MarcelBochtler MarcelBochtler self-assigned this Nov 30, 2020
@sschuberth
Copy link
Member

Just a remark, such a file should be called license-choices.yml (no camel-case but a dash), and we should probably also give it a though whether we could add license choices to the existing license-classifications.yml instead of adding yet another configuration file.

@MarcelBochtler MarcelBochtler changed the title Add functionality to handle dual licensed packages Add functionality to handle license choices Dec 2, 2020
@mnonnenmacher
Copy link
Member

Copied comment from Slack which contains info relevant for this topic:

I would separate the discussions about (1) for which scope license choices should be configurable and (2) how it should be implemented.

For (1) I believe we need (a) global license choices (e.g. in license-choices.yml), something like "Apache-2.0 OR MIT" always chose "Apache-2.0". The question is, should this also be applied to supexpressions automatically? (b) Per package license choices, because for some packages we might want to make a different choice. This should override global choices. This could be configured in curations.yml. (c) Per project license choices, because two different projects might want to make a different choice for the same library. This should be configurable in ort.yml and override (a) and (b). And maybe in ort.yml we also need to support global and per package choices to support all possible use cases.

If we really want to implement all these options depends on how important we think the different use-cases are.

(2) The logic that applies the license choice should be implemented in LicenseInfoResolver. The problem here is that the ResolvedLicenseInfo for a package does currently not provide an SPDX expression but simply a list of individual licenses as ResolvedLicense objects.

(a) The first step would be to find out if there is a license choice to be able to provide a resolved license expression that uses the correct AND and OR operators in ResolvedLicense. Because if the model does not provide the information if there is a license choice, it does not make sense to implement a way to configure license choices. For this we can check if the declared license contains a license choice, and we could implement support for license choices detected by ScanCode. At least I was told that ScanCode would provide such information in the latest versions, but I have never verified that myself. Otherwise we have to assume that all detected licenses are concatenated by AND.

(b) If we have solved (a) the implementation in LicenseInfoResolver should follow the principle to be completely transparent, that means the API must allow to check what the original SPDX expression was, which choices were applied, and what the resulting SPDX expression is.

@MarcelBochtler
Copy link
Member Author

MarcelBochtler commented Jan 18, 2021

The question is, should this also be applied to supexpressions automatically?

@mnonnenmacher It was my understanding that it should be applied to subexpressions automatically. This was also covered in this test:
https://github.com/oss-review-toolkit/ort/blob/master/spdx-utils/src/test/kotlin/SpdxExpressionTest.kt#L388-L395

@MarcelBochtler
Copy link
Member Author

MarcelBochtler commented Jan 18, 2021

At least I was told that ScanCode would provide such information in the latest versions, but I have never verified that myself.

ScanCode 3.2.3 result for a file from h2database which is licensed under MPL-2.0 OR EPL-2.0:

          "matched_rule": {
            "identifier": "mpl-2.0_or_epl-1.0_and_proprietary-license_2.RULE",
            "license_expression": "(mpl-2.0 OR epl-1.0) AND proprietary-license",
            "licenses": [
              "mpl-2.0",
              "epl-1.0",
              "proprietary-license"
            ],
            "is_license_text": false,
            "is_license_notice": true,
            "is_license_reference": false,
            "is_license_tag": false,
            "matcher": "3-seq",
            "rule_length": 54,
            "matched_length": 11,
            "match_coverage": 20.37,
            "rule_relevance": 100
          }

Edit: Same result with the already used ScanCode 3.2.1rc2
Edit 2: Created an issue to implement the parsing of license choices from ScanCode results: #3523

@MarcelBochtler
Copy link
Member Author

For the already discussed scope: Project specific for a specific package.

We have multiple possibilities to design the configuration model for the .ort.yml:

(1) This design allows the user to set the absolute desired result license.

- id: package-id
  given: (A OR B) AND C
  choice: A AND C

(2) This design allows the user to set a choice with an optional sub-expression. The desired result license would be computed. Several choices can be given.

- id: `package-id`
  given: (A OR B) AND C
  apply-choice:
  - choice: A
    sub-expression?: A OR B

(3) This design allows the user to EITHER set the desired result license or to set optional sub-expressions with corresponding choices or both.

- id: package-id
  given: (A OR B) AND C
  choice?: A AND C
  sub-expressions?:
  - expression: A OR B
    choice: A

Design (1) would not be possible when applying the license choice without a specific package and would therefore result in a different configuration model.
Design (2) and (3) allow the user to simply choose their desired sub-expressions and their choices without having to calculate the absolute resulting license themselves.

@fviernau, @mnonnenmacher, @sschuberth
What is your preferred solution?

@neubs-bsi
Copy link
Contributor

We looked at our use case again. When different given are provided (e.g. detected has two license findings for the same package with different OR expressions) we need to be able to handle that. This is not covered by any of the designs (1) to (3). A possible solutions would be:

(4) The design allows for one package identifier to get several choices that can have sub-expressions. These choices can be applied to declared, detected and concluded, which can have different given license expressions. This design also allows to choose an "absolute" license for a given license expression, as can be seen in the second given choice in the example.

- id: package-id
  choices: 
  - given: (A OR B) AND C
    choice: A
    sub-expressions?: A OR B
  - given: (A OR B) AND C
    choice: A AND C
  - given: (D OR F)
    choice: D

What do you think?
@fviernau @sschuberth @mnonnenmacher

MarcelBochtler added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 3, 2021
Apply license choices specified for a package defined in the repository
configuration file.
The choice reflects in the ResolvedLicense where only the chosen
licenses get added to the final list of resolved licenses.
The information of the non-chosen license is not lost as it is still
present in the `originalLicenseExpressions`.

Relates to oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <stephanie.neubauer@bosch.io>
Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.io>
MarcelBochtler added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 3, 2021
Apply license choices specified for a package defined in the repository
configuration file.
The choice reflects in the ResolvedLicense where only the chosen
licenses get added to the final list of resolved licenses.
The information of the non-chosen license is not lost as it is still
present in the `originalLicenseExpressions`.

Relates to oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <stephanie.neubauer@bosch.io>
Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.io>
MarcelBochtler added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 3, 2021
Apply license choices specified for a package defined in the repository
configuration file.
The choice reflects in the ResolvedLicense where only the chosen
licenses get added to the final list of resolved licenses.
The information of the non-chosen license is not lost as it is still
present in the `originalLicenseExpressions`.

Relates to oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <stephanie.neubauer@bosch.io>
Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.io>
MarcelBochtler added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 3, 2021
Apply license choices specified for a package defined in the repository
configuration file.
The choice reflects in the ResolvedLicense where only the chosen
licenses get added to the final list of resolved licenses.
The information of the non-chosen license is not lost as it is still
present in the `originalLicenseExpressions`.

Relates to oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <stephanie.neubauer@bosch.io>
Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.io>
MarcelBochtler added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 3, 2021
Apply license choices specified for a package defined in the repository
configuration file.
The choice reflects in the ResolvedLicense where only the chosen
licenses get added to the final list of resolved licenses.
The information of the non-chosen license is not lost as it is still
present in the `originalLicenseExpressions`.

Relates to oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <stephanie.neubauer@bosch.io>
Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.io>
@MarcelBochtler
Copy link
Member Author

MarcelBochtler commented Mar 3, 2021

Opened a PR for the suggestion by @neubs-bsi

MarcelBochtler added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 4, 2021
Apply license choices specified for a package defined in the repository
configuration file.
The choice reflects in the ResolvedLicense where only the chosen
licenses get added to the final list of resolved licenses.
The information of the non-chosen license is not lost as it is still
present in the `originalLicenseExpressions`.

Relates to oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <stephanie.neubauer@bosch.io>
Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.io>
MarcelBochtler added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 4, 2021
Apply license choices specified for a package defined in the repository
configuration file.
The choice reflects in the ResolvedLicense where only the chosen
licenses get added to the final list of resolved licenses.
The information of the non-chosen license is not lost as it is still
present in the `originalLicenseExpressions`.

Relates to oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <stephanie.neubauer@bosch.io>
Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.io>
MarcelBochtler added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 9, 2021
Apply license choices specified for a package defined in the repository
configuration file.
The choice reflects in the ResolvedLicense where only the chosen
licenses get added to the final list of resolved licenses.
The information of the non-chosen license is not lost as it is still
present in the `originalLicenseExpressions`.

Relates to oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <stephanie.neubauer@bosch.io>
Signed-off-by: Marcel Bochtler <marcel.bochtler@bosch.io>
neubs-bsi added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 23, 2021
Resolves oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <Stephanie.Neubauer@bosch.io>
neubs-bsi added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 23, 2021
Resolves oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <Stephanie.Neubauer@bosch.io>
neubs-bsi added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 23, 2021
Resolves oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <Stephanie.Neubauer@bosch.io>
neubs-bsi added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 26, 2021
Resolves oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <Stephanie.Neubauer@bosch.io>
neubs-bsi added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 26, 2021
Resolves oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <Stephanie.Neubauer@bosch.io>
neubs-bsi added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 26, 2021
Resolves oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <Stephanie.Neubauer@bosch.io>
neubs-bsi added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 29, 2021
Resolves oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <Stephanie.Neubauer@bosch.io>
neubs-bsi added a commit to boschglobal/oss-review-toolkit that referenced this issue Mar 29, 2021
Resolves oss-review-toolkit#3396.

Signed-off-by: Stephanie Neubauer <Stephanie.Neubauer@bosch.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants