Skip to content

fix: remove bom-refs only for deleted components in build public#196

Merged
italvi merged 18 commits intomainfrom
154-build-public-messes-up-compositions
Jul 5, 2024
Merged

fix: remove bom-refs only for deleted components in build public#196
italvi merged 18 commits intomainfrom
154-build-public-messes-up-compositions

Conversation

@CBeck-96
Copy link
Copy Markdown
Collaborator

@CBeck-96 CBeck-96 commented Jun 3, 2024 •
edited
Loading

closes #154

@CBeck-96 CBeck-96 linked an issue Jun 3, 2024 that may be closed by this pull request
build-public messes up compositions #154
Closed
@github-actions github-actions bot added enhancement New feature or request unittests labels Jun 3, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 3, 2024 •
edited
Loading

Coverage

Coverage Report •
FileStmtsMissCoverMissing
build_public_bom.py630100% 
auxiliary
   sbomFunctions.py149397%68, 76, 153
TOTAL16849194% 

Tests Skipped Failures Errors Time
297 2 💤 0 ❌ 0 🔥 4.968s ⏱️

@CBeck-96 CBeck-96 requested a review from italvi June 3, 2024 14:19
@CBeck-96 CBeck-96 mentioned this pull request Jun 10, 2024
Merged
@CBeck-96 CBeck-96 changed the title Fix: correct compositions in build public fix: correct compositions in build public Jun 10, 2024
@CBeck-96 CBeck-96 mentioned this pull request Jun 18, 2024
Merged
@italvi italvi requested a review from mmarseu June 26, 2024 05:59
mmarseu
mmarseu previously approved these changes Jun 26, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@mmarseu mmarseu left a comment

Choose a reason for hiding this comment

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

As requested, I tried the changes without reviewing the code. LGTM.

italvi
italvi requested changes Jun 27, 2024
View reviewed changes
Comment thread cdxev/build_public_bom.py Outdated
Comment thread tests/test_sbom_functions.py
@CBeck-96 CBeck-96 force-pushed the 154-build-public-messes-up-compositions branch from a3c3bb0 to 81035da Compare June 28, 2024 16:44
@CBeck-96 CBeck-96 mentioned this pull request Jun 28, 2024
Closed
@mmarseu mmarseu mentioned this pull request Jul 2, 2024
Merged
5 tasks
@CBeck-96 CBeck-96 requested a review from italvi July 3, 2024 11:14
italvi
italvi requested changes Jul 4, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@italvi italvi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

After including the compositions from #154, just for double-check, into my SBOM, where the bom-refs are not used for any component, I made the observation that you delete the bom-refs.

Looking at your code, you are using list_of_components to get your bom-refs and as they are not within the components of my SBOM you remove the bom-refs from #154 in composition. This is definitely a "plausibility check", not something I would expect from build-public. Therefore, please change this behavior.

Do you agree, @mmarseu? (Let's not talk about the irony that we just closed the PR #14 for plausibility check 😆)

@mmarseu
Copy link
Copy Markdown
Collaborator

mmarseu commented Jul 4, 2024

After including the compositions from #154, just for double-check, into my SBOM, where the bom-refs are not used for any component, I made the observation that you delete the bom-refs.

Looking at your code, you are using list_of_components to get your bom-refs and as they are not within the components of my SBOM you remove the bom-refs from #154 in composition. This is definitely a "plausibility check", not something I would expect from build-public. Therefore, please change this behavior.

Do you agree, @mmarseu? (Let's not talk about the irony that we just closed the PR #14 for plausibility check 😆)

Yes, I agree. build-public should strictly only delete bom-refs for deleted components

@CBeck-96 CBeck-96 requested a review from italvi July 4, 2024 07:22
@italvi italvi changed the title fix: correct compositions in build public fix: remove bom-refs only for deleted components in build public Jul 5, 2024
@italvi italvi merged commit 6b26337 into main Jul 5, 2024
@italvi italvi deleted the 154-build-public-messes-up-compositions branch July 5, 2024 12:53
@italvi italvi mentioned this pull request Jul 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mmarseu mmarseu mmarseu left review comments

@italvi italvi italvi approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request unittests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

build-public messes up compositions

3 participants

@CBeck-96 @mmarseu @italvi