feat: option to choose license format #410
Conversation
…he original - Updated the readme - Fixed a bug when using the license from the classifier Signed-off-by: Jonas Van Den Bossche <jonas.van.den.bossche@awingu.com>
Jonas-vdb
force-pushed
the
license-format
branch
from
1f10a98
to
8757064
Compare
cyclonedx_py/parser/environment.py
Outdated
| class EnvironmentParser(BaseParser): | ||
| """ | ||
| This will look at the current Python environment and list out all installed packages. | ||
|
|
||
| Best used when you have virtual Python environments per project. | ||
| """ | ||
|
|
||
| def __init__(self, use_purl_bom_ref: bool = False) -> None: | ||
| def __init__(self, use_purl_bom_ref: bool = False, licence_output_format: str = 'expression') -> None: |
There was a problem hiding this comment.
❌
licence_output_formatis supposed to be not an arbitrary string, but of the type the new enum is.- the default value should be derived from that enum
- type:
licence_output_format->license_output_format
cyclonedx_py/parser/environment.py
Outdated
| from cyclonedx.model.component import Component | ||
| from cyclonedx.parser import BaseParser | ||
|
|
||
|
|
||
| @enum.unique | ||
| class LICENSE_OUTPUT_FORMAT(enum.Enum): |
There was a problem hiding this comment.
this is an enum, not a constant. user proper casing ala pep8 - https://peps.python.org/pep-0008/#class-names
is there a pep that encourages all-upper-case for enums?
|
Hello @Jonas-vdb , Thank you for the contribution. Some of the tests did not pass - see https://github.com/CycloneDX/cyclonedx-python/runs/8255584075?check_suite_focus=true |
jkowalleck
changed the title
- Filter getting Classifier headers using the get_all function - Fix isort and pep8 issues Signed-off-by: Jonas Van Den Bossche <jonas.van.den.bossche@awingu.com>
|
@jkowalleck Thanks for the PR review.
|
| @enum.unique | ||
| class LicenseOutputFormat(enum.Enum): | ||
| EXPRESSION = 'expression' | ||
| LICENSE = 'license' |
There was a problem hiding this comment.
❌ please UpperCalecase names
❌ named license should be made clearer
❌ even though this enum will eventually affect the output(CLI result), it affects the builder/factory results - therefore rename
@enum.unique
class LicenseFormat(enum.Enum):
Expression = 'license-expression'
Name = 'named-license'| from cyclonedx.model.component import Component | ||
| from cyclonedx.parser import BaseParser | ||
|
|
||
|
|
||
| @enum.unique | ||
| class LicenseOutputFormat(enum.Enum): |
There was a problem hiding this comment.
❌
as this is a global config in the CLI,
this class definition should be soomwhere centalized - like in parser/__init__.py
| if str(cl_header).startswith('License :: OSI Approved :: '): | ||
| cl_license = str(cl_header).replace('License :: OSI Approved :: ', "").strip() | ||
| if licence_output_format == LicenseOutputFormat.EXPRESSION: | ||
| c.licenses.add( |
There was a problem hiding this comment.
can this we made shorter - a oneliner?
| @@ -96,6 +96,9 @@ SBOM Output Configuration: | |||
| exists, it will be overwritten. | |||
| -pb, --purl-bom-ref Use a component's purl for the bom-ref value, instead | |||
| of a random UUID | |||
| --licence_output_format {expression,license} | |||
There was a problem hiding this comment.
indention seams off.
did you actually call the CLI with -h and copy-paste the new help page paragraph here?
|
discussed the topic with @madpah , and we think we will prefer ad different solution: Therefore, this PR will stay open to contract the different solutions, |
Updates
Important change: the license classifier header will only be used if the license header is not available.
This codepath was not working before, it was looping over the different letters of the classifier values instead of looping over all the classifier headers.
Note: When choosing license as license output format, the SPDX expression is set as the license name property. The license format is not a valid SPDX licenseID. So this PR will only add the ability to SEE the license. The matching will no actually work.
See comment from @madpah in #377
fixes #378