Fix/issue765 SPDX is_compound_expression does not strictly check for compound expression #773
Conversation
… expression Introduces strict checks for SPDX compound expressions, plus SPDX identifiers and SPDX simple expressions as well to be consistent. Changed SPDX data dependencies to license-expression module to have a single, consistent source. Changed license factory to use new/updated functions to replace try/except/pass statements. fixes CycloneDX#765 Signed-off-by: Joerg Arndt <[email protected]>
… PR. Signed-off-by: Joerg Arndt <[email protected]>
There was a problem hiding this comment.
I see a mix of scopes here
- adding documentation
- fixing typos
- modifying existing behavior
- changing public API
Please split this PR into individual pullrequest, one per scope.
This allowed the autonomous PRs to move on their own pace, be reviewed by the dedicated audience, and simply make reviews easier.
Thank you in advance.
| __SPDX_EXPRESSION_LICENSING: 'Licensing' = get_spdx_licensing() | ||
| __KNOWN_IDS = ([entry['spdx_license_key'] for entry in get_license_index() |
There was a problem hiding this comment.
do i see this right? Instead of using CycloneDX own well-known list of allowed values for a SPDX license ID,
the new code uses an arbitrary list maintained by an unauthoritative 3rd party?
If this is what this is about, then a clear "no". the CycloneDX explicitly states a defined list of well-known values here - which is maintained in https://github.com/CycloneDX/specification/blob/master/schema/spdx.{xsd,schema.json)}
There was a problem hiding this comment.
You are using the license-expression module for license validation in the is_compound _expression. And this module internally takes already the data I use for the SPDX IDs to do this verification for official and specified LicenseRef-scancode-*. So I did not introduce something new.
To use different data from different sources is inconsistent for me an leds to problems if one of the modules, cyclonedx-python-lib and license-expression are not most up to date and correct at the same time. Depending on usage on a single or compound the use of same licenses may result in different IDs. This is what we currently already have.
It could be fine to have a text (for the target application) to check license-expressions SPDX license list IDs data.
During my development (having the dedicated SPDX list as reference) a test was failing when my implementation used license-expression data. Errors may happen, but the latest license-expression maintainers fixed the issue already and converted a formerly existing LicenseRef-scancode- ID into the now official one.
Furthermore, the BSI explicitly recommends the usage of the AboutCode/ScanCode database in their SBOM TR (for the LicenseRef-scancode-* custom IDs that also is incorporated in the license-expression module. And the module also receives updates for the SPDX license list.
If your point to use both at the same time, this is your decision as a core maintainer.
But for me I say that I do not want to rely on two different lists burned manually into each module version.
There was a problem hiding this comment.
You are using the license-expression module for license validation in the is_compound _expression. And this module internally takes already the data I use for the SPDX IDs to do this verification for official and specified LicenseRef-scancode-*. So I did not introduce something new.
This is a complete behavioral change.
The own shipped list of well-known SPDX ID's is a different one than the one any 3rd party library uses.
To use different data from different sources is inconsistent for me an leds to problems [...]
there were no different data. there was just one data: the official list of well-known SPDX license IDs from the official CycloneDX spec. please see below.
If your point to use both at the same time, this is your decision as a core maintainer.
it is not my decision. it is adherence to the spec.
this is the official spec:
- https://github.com/CycloneDX/specification/blob/aadc597de0a239f78403f385c79bdf9099ea366a/schema/bom-1.6.schema.json#L1252-L1257
- https://github.com/CycloneDX/specification/blob/aadc597de0a239f78403f385c79bdf9099ea366a/schema/bom-1.6.xsd#L773-L777
Furthermore, the BSI explicitly recommends the usage of the AboutCode/ScanCode database in their SBOM TR (for the LicenseRef-scancode-* custom IDs that also is incorporated in the license-expression module. And the module also receives updates for the SPDX license list.
This is the BSI's very own narrow profile/view/flavor of SBOM. and it is a recommendation that nobody is forced to follow.
This library is about the international standard CycloneDX - which is much more than SBOM - and much more than a recommendation.
|
current scope-mix PR is not intended to be reviewed as is. |
|
I'm sorry to say that at this time I cannot spend more of my (private) time on this PR. |
feel free to come back any time. |
This pull request solves the issue that the function is_compound_expression is returning true also for other expressions that are not compound expressions.
To be consistent, functions are introduced that form a clear distinction between SPDX license identifiers (SPDX IDs, present in the official SPDX list), simple expressions (either SPDX IDs or custom LicenseRef IDs) and compound expressions.
The implementation now also accesses exclusively the SPDX data coming with the license-expression module that is frequently updated with new SPDX IDs and other licenses - SPDX custom licenses - coming from aboutcode-org.