Path based license conclusions for projects

fviernau commented 5 years ago

Rule violations triggered by incorrect license detections can currently be addressed as follows

For projects:

create a rule violation resolution

For packages (project dependencies):

create a rule violation resolution
create a concluded license

Problems

Rule violation resolutions:
- Only the error message can be matched which in reasonable cases we can assume to be on package / license granularity rather than on actual location of the findings. Due to the coarse granularity the validity of the resolution cannot be automatically verified and thus it has to be verified manually for each ORT result.
- Due to above should be used only as last resort while incorrect detections are the expected case
License conclusions (for packages):
- Creating a concluded license for a while package requires investigating all license detections and the declared license as well. So instead of just investigating a single incorrect finding one gets dragged into investigation all detection which creates unnecessary manual effort.
- Concluded licenses in the current state can only be re-applied / ported onto scan-result created using a different scanner configuration.

Proposed feature

Below example shows a curation for all license findings of all .cpp and .h files found underneath src in lines 5-18 where CDDL-1.1 was detected. It corrects all the matched findings with CDDL-1.0

curations:
   paths:
    pattern: "/src/**{.cpp|.h}"
    start_line: 5
    line_count: 13
    detected_license: "CDDL-1.1"
    reason: MATCH_INCORRECT
        comment: "the license version actually is 1.0"
        concluded_license: "CDDL-1.0

Spec:

path: a glob matching file paths
start_line: start line matcher of the license finding. Not provided means -1. -1 matches any line.
line_count: matcher for the line count. Not provided means -1. -1 matches any.
license: matcher for the detected license. Use a string for now for simplicity. Not provided matches any
concluded license: SPDX, use none expression: https://spdx.org/spdx-specification-21-web-version#h.jxpfx0ykyb60 see Data Format: | “NONE” | “NOASSERTION”
reason: {MATCH_INCORRECT | CHOICE | NOT_DETECTED | REFERENCE (url, SEE LICENSE) | MENTION (inspired by) | EXAMPLE_OF (code example) | DOCUMENTATION_OF | DATA_FILE_OF}

Advantages

reduces manual effort: less license conclusions needed
corrections can be processed and reflected for copyright notice generation as opposed to rule violation resolutions
automatic verification, if the file changed an the matcher is not to broad it will stop matching. If not changed it still applies. No more manual reverification.
leads to reduction of licenses to be investigate for concluding package licenses as the incorrect findings are already addressed
step towards re-applying the configuration onto scan-results created with a different scanner configuration.

Scope While this would be useful for dependencies as well limit the scope to local configurations aka ort.yml as this has highest priority currently.

fviernau commented 5 years ago

how about using those ort.yml properties:

curations:
   license_findings:

..and call it license finding curation

moreover, how about using list as type for start lines?

fviernau commented 5 years ago

My attempts to implemented this resulted in one bigger and one small potentiall refactoring topic:

The association between license findings and copyright statements currently made in scanCode and then lost as not represented in the model (ScanSummary.kt)
Whether the application of the curations results in a new LicenseView e.g. which would satisfy a refactoring to generalize the implementation (which imo would already make sense anyway due to redundancy, as indicator just see the almost identical test cases which are heavily redundant).

Association between license findings and copyrights

The probably simplest approach (and imo only approach which keeps this ticket reasonably small) would be to recompute the association between license findings and copyright, e.g. in a function similar to OrtResult.collectLicenseFindings
Do a minimal invasive change to the model so that the relation between license findings and copyright findings is represented.

Don't make an association of the findings in the Scanner / ScanResult at all and do this on the fly instead via adding a function to OrtResult. On the go eliminate grouped finding classes from the model replacing it with classes representing a single finding.

current representation:

"license_findings" : [ {
      "license" : "ISC",
      "locations" : [ {
        "path" : "LICENSE",
        "start_line" : 1,
        "end_line" : 15
      } ],
      "copyrights" : [ {
        "statement" : "Copyright (c) Isaac Z. Schlueter and Contributors",
        "locations" : [ {
          "path" : "LICENSE",
          "start_line" : 3,
          "end_line" : 3
        } ]
      } ]
    } ]

proposal (separate licenses and copyrights + singular instead of plural location(s))

"licenses" : [
{
      "license" : "ISC",
      "location" : [ {
        "path" : "LICENSE",
        "start_line" : 1,
        "end_line" : 15
} ],
"copyrights" : [ 
{
        "statement" : "Copyright (c) Isaac Z. Schlueter and Contributors",
        "location" : [ {
          "path" : "LICENSE",
          "start_line" : 3,
          "end_line" : 3
}
]

Rationale:

Move association making out of the scanner and compute it on the fly. This means we can easily change the code making the association without invalidating the scan cache entries.
Dealing with classes representing a single item is easier than dealing with classes having things grouped in a way suitable for a specific use case X which does not exactly match with the current use case. Grouped classes could still exist, but non-grouped ones seem simpler to deal with. The value the grouping brings is lower than the issues it brings imo.
Allows moving the association making into a separate class which takes as input lists of single license and copyright findings and outputs the model(s) needed. This is nicely unit testable btw.
improves on simplicity of license findings / copyright handling and seems a good basis to address also other tech debt related to that

New LicenseView

generalizing existing license view is valuable anyway
adding a new license view after that refactoring is a tiny tasks
rules can choose to operate on DETECTED or DETECTED_CORRECTED which maximized flexibility

generalization:

construct the license view as below, e.g. using vararg ctor param:

LicenseView(getAllConcludedLicenses(), getAllDetectedLicenses())
LicenseView(getAllConcludedLicenses(), getAllDetectedLicenses() + getAllDeclaredLicenses())

fviernau commented 5 years ago

The very basic feature is implemented and of course already usable. This MVP for now applies the curations at a single place and uses curated detected licenses instead of raw detected licenses all over the place, e.g. Reporters/UI and rules/evaluator . What is planned still is

adjust OrtResult.collectLicenseFindings to make application of curations optional
adjust LicenseView such that the rules can choose to operate on raw or curated detected licenses
find some nice way to vizualize the curations in the HTML/WebApp report.

Here is how the basic feature was implemented:

fviernau commented 4 years ago

Closing this ticket as the basic feature has been merged to master. Identified next steps can be:

1894
1962
1895

oss-review-toolkit / ort

Path based license conclusions for projects #1726

1894

1962

1895