Skip to content

fix(spirv-builder): error instead of panic when cargo can't be spawned - #694

Closed
sorinirimies wants to merge 1 commit into
Rust-GPU:mainfrom
sorinirimies:fix/spirv-builder-cargo-spawn-no-panic
Closed

sorinirimies wants to merge 1 commit into
Rust-GPU:mainfrom
sorinirimies:fix/spirv-builder-cargo-spawn-no-panic

Conversation

@sorinirimies

Copy link
Copy Markdown

Summary

Two more panics in SpirvBuilder::invoke_rustc (follow-up to #689):

-let build = cargo.output().expect("failed to execute cargo build");
+let build = cargo.output().map_err(SpirvBuilderError::CargoSpawnFailed)?;
 ...
-let stdout = String::from_utf8(build.stdout).unwrap();
+let stdout = String::from_utf8_lossy(&build.stdout).into_owned();

New variant SpirvBuilderError::CargoSpawnFailed(io::Error) — "failed to run cargo: {0}". Lossy decoding is fine: cargo's JSON messages are UTF-8, anything else on stdout is only forwarded to the user.

Evidence

multibuilder with cargo absent from PATH (only rustc present; CARGO set so cargo metadata still works):

Before (main) — panic inside spirv-builder:

thread 'main' panicked at crates/spirv-builder/src/lib.rs:1146:36:
failed to execute cargo build: Os { code: 2, kind: NotFound, message: "No such file or directory" }

After — build() returns Err; the panic is the example's own .unwrap():

thread 'main' panicked at examples/multibuilder/src/main.rs:9:34:
called `Result::unwrap()` on an `Err` value: CargoSpawnFailed(Os { code: 2, kind: NotFound, message: "No such file or directory" })

Clippy -D warnings clean.

Merge Danger

Door: two-way

Blast Radius: tiny — SpirvBuilderError is #[non_exhaustive]; only previously-panicking paths change.

- `cargo.output().expect(..)` -> new `SpirvBuilderError::CargoSpawnFailed`
  (the enum is `#[non_exhaustive]`).
- Decode cargo's stdout lossily instead of unwrapping `from_utf8`.
@Firestar99 Firestar99 closed this Nov 7, 2026
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.

2 participants