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

Update metadata contract to be backcompatible with SPDX 2.2 parser #918

Merged
merged 5 commits into from
Feb 3, 2025

Conversation

pragnya17
Copy link
Contributor

The metadata class name was updated in an earlier PR (https://github.com/microsoft/sbom-tool/pull/860/files) which was a breaking change for consumers of the SPDX 2.2 parser. We are introducing this change to allow for backwards compatibility with the SPDX 2.2 parser and compatibility with the SPDX 3.0 parser.

@pragnya17 pragnya17 requested a review from a team as a code owner February 3, 2025 22:04
Copy link
Member

@sfoslund sfoslund left a comment

Choose a reason for hiding this comment

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

The one other place we will need to change is SbomParser, which references SpdxMetadata right now in the GetMetadata method, making it not backwards compatible. Depending on how permanent we want these changes to be, we could do this by making ISbomParser generic depending on metadata type, or we can just temporarily revert it to only use the 2.2 metadata version

Copy link
Contributor

@DaveTryon DaveTryon left a comment

Choose a reason for hiding this comment

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

Left 1 question to consider.

@pragnya17
Copy link
Contributor Author

The one other place we will need to change is SbomParser, which references SpdxMetadata right now in the GetMetadata method, making it not backwards compatible. Depending on how permanent we want these changes to be, we could do this by making ISbomParser generic depending on metadata type, or we can just temporarily revert it to only use the 2.2 metadata version

Would you mind linking which file you're referring to? I looked through all the references of GetMetadata and it seems right to me

@sfoslund
Copy link
Member

sfoslund commented Feb 3, 2025

Yeah, this is where I think we need an update: https://github.com/microsoft/sbom-tool/blob/main/src/Microsoft.Sbom.Parsers.Spdx22SbomParser/Parser/SPDXParser.cs#L175. DM'ed you to discuss this break in more detail :)

@pragnya17 pragnya17 requested a review from sfoslund February 3, 2025 23:27
@sfoslund
Copy link
Member

sfoslund commented Feb 3, 2025

/azp run

@pragnya17 pragnya17 enabled auto-merge (squash) February 3, 2025 23:40
@pragnya17 pragnya17 merged commit cb45054 into main Feb 3, 2025
4 checks passed
@pragnya17 pragnya17 deleted the sfoslund/metadataContract branch February 3, 2025 23:53
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.

3 participants