Integrate review feedback

* use `PackageIdSpec` instead of only `PackageId` in SBOM output
* change `version` of a dependency to `Option<Version>`
* output `Vec<CrateType>` instead of only the first found crate type
* output rustc workspace wrapper
* update 'warning' string in test using `[WARNING]`
* use `serde_json::to_writer` to serialize SBOM

Author: justahero

src/cargo/core/compiler/output_sbom.rs

@@ -2,16 +2,17 @@
 //! See [`output_sbom`] for more.
 
 use std::collections::BTreeSet;
-use std::io::{BufWriter, Write};
+use std::io::BufWriter;
 use std::path::PathBuf;
 
 use cargo_util::paths::{self};
 use cargo_util_schemas::core::PackageIdSpec;
 use itertools::Itertools;
+use semver::Version;
 use serde::Serialize;
 
 use crate::{
-    core::{profiles::Profile, PackageId, Target, TargetKind},
+    core::{profiles::Profile, Target, TargetKind},
     util::Rustc,
     CargoResult,
 };
@@ -43,16 +44,16 @@ impl<const V: u32> Serialize for SbomFormatVersion<V> {
 #[derive(Serialize, Clone, Debug)]
 struct SbomDependency {
     name: String,
-    package_id: PackageId,
-    version: String,
+    package_id: PackageIdSpec,
+    version: Option<Version>,
     features: Vec<String>,
 }
 
 impl From<&UnitDep> for SbomDependency {
     fn from(dep: &UnitDep) -> Self {
-        let package_id = dep.unit.pkg.package_id();
+        let package_id = dep.unit.pkg.package_id().to_spec();
         let name = package_id.name().to_string();
-        let version = package_id.version().to_string();
+        let version = package_id.version();
         let features = dep
             .unit
             .features
@@ -71,9 +72,9 @@ impl From<&UnitDep> for SbomDependency {
 
 #[derive(Serialize, Clone, Debug)]
 struct SbomPackage {
-    package_id: PackageId,
+    package_id: PackageIdSpec,
     package: String,
-    version: String,
+    version: Option<Version>,
     features: Vec<String>,
     build_type: SbomBuildType,
     extern_crate_name: String,
@@ -86,9 +87,9 @@ impl SbomPackage {
         dependencies: Vec<SbomDependency>,
         build_type: SbomBuildType,
     ) -> Self {
-        let package_id = dep.unit.pkg.package_id();
+        let package_id = dep.unit.pkg.package_id().to_spec();
         let package = package_id.name().to_string();
-        let version = package_id.version().to_string();
+        let version = package_id.version();
         let features = dep
             .unit
             .features
@@ -111,7 +112,7 @@ impl SbomPackage {
 #[derive(Serialize)]
 struct SbomTarget {
     kind: TargetKind,
-    crate_type: Option<CrateType>,
+    crate_types: Vec<CrateType>,
     name: String,
     edition: String,
 }
@@ -120,7 +121,7 @@ impl From<&Target> for SbomTarget {
     fn from(target: &Target) -> Self {
         SbomTarget {
             kind: target.kind().clone(),
-            crate_type: target.kind().rustc_crate_types().first().cloned(),
+            crate_types: target.kind().rustc_crate_types().clone(),
             name: target.name().to_string(),
             edition: target.edition().to_string(),
         }
@@ -131,6 +132,7 @@ impl From<&Target> for SbomTarget {
 struct SbomRustc {
     version: String,
     wrapper: Option<PathBuf>,
+    workspace_wrapper: Option<PathBuf>,
     commit_hash: Option<String>,
     host: String,
 }
@@ -140,6 +142,7 @@ impl From<&Rustc> for SbomRustc {
         Self {
             version: rustc.version.to_string(),
             wrapper: rustc.wrapper.clone(),
+            workspace_wrapper: rustc.workspace_wrapper.clone(),
             commit_hash: rustc.commit_hash.clone(),
             host: rustc.host.to_string(),
         }
@@ -196,9 +199,8 @@ pub fn output_sbom(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> Cargo
     for sbom_output_file in build_runner.sbom_output_files(unit)? {
         let sbom = Sbom::new(unit, packages.clone(), rustc.clone());
 
-        let mut outfile = BufWriter::new(paths::create(sbom_output_file)?);
-        let output = serde_json::to_string(&sbom)?;
-        write!(outfile, "{}", output)?;
+        let outfile = BufWriter::new(paths::create(sbom_output_file)?);
+        serde_json::to_writer(outfile, &sbom)?;
     }
 
     Ok(())

src/doc/src/reference/unstable.md

@@ -390,6 +390,9 @@ build process that are difficult or impossible to obtain in another way.
 To enable this feature either set the `sbom` field in the `.cargo/config.toml`
 
 ```toml
+[unstable]
+sbom = true
+
 [build]
 sbom = true
 ```

tests/testsuite/sbom.rs

@@ -39,7 +39,7 @@ fn build_sbom_without_passing_unstable_flag() {
         .masquerade_as_nightly_cargo(&["sbom"])
         .with_stderr(
             "\
-            warning: ignoring 'sbom' config, pass `-Zsbom` to enable it\n\
+            [WARNING] ignoring 'sbom' config, pass `-Zsbom` to enable it\n\
             [COMPILING] foo v0.5.0 ([..])\n\
             [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]\n",
         )
@@ -74,7 +74,9 @@ fn build_sbom_using_cargo_config() {
                 "kind": [
                     "bin"
                 ],
-                "crate_type": "bin",
+                "crate_types": [
+                    "bin"
+                ],
                 "name": "foo",
                 "edition": "2015"
             },
@@ -100,6 +102,7 @@ fn build_sbom_using_cargo_config() {
             "rustc": {
                 "version": "[..]",
                 "wrapper": null,
+                "workspace_wrapper": null,
                 "commit_hash": "[..]",
                 "host": "[..]"
             }
@@ -171,7 +174,10 @@ fn build_sbom_with_simple_build_script() {
         .file("src/main.rs", "#[cfg(foo)] fn main() {}")
         .file(
             "build.rs",
-            r#"fn main() { println!("cargo::rustc-cfg=foo"); }"#,
+            r#"fn main() {
+                println!("cargo::rustc-check-cfg=cfg(foo)");
+                println!("cargo::rustc-cfg=foo");
+            }"#,
         )
         .build();
 
@@ -187,15 +193,17 @@ fn build_sbom_with_simple_build_script() {
         r#"
         {
             "format_version": 1,
-            "package_id": "path+file:///[..]/foo#0.0.1",
+            "package_id": "path+file://[ROOT]/foo#0.0.1",
             "name": "foo",
             "version": "0.0.1",
             "source": "[ROOT]/foo",
             "target": {
                 "kind": [
                     "bin"
                 ],
-                "crate_type": "bin",
+                "crate_types": [
+                  "bin"
+                ],
                 "name": "foo",
                 "edition": "2015"
             },
@@ -223,18 +231,18 @@ fn build_sbom_with_simple_build_script() {
                         {
                             "features": [],
                             "name": "foo",
-                            "package_id": "foo 0.0.1 (path+file:///[..]/foo)",
+                            "package_id": "path+file://[ROOT]/foo#0.0.1",
                             "version": "0.0.1"
                         }
                     ],
                     "extern_crate_name": "build_script_build",
                     "features": [],
                     "package": "foo",
-                    "package_id": "foo 0.0.1 (path+file:///[..]/foo)",
+                    "package_id": "path+file://[ROOT]/foo#0.0.1",
                     "version": "0.0.1"
                 },
                 {
-                    "package_id": "foo 0.0.1 (path+file:///[..]/foo)",
+                    "package_id": "path+file://[ROOT]/foo#0.0.1",
                     "package": "foo",
                     "version": "0.0.1",
                     "features": [],
@@ -247,6 +255,7 @@ fn build_sbom_with_simple_build_script() {
             "rustc": {
                 "version": "[..]",
                 "wrapper": null,
+                "workspace_wrapper": null,
                 "commit_hash": "[..]",
                 "host": "[..]"
             }
@@ -274,7 +283,10 @@ fn build_sbom_with_build_dependencies() {
         .file("src/lib.rs", "pub fn bar() -> i32 { 2 }")
         .file(
             "build.rs",
-            r#"fn main() { println!("cargo::rustc-cfg=foo"); }"#,
+            r#"fn main() {
+                println!("cargo::rustc-check-cfg=cfg(foo)");
+                println!("cargo::rustc-cfg=foo");
+            }"#,
         )
         .publish();
 
@@ -312,7 +324,9 @@ fn build_sbom_with_build_dependencies() {
                 "kind": [
                     "bin"
                 ],
-                "crate_type": "bin",
+                "crate_types": [
+                    "bin"
+                ],
                 "name": "foo",
                 "edition": "2015"
             },
@@ -335,7 +349,7 @@ fn build_sbom_with_build_dependencies() {
             },
             "packages": [
                 {
-                    "package_id": "bar 0.1.0 (registry+[..])",
+                    "package_id": "registry+[..]#[email protected]",
                     "package": "bar",
                     "version": "0.1.0",
                     "features": [],
@@ -344,14 +358,14 @@ fn build_sbom_with_build_dependencies() {
                     "dependencies": [
                         {
                             "name": "bar",
-                            "package_id": "bar 0.1.0 (registry+[..])",
+                            "package_id": "registry+[..]#[email protected]",
                             "version": "0.1.0",
                             "features": []
                         }
                     ]
                 },
                 {
-                    "package_id": "bar 0.1.0 (registry+[..])",
+                    "package_id": "registry+[..]#[email protected]",
                     "package": "bar",
                     "version": "0.1.0",
                     "features": [],
@@ -360,14 +374,14 @@ fn build_sbom_with_build_dependencies() {
                     "dependencies": [
                         {
                             "name": "bar",
-                            "package_id": "bar 0.1.0 (registry+[..])",
+                            "package_id": "registry+[..]#[email protected]",
                             "version": "0.1.0",
                             "features": []
                         }
                     ]
                 },
                 {
-                    "package_id": "bar 0.1.0 (registry+[..])",
+                    "package_id": "registry+[..]#[email protected]",
                     "package": "bar",
                     "version": "0.1.0",
                     "features": [],
@@ -376,14 +390,14 @@ fn build_sbom_with_build_dependencies() {
                     "dependencies": [
                         {
                             "name": "baz",
-                            "package_id": "baz 0.1.0 (registry+[..])",
+                            "package_id": "registry+[..]#[email protected]",
                             "version": "0.1.0",
                             "features": []
                         }
                     ]
                 },
                 {
-                    "package_id": "baz 0.1.0 (registry+[..])",
+                    "package_id": "registry+[..]#[email protected]",
                     "package": "baz",
                     "version": "0.1.0",
                     "features": [],
@@ -396,6 +410,7 @@ fn build_sbom_with_build_dependencies() {
             "rustc": {
                 "version": "[..]",
                 "wrapper": null,
+                "workspace_wrapper": null,
                 "commit_hash": "[..]",
                 "host": "[..]"
             }