-
Notifications
You must be signed in to change notification settings - Fork 155
[DNM] Generate SBOMs for repaired libraries #577
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #577 +/- ##
==========================================
- Coverage 92.78% 91.98% -0.80%
==========================================
Files 21 22 +1
Lines 1760 1797 +37
Branches 332 336 +4
==========================================
+ Hits 1633 1653 +20
- Misses 77 86 +9
- Partials 50 58 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Taking this pull request out of draft to begin inviting reviews. I still need to add some integration tests to show that the SBOM documents get generated for many different operating systems offered by manylinux images :) cc @lkollar, @mayeut, and @captn3m0 who expressed interested in the linked issues. |
@@ -0,0 +1,359 @@ | ||
# SPDX-License-Identifier: MIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a vendored project "whichprovides" which I maintain. It's the abstraction of the package provider detection that was mentioned here: #541 (comment)
Please feel free to read and review this code, too. This code hasn't been reviewed by anyone except myself, so your perspective is more than welcome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces automatic SBOM generation for repaired wheels while cleaning up logging in docker execution during testing. Key changes include replacing direct print statements with logger calls in test scripts, adding a new SBOM generation module (CycloneDX format), and integrating SBOM creation into the wheel repair process.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/integration/test_manylinux.py | Updated logging for docker_exec error messages instead of print |
src/auditwheel/sboms.py | New module implementing SBOM generation using CycloneDX schema |
src/auditwheel/repair.py | Integrated SBOM generation into the wheel repair process |
src/auditwheel/_vendor/whichprovides/* | Bundled vendor code update for the whichprovides project |
src/auditwheel/_vendor/whichprovides/LICENSE.txt | Added/updating the license for the bundled vendor code |
@@ -336,7 +336,7 @@ def docker_exec( | ||
ec, output = container.exec_run(cmd, workdir=cwd, environment=env) | ||
output = output.decode("utf-8") | ||
if ec != expected_retcode: | ||
print(output) | ||
logger.info("docker exec error %s: %s", container.id[:12], output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using logger.error for logging error messages to better distinguish error severity from informational logs.
logger.info("docker exec error %s: %s", container.id[:12], output) | |
logger.error("docker exec error %s: %s", container.id[:12], output) |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly made this change so that the logs are visible in the test output.
@@ -63,6 +66,9 @@ def repair_wheel( | ||
raise ValueError(msg) | ||
|
||
dest_dir = Path(match.group("name") + lib_sdir) | ||
dist_info_dirs = glob.glob(str(ctx.path / "*.dist-info")) | ||
assert len(dist_info_dirs) == 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an assert to check for a single '.dist-info' directory may be too brittle; consider raising an explicit exception with a descriptive error message for clarity in failure cases.
assert len(dist_info_dirs) == 1 | |
if len(dist_info_dirs) != 1: | |
raise ValueError( | |
f"Expected exactly one '.dist-info' directory, but found {len(dist_info_dirs)}: {dist_info_dirs}" | |
) |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see the need to have an actual exception here, but I can change this if desirable.
for more information, see https://pre-commit.ci
(Draft, do not merge) This is my initial pull request for adding automatic SBOM generation based on package manager info for libraries that are repaired into wheels. I've tested locally by building wheels from various projects and operating systems, will work on getting those pulled into the test suite.
The majority of the "logic" comes from the project "whichprovides" which gets bundled into auditwheel as a single file. You can review that project in its entirety within this pull request. If you'd like to submit comments that you have about "whichprovides" you can do so here and I'll get them addressed in the upstream project.
Closes #541
Closes #398