Skip to content

[spdx] Add installed package files to SPDX SBOM file #1744

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

Merged

Conversation

ekilmer
Copy link
Contributor

@ekilmer ekilmer commented Aug 1, 2025

Also remove repetition of relationships. These are no longer present in v3 of the SPDX spec. Keeping these relationships would also make the SBOM very noisy.

Also remove repetition of relationships. These are no longer present in v3 of
the SPDX spec. Keeping these relationships would also make the SBOM very
noisy.
@BillyONeal BillyONeal requested a review from ras0219-msft August 1, 2025 23:40
Copy link
Collaborator

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

Overall, this looks great -- thank you for the PR!

Because this is changing the output packages, it needs to change the output package ABI (so we'll regenerate all packages with the new SBOM information). Can you add a new entry after AbiTagPostBuildChecks for this?

https://github.com/microsoft/vcpkg-tool/blob/main/src/vcpkg/commands.build.cpp#L1458

@ekilmer ekilmer force-pushed the ek/spdx-track-installed-files branch from 7a0abc4 to d589226 Compare August 14, 2025 18:19
Because this is changing the output packages, it needs to change the output
package ABI (so we'll regenerate all packages with the new SBOM information)
@ekilmer
Copy link
Contributor Author

ekilmer commented Aug 14, 2025

Overall, this looks great -- thank you for the PR!

Because this is changing the output packages, it needs to change the output package ABI (so we'll regenerate all packages with the new SBOM information). Can you add a new entry after AbiTagPostBuildChecks for this?

https://github.com/microsoft/vcpkg-tool/blob/main/src/vcpkg/commands.build.cpp#L1458

Thank you for the review and for pointing out the ABI change. Honestly, I'm not sure what's expected there, but I attempted something. I'm happy to change it to something else if you can help guide me towards a better alternative implementation.

// Gather all the files in the package directory
// Note: For packages with many files, this sequential hashing may be slow
const auto relative_package_files =
fs.get_regular_files_recursive_lexically_proximate(package_dir, VCPKG_LINE_INFO);
Copy link
Member

Choose a reason for hiding this comment

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

:sigh: We really need to start doing this once per port and then feeding it into everything downstream.

No change requested.

@BillyONeal BillyONeal merged commit de63c0b into microsoft:main Aug 15, 2025
7 checks passed
@BillyONeal
Copy link
Member

Thanks!

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