fix(spdx): Set the licenseConcluded via ORT's effective license
#11131
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11131 +/- ##
=========================================
Coverage 57.40% 57.40%
Complexity 1703 1703
=========================================
Files 346 346
Lines 12831 12831
Branches 1217 1217
=========================================
Hits 7365 7365
Misses 4997 4997
Partials 469 469
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| val licenseDeclared = resolvedLicenseInfo.mainLicense()?.simplify() | ||
|
|
||
| val licenseConcluded = resolvedLicenseInfo.effectiveLicense(LicenseView.CONCLUDED_OR_DECLARED_AND_DETECTED) | ||
| .takeUnless { it == licenseDeclared } |
There was a problem hiding this comment.
Finally, the
licenseConcludedshould only be set if (human) clearance work was involved, so only set it if it differs from thelicenseDeclared.
Two questions about this:
- The fact that there are detected licenses does not mean that anyone looked at them, so is it still correct to set this field when no concluded license was set?
- If the clearance result is the same as the declared license, should this not be made explicit?
There was a problem hiding this comment.
The fact that there are detected licenses does not mean that anyone looked at them, so is it still correct to set this field when no concluded license was set?
I need to think about this a bit more. In essence, what I want to know if whether a license expression has been modified either by a package curation (via the concluded license) or by a package configuration (via a license finding conclusion).
If the clearance result is the same as the declared license, should this not be made explicit?
Yes, it should, but I believe we have no good mechanism to do so. The only way I could think of to "acknowledge" that the declared license is correct as-is is to set the concluded license to the same expression.
So, I probably should change the logic to always prefer a concluded license, and only fall back to the effective license (if it is different from the declared license).
sschuberth
force-pushed
the
spdx-effective-license
branch
2 times, most recently
from
1e913e0 to
483895b
Compare
sschuberth
force-pushed
the
spdx-effective-license
branch
from
483895b to
345f650
Compare
| .applyChoices(ortResult.getPackageLicenseChoices(id)) | ||
| .applyChoices(ortResult.getRepositoryLicenseChoices()) | ||
|
|
||
| val licenseDeclared = resolvedLicenseInfo.mainLicense()?.simplify() |
There was a problem hiding this comment.
If a human only / exclusively has looked at a single detected license finding, and changed it with a curation, setting the SPDX's licenseConcluded would mean, that the human has looked at the overall (effective license expression) and concluded it to be ok. Which is not the case here. What do you think about this?
Appart from that, the effective license not only can be altered by license finding curations, but also by
- path excludes
- license choices
Should these be considered too (in particular the choices)?
There was a problem hiding this comment.
Which is not the case here. What do you think about this?
It's not necessarily the case, correct. We have no mechanism to signal that "a human has finished looking at all license findings and made curations as necessary", so I'm not sure there's a way to handle this.
Should these be considered too (in particular the choices)?
I believe so, i.e. I do not see why not. For example, I don't think it's a problem that a single fixed software project could generate different SPDX files if different license choices were configured.
Previously, only ORT's `concludedLicense` from a package curation was taken into account. However, if solely detected license findings were cleared via license finding curations from package configurations, that did not have any impact at all until now. To fix this, use the effective license with a custom license view of the declared and detected licenses as a fallback if no concluded license is set. Finally, the `licenseConcluded` should only be set if (human) clearance work was involved, so only set it if it differs from the `licenseDeclared`. If `licenseDeclared` already was correct from the start, then this needs to be "acknowledged" by manually setting the concluded license to the same expression. Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth
force-pushed
the
spdx-effective-license
branch
from
345f650 to
eb3ff11
Compare
Previously, only ORT's
concludedLicensefrom a package curation was taken into account. However, if solely detected license findings were cleared via license finding curations from package configurations, that did not have any impact at all until now.To fix this, use the effective license with a custom license view of the declared and detected licenses as a fallback if no concluded license is set.
Finally, the
licenseConcludedshould only be set if (human) clearance work was involved, so only set it if it differs from thelicenseDeclared. IflicenseDeclaredalready was correct from the start, then this needs to be "acknowledged" by manually setting the concluded license to the same expression.