clearlydefined / service

The service side of clearlydefined.io
MIT License
45 stars 40 forks source link

Update license mapping with latest ScanCode LicenseDB data #1049

Closed github-actions[bot] closed 5 months ago

github-actions[bot] commented 7 months ago

This pull request updates the license mapping with the latest ScanCode LicenseDB data. For more information, see https://github.com/nexB/scancode-licensedb

lumaxis commented 7 months ago

I'll take a look at the code to make sure what the implications are of adding all the new licenses and maybe add a test to ensure the desire behavior.

lumaxis commented 6 months ago

Added a check to ensure we don't accidentally use LicenseRef- right now

qtomlinson commented 6 months ago

Added a check to ensure we don't accidentally use LicenseRef- right now

Thank you for updating the mapping and for the extra effort in checking for LicenseRef. However, I don't believe that checking for licenseRef is necessary in this case. The presence of LicenseRefs in the license mapping does not seem to affect the license parsing. We already have an existing licenseRef mapping ['here-proprietary', 'LicenseRef-Proprietary-HERE']. To verify this, we can add a test case, [buildRule('MIT AND Apache-2.0 AND here-proprietary', ['mit', 'apache-2.0', 'here-proprietary']), 'MIT AND Apache-2.0 AND NOASSERTION'], to the existing unit test. The expected result expression for this test case is 'MIT AND Apache-2.0 AND NOASSERTION'. The presence of ['here-proprietary', 'LicenseRef-Proprietary-HERE'] in scancodeMap.js is not reflected in the result.

The reason is that the licenseVisitor, where the scancodeMap is used, is only utilized during the parseLicense process. After looking up in the scancodeMap, the result value from the map is further normalized (normalizeSingle) to return a SPDX identifier. LicenseRef- values from the scancodeMap are converted NOASSERTION during normalizeSingle.

lumaxis commented 5 months ago

@qtomlinson I added the test you suggested and removed the explicit check for now to avoid confusion, see also my comment for further info on why LicenseRef- wouldn't currently be used.