Fix logic error in get_components_without_suppliers #176
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Only the supplier field should be checked to determine if the supplier field is missing a value. Previously the code checked both the supplier and the originator fields, on the mistaken assumption that either field counted as the package supplier. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>
jspeed-meyers
commented
Mar 23, 2024
Comment on lines
-98
to
-100
| no_package_originator = package.originator is None or isinstance( | ||
| package.originator, SpdxNoAssertion | ||
| ) |
There was a problem hiding this comment.
This is the critical logic change.
jspeed-meyers
commented
Mar 23, 2024
Comment on lines
-92
to
-94
| # both package supplier and package originator satisfy the "supplier" | ||
| # requirement | ||
| # https://spdx.github.io/spdx-spec/v2.3/package-information/#76-package-originator-field |
There was a problem hiding this comment.
I removed this erroneous comment.
jspeed-meyers
commented
Mar 23, 2024
| # requirement | ||
| # https://spdx.github.io/spdx-spec/v2.3/package-information/#76-package-originator-field | ||
| no_package_supplier = package.supplier is None or isinstance( | ||
| no_supplier = package.supplier is None or isinstance( |
There was a problem hiding this comment.
I simplified the variable naming here, changing no_package_supplier to no_supplier
jspeed-meyers
commented
Mar 23, 2024
| "originator" : "Organization: ExampleCodeInspect (contact@example.com)", | ||
| "originator" : "NOASSERTION", |
There was a problem hiding this comment.
These changes exemplify the test suite changes I made over and over. I removed the originator value, changing it to "NOASSERTION" and did provide a value for supplier.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #157
Only the supplier field should be checked to determine if the supplier field is missing a value. Previously the code checked both the supplier and the originator fields, on the mistaken assumption that either field counted as the package supplier. Oops.
THIS IS A BREAKING CHANGE.
I revised the test suite substantially since this was a logic error. Many test SBOM documents needed a minor tweak.