Skip to content

Conversation

@baprusty
Copy link

@baprusty baprusty commented Nov 5, 2025

This is currently a work in progress. I’ll provide an update once it’s completed.

"""
if "spdxVersion" in data:
return "spdx"
if data.get("bomFormat", "").lower() == "cyclonedx":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the constants we provide to define the format.



@classmethod
def load_spdx_sbom(cls, path):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put this behind a factory and add the concrete implementations into compare/spdx.py and compare/cdx.py.

@classmethod
def load_spdx_sbom(cls, path):
"""Return packages keyed by purl or fallback name@version."""
with open(path) as f:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bomreader class should already provide the infrastructure to read SBOMs. By that, probably all type-specific sbom loading can be avoided.



@classmethod
def build_extra_cdx(cls, extra_components, new_metadata=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the bomwriter logic to re-create an sbom.



@classmethod
def compare_items(cls, base_sbom_comp, target_sbom_comp, key_name):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we already have a similar infrastructure in merge?

help="Path or URL to the target (new) SBOM file"
)

parser.add_argument(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please support the unified output (e.g. file or stdout), similar to the other output options. Please also consider to make it an optional positional argument instead of an option.

The same applies to the input options.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants