feat: add support for license expression details#908
feat: add support for license expression details#908Churro wants to merge 1 commit intoCycloneDX:mainfrom
Conversation
|
before i look into the details of the implementation, please explain what the goal is.
|
The goal is to add XML and JSON serialization/deserialization support for license expression details.
I used the following schema test data with license expression details as a reference on how it should look like: The implementation mimics the structure of
A “skipped” warning is issued when attempting to serialize this data type for CDX < 1.7. No “backport”-like functionality. |
|
I would really love to have minimal backport capabilities in the following sense:
so this would become {
"$schema": "http://cyclonedx.org/schema/bom-1.6.schema.json",
"bomFormat": "CycloneDX",
"specVersion": "1.6",
"serialNumber": "urn:uuid:8ad91ceb-1741-4d58-8d22-4488a0f68dbe",
"version": 1,
"components": [
{
"type": "application",
"name": "my-application",
"version": "1.33.7",
"description": "This application is composed of multiple things, and therefore has multiple licenses applied:\n* custom code - custom license\n* component A - EPL or GPL\n* component B - MIT\n* component C - MIT",
"licenses": [
{
"bom-ref": "my-application-license",
"acknowledgement": "declared",
"expression": "LicenseRef-my-custom-license AND (EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0) AND MIT"
}
]
}
]
}this "transformation" might be tricky - and might be added as a capability in a later iteration as a followup. do you think this is possible? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for LicenseExpressionDetailed and ExpressionDetails classes to handle detailed license expression information introduced in CycloneDX v1.7. The changes include new model classes, serialization/deserialization logic, and comprehensive test coverage.
- Introduces
LicenseExpressionDetailedclass to represent detailed license expressions with expression details and properties - Introduces
ExpressionDetailsclass to represent individual license identifier details within an expression - Updates the
Licensetype alias to include the newLicenseExpressionDetailedtype
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cyclonedx/model/license.py | Adds new ExpressionDetails and LicenseExpressionDetailed classes with full serialization support for CycloneDX v1.7, updates License type union, and implements comparison logic for proper sorting |
| tests/test_model_license.py | Adds comprehensive test coverage for the new LicenseExpressionDetailed and ExpressionDetails classes including creation, update, equality, and sorting tests |
| tests/_data/models.py | Adds test data for components with expression details to validate serialization across different schema versions |
| tests/_data/snapshots/*.bin | Updates snapshot files for all CycloneDX versions (1.0-1.7) to include the new test component with expression details |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
In ebe9f2e, I've now added a pragmatic transformation that maps With JSON serialization, it's comparably easy because |
|
From a user's perspective, I would find it more appealing to have no new data model, but use the existing The (de)normalization is non-user-facing, non-public, and custom-made in the I mean, this should be feasible, right? |
|
I've now merged this into An inconsistency that could still be addressed in this PR is that |
Properties is a completely different scope. |
| view: Optional[type[serializable.ViewType]], | ||
| xmlns: Optional[str] | ||
| ) -> Element: | ||
| elem: Element = license_expression.as_xml( # type:ignore[attr-defined] |
There was a problem hiding this comment.
note to self: this looks odd - a strange back and forth of already built XML foo.
i would have expected completely different, need to look into the details again.
| raise CycloneDxDeserializationException(f'unexpected content: {li!r}') | ||
| license_expression = LicenseExpression.from_xml( # type:ignore[attr-defined] | ||
| li, default_ns) | ||
| license_expression.value = expression_value |
There was a problem hiding this comment.
note to self: this looks odd. the detection is not quite intuitive from a first look. need to look into the details again.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I may not be able to review this pull request until early next year due to other priorities. I appreciate your patience. |
Documentation build overview
|
f562f06 to
3fc6419
Compare
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 31 |
| Duplication | 3 |
TIP This summary will be updated as you push new changes. Give us feedback
Signed-off-by: Johannes Feichtner <johannes@web-wack.at>
3fc6419 to
72fb108
Compare
|
@jkowalleck, I've resolved conflicts with latest main. Could you please have another look and see if its ready for merge? |
sure. need some time to review time in detail. |
Description
Adds supports for license expression details as part of #903
A clear and concise summary of the change and which issue (if any) it fixes. Should also include relevant motivation and context.
Resolves or fixes issue:
AI Tool Disclosure
[e.g. GitHub CoPilot, ChatGPT, JetBrains Junie etc.][e.g. GPT-4.1, Claude Haiku 4.5, Gemini 2.5 Pro etc.][Summarize the key prompts or instructions given to the AI tools]Affirmation