-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add SBOM export support #16523
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?
Add SBOM export support #16523
Conversation
31c2e14 to
4d897e8
Compare
9901b84 to
e110bcd
Compare
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.
Thanks! Some questions and nitpicks 🙂
| Source::Registry(_) | Source::Git(_, _) | Source::Direct(_, _) => { | ||
| // Workspace packages are always local dependencies | ||
| unreachable!( | ||
| "Workspace member {:?} has non-local source {:?}", | ||
| node.package.id.name, node.package.id.source, | ||
| ) | ||
| } |
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.
| let mut output = Vec::<u8>::new(); | ||
|
|
||
| export.output_as_json_v1_5(&mut output)?; | ||
|
|
||
| let output_str = String::from_utf8(output)?; | ||
| write!(writer, "{output_str}")?; |
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.
Q: instead of building an intermediate buffer + string, why not send export.output_as_json_v1_5 directly to writer? If I'm following right, that should avoid rebuffering and allocations here.
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.
Had a go at this but seeing the error below - I think it's because output_as_json_v1_5 has a bound of std::io::Write which OutputWriter doesn't implement, it just has write_fmt defined
$ cargo check
Checking uv v0.9.7 (/Users/thomasschafer/Development/uv/crates/uv)
error[E0277]: the trait bound `OutputWriter<'_>: std::io::Write` is not satisfied
--> crates/uv/src/commands/project/export.rs:367:40
|
367 | export.output_as_json_v1_5(&mut writer)?;
| ------------------- ^^^^^^^^^^^ the trait `std::io::Write` is not implemented for `OutputWriter<'_>`
| |
| required by a bound introduced by this call
|
note: required by a bound in `cyclonedx_bom::models::bom::Bom::output_as_json_v1_5`
--> /Users/thomasschafer/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cyclonedx-bom-0.8.0/src/models/bom.rs:315:35
|
315 | pub fn output_as_json_v1_5<W: std::io::Write>(
| ^^^^^^^^^^^^^^ required by this bound in `Bom::output_as_json_v1_5`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `uv` (lib) due to 1 previous errorI could implement Write for OutputWriter if you think it's worth it? (Or I could be missing something else, let me know!)
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.
Was fairly straightforward to implement Write for OutputWriter and I could then remove the write_fmt method so pushed that up, let me know what you think
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.
332140c to
8e701cc
Compare
SBOM export support - more changes
See PR here for reasoning: CycloneDX/cyclonedx-property-taxonomy#142
docs: uv export documentation
8e701cc to
49fae89
Compare
| @@ -4660,3 +4660,3926 @@ fn export_lock_workspace_mismatch_with_frozen() -> Result<()> { | |||
|
|
|||
| Ok(()) | |||
| } | |||
|
|
|||
| #[test] | |||
| fn cyclonedx_export_basic() -> Result<()> { | |||
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.
Thinking out loud: these snapshots are pretty large, in part because anyio==3.7.0 has a nontrivial number of dependencies.
We definitely want to exercise the dependency paths here, but maybe we could pick a slimmer package to use for testcases here? Some candidates:
urllib3has fewer dependencies (particularly when no extras are used)cryptographyhas very few dependencies by default, but has extras we could use to exercise dependency cases. It has relatively large wheels, however.
Summary
This PR adds a new SBOM format (CycloneDX v1.5 JSON) to the
uv exportcommand.One notable point about the implementation is the use of a synthetic root when using the
--all-projectsflag. This has been discussed separately in more detail, but on a high level, it is possible for workspace packages to be disconnected from the workspace root, so if we had the workspace root as the root component in the SBOM then in such cases there would be unreachable components, which causes issues with some SBOM tooling. By having a synthetic root we ensure that all components can be reached by traversing from the root of the SBOM.Resolves #6012
Test Plan
We've tested manually using a variety of uv projects locally, and have added a variety of tests to
crates/uv/tests/it/export.rs.