-
Notifications
You must be signed in to change notification settings - Fork 41
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 LicenseRef support #1148
Add LicenseRef support #1148
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got a question about the test results.
@@ -58,7 +58,7 @@ describe('ScanCodeNewSummarizer basic compatability', () => { | |||
const coordinates = { type: 'pypi', provider: 'pypi' } | |||
const harvestData = getHarvestData(scancodeVersion, 'pypi-complex-declared-license') | |||
const result = summarizer.summarize(coordinates, harvestData) | |||
assert.equal(result.licensed.declared, 'HPND') | |||
assert.equal(result.licensed.declared, 'LicenseRef-scancode-secret-labs-2011') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test originally had a license HPND
. And now it is just LicenseRef-scancode-secret-labs-2011
. Where did the original license come from? I would have thought a change would end up something like HPND AND LicenseRef-scancode-secret-labs-2011
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secret-labs-2011
is the declared license according to the raw ScanCode results. Before this change, our logic fell back to the first package's declared license which is HPND
.
I'm not sure which is the ultimately correct one but we need this change to surface the ScanCode result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to be sure I am understanding logic fell back to the first package's declared license
correctly. Looking at the fixture, I see a license under summary which seems like the correct license...
"summary": {
"declared_license_expression": "secret-labs-2011",
and farther down, I see the first package (transient dependency) has the HPND
as its declared license...
"packages": [
{
"type": "pypi",
"namespace": null,
"name": "Pillow",
"version": "9.5.0",
...
"declared_license_expression": "historical",
"declared_license_expression_spdx": "HPND",
It would be interesting to understand if that is a correct interpretation of how HPND
was identified as the license and why that approach was chosen. To me, that doesn't seem correct as that is the license for Pillow 9.5.0.
@qtomlinson any insights into this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In v30 result (line 760-766) shows content.packages[0].declared_license as HPND
"license_expression": "historical",
"declared_license": {
"license": "HPND",
"classifiers": [
"License :: OSI Approved :: Historical Permission Notice and Disclaimer (HPND)"
]
},
Reading from content.packages[0].declared_license was the preferred way before deriving from files in v30 scancode results. So using v30 scancode, the license would be HPND.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pypi also shows "License as [OSI Approved :: Historical Permission Notice and Disclaimer (HPND)]"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed Pillow 9.5 was curated as HPND
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secret-labs-2011
is the declared license according to the raw ScanCode results.
Just noticed another case where declared_license_expression (v32) seems to be different from what is declared from the package. Added here for documentation purposes.
32.3.0.json:
"summary": {
"declared_license_expression": "cc-by-4.0 AND cc-by-sa-4.0 AND gpl-2.0",
...
package[0]
"declared_license_expression": "gpl-2.0-plus AND gpl-2.0",
"declared_license_expression_spdx": "GPL-2.0-or-later AND GPL-2.0-only",
...
files:
{
"path": "pylint-3.2.3/LICENSE",
"detected_license_expression": "gpl-2.0",
"detected_license_expression_spdx": "GPL-2.0-only",
package[0]
"license_expression": "gpl-2.0-plus AND gpl-2.0",
"declared_license": {
"license": "GPL-2.0-or-later",
"classifiers": [
"License :: OSI Approved :: GNU General Public License v2 (GPLv2)"
]
},
....
files:
{
"path": "pylint-3.2.3/LICENSE",
"key": "gpl-2.0",
"cc-by-4.0 AND cc-by-sa-4.0 AND gpl-2.0" in v32 is different from "gpl-2.0-plus AND gpl-2.0" in v30
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way, I think all of this cases are bugs/regressions in ScanCode, right? Meaning, our code is behaving as expected here, just producing unexpected/wrong results based on the underlying raw data 🤔
A couple comments:
Also, do you mind filing issues ScanCode Toolkit? This will not be tracked otherwise! |
Test failures are related to normalizeLicenseExpression, which depends on the update and release of @clearlydefined/spdx. Its release in turns depends on the merge of the following PRs: |
Also updated spdx-license-list to the most recent version, 6.9.0, to be consistent with @clearlydefined/spdx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I added a test suggestion that will catch a known issue with the scancode map generation workflow.
Co-authored-by: E. Lynette Rayle <elrayle@users.noreply.github.com>
This is a new version of lumaxis#2
Discussion around this at #1096 (comment)