Repository navigation
fix(cli): surface non-zero child exits in toolchain install/build - #2889
Merged
fakedev9999 merged 3 commits intoSep 10, 2026
Merged
Conversation
`cargo prove` toolchain commands treated child-process failures as success because only spawning/waiting was checked, never `ExitStatus::success()`. `Command::status()` returns `Err` only when the process cannot be spawned or waited on; a process that runs and exits non-zero still returns `Ok(status)`. So a failed `tar` extraction, `rustup toolchain link`, `git clone`, or `python3 x.py build` could be ignored, and `install-toolchain` could still print "Successfully linked toolchain to rustup." after a non-zero link exit. - build_toolchain: `CommandExecutor::run()` now drives the command with `.status()` and returns an error unless the process exited successfully. This covers every command routed through it (git clone, git reset/submodule, x.py build, rustup link, tar). - install_toolchain: add a small `run_checked()` helper that reports a non-zero exit as an error, and use it for the `tar` extract and `rustup toolchain link` calls . . . the only two bare `.status()?` sites. The deliberately-lenient "rustup toolchain remove" and "is rust installed" probes are left unchanged. Add `#[cfg(unix)]` regression tests in both modules asserting that a non-zero child exit becomes an error and a zero exit is `Ok`. Closes succinctlabs#2856 Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
fakedev9999
approved these changes
Sep 10, 2026
fakedev9999
left a comment
fakedev9999
left a comment
Member
There was a problem hiding this comment.
Thanks @MavenRain, lgtm. I added a follow-up to exclude credentials from command errors, with tests covering nonzero exits and spawn failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Fixes #2856.
Several
cargo provetoolchain commands treat child-process failures as success.Command::status()returnsErronly when the process cannot be spawned or waited on — a process that runs and exits non-zero still returnsOk(status). The affected paths only checked theOk/Errsplit, neverExitStatus::success():crates/cli/src/commands/install_toolchain.rs— thetar -xzf ...extract and therustup toolchain link ...calls used.status()?with no success check, so a failed extraction or link was ignored and the command still printedSuccessfully linked toolchain to rustup.crates/cli/src/commands/build_toolchain.rs— the sharedCommandExecutor::run()helper called.output().map(|_| ()), sogit clone,git submodule update,python3 x.py build,rustup toolchain link, andtarwere all considered successful as long as the process spawned, regardless of exit code.The net effect is that a broken toolchain install/build can be reported as successful, surfacing later as confusing downstream failures instead of a clear error at the point of failure.
Solution
Check the exit status wherever these toolchain commands run to completion, and stop with a clear, command-tagged error on a non-zero exit:
build_toolchain.rs—CommandExecutor::run()now drives the command with.status()(stdio inheritance is unchanged) and returns an error unlessstatus.success(). Because every command in that module goes throughrun(), this fixes all of them at once.install_toolchain.rs— adds a small privaterun_checked(&mut Command)helper with the same success check, used for the two bare.status()?sites (tarextract,rustup toolchain link).Intentionally left as-is: the "rustup toolchain remove" paths (which should tolerate "no toolchain installed") and the
rustup --version/forge --version"is it installed?" probes, which legitimately only care whether the process could be spawned. I also audited the rest ofcrates/cli; the one other capturing call site (new.rsgit clone) already checksoutput.status.success().Regression tests (
#[cfg(unix)], usingsh -c "exit N") assert a non-zero child exit becomes an error and a zero exit returnsOkin both modules.Behavior for the success path is unchanged; only genuinely failing commands now abort with an error instead of being silently ignored.
PR Checklist
run_checkedhelper)