Skip to content
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

feat!: make bom-ref part of equality checks #753

Open
jkowalleck opened this issue Jan 17, 2025 · 4 comments · Fixed by #754 · May be fixed by #777
Open

feat!: make bom-ref part of equality checks #753

jkowalleck opened this issue Jan 17, 2025 · 4 comments · Fixed by #754 · May be fixed by #777
Assignees
Labels
breaking change enhancement New feature or request

Comments

@jkowalleck
Copy link
Member

jkowalleck commented Jan 17, 2025

context

Currently, components that differ only in the property bom-ref are treated as equal.
This is for the reason, that bom-ref is actually just a decorative marker for interlinkage.

Anyway, there are external tools and processes that produce CycloneDX like this:

{
  "$schema": "http://cyclonedx.org/schema/bom-1.5.schema.json",
  "bomFormat": "CycloneDX",
  "specVersion": "1.5",
  "version": 1,
  "metadata": {
      "component": {
      "type": "application",
      "name": "example",
      "version": "1.2.3",
      "bom-ref": "topref"
    }
  },
  "components": [
    {
      "type": "library",
      "name": "styled-engine",
      "version": "5.16.6",
      "bom-ref": "@mui/styled-engine@npm:5.16.6 [296f2]"
    },
    {
      "type": "library",
      "name": "styled-engine",
      "version": "5.16.6",
      "bom-ref": "@mui/styled-engine@npm:5.16.6 [3135b]"
    }
  ],
  "dependencies": [
    {
      "ref": "topref",
      "dependsOn": [
        "@mui/styled-engine@npm:5.16.6 [296f2]",
        "@mui/styled-engine@npm:5.16.6 [3135b]"
      ]
    }
  ]
}

These are, schema-wise, valid CycloneDX.

the problem

when these things are imported by this library, the components get deduplicated (since they are equal) - which causes issues with the dependency tree - and alternates the structure when exporting.

solution

bom-ref MUST be taken into account for the following processes

  • equality and comparison (__eq__, __lt__, __gt__, ...)
  • hashing (__hash__)

❗ Since BomRef will become a central part of hashing and comparison, we need to revisit the respective dunder methods on BomRef.

related

closes #753

@jkowalleck
Copy link
Member Author

@madpah FYI

@jkowalleck jkowalleck self-assigned this Jan 17, 2025
@jkowalleck
Copy link
Member Author

jkowalleck commented Jan 17, 2025

I will work on a fix, soon.

@hirsche
Copy link

hirsche commented Jan 30, 2025

I'd ask for this one as well.

I was debugging my code for ages, not getting why some of my "LIBRARY"-Components, I have been adding successfully, where lost after adding other components - well just because they where regarded as the same component and then got overwritten with another component having a different bom-ref but same name.

Of course usually you'd have more properties assigned making them distinguishable over the hash, however, when you start with some basic code and add an "assumed-to-be-a-reference" it is a bit confusion when components get lost.

jkowalleck added a commit that referenced this issue Feb 12, 2025
For some this is considered a bug-fix, for others this is a feature - it
is a breaking change anyway since it modifies the order of things.

----

TODO:
- [x] **every** symbol that has a property `bom-ref` MUST utilize it for
dunder methods `hash`,`eq`,`gt`,`lt`,...
- [x] add new test cases from #753
- [x] add new test cases from #540
- [x] add new test cases from #677
- [x] create new tests snapshots (if applicable)

----

> [!important]
> depends on #755

supersedes #678
closes #678

fixes #753
fixes #540
fixes #677

---------

Signed-off-by: wkoot <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Co-authored-by: wkoot <[email protected]>
@jkowalleck jkowalleck linked a pull request Feb 12, 2025 that will close this issue
@jkowalleck
Copy link
Member Author

A preview of the fix/feature is available via https://github.com/CycloneDX/cyclonedx-python-lib/releases/tag/v9.0.1-rc.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment