Skip to content

Commit c70d63e

Browse files
committed
Make command output capturing more explicit
Now there are separate functions for running a command without capturing, running while capturing stdout and running while capturing everything. This should help avoid situations where stdout/stderr is accessed when it was not captured.
1 parent 2ccafed commit c70d63e

19 files changed

+120
-145
lines changed

src/bootstrap/src/core/build_steps/compile.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1481,7 +1481,7 @@ pub fn compiler_file(
14811481
let mut cmd = command(compiler);
14821482
cmd.args(builder.cflags(target, GitRepo::Rustc, c));
14831483
cmd.arg(format!("-print-file-name={file}"));
1484-
let out = cmd.capture_stdout().run(builder).stdout();
1484+
let out = cmd.run_capture_stdout(builder).stdout();
14851485
PathBuf::from(out.trim())
14861486
}
14871487

@@ -1845,7 +1845,7 @@ impl Step for Assemble {
18451845
builder.ensure(llvm::Llvm { target: target_compiler.host });
18461846
if !builder.config.dry_run() && builder.config.llvm_tools_enabled {
18471847
let llvm_bin_dir =
1848-
command(llvm_config).capture_stdout().arg("--bindir").run(builder).stdout();
1848+
command(llvm_config).arg("--bindir").run_capture_stdout(builder).stdout();
18491849
let llvm_bin_dir = Path::new(llvm_bin_dir.trim());
18501850

18511851
// Since we've already built the LLVM tools, install them to the sysroot.
@@ -2171,7 +2171,7 @@ pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path)
21712171
}
21722172

21732173
let previous_mtime = t!(t!(path.metadata()).modified());
2174-
command("strip").capture().arg("--strip-debug").arg(path).run(builder);
2174+
command("strip").arg("--strip-debug").arg(path).run_capture(builder);
21752175

21762176
let file = t!(fs::File::open(path));
21772177

src/bootstrap/src/core/build_steps/dist.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ fn make_win_dist(
182182
//Ask gcc where it keeps its stuff
183183
let mut cmd = command(builder.cc(target));
184184
cmd.arg("-print-search-dirs");
185-
let gcc_out = cmd.capture_stdout().run(builder).stdout();
185+
let gcc_out = cmd.run_capture_stdout(builder).stdout();
186186

187187
let mut bin_path: Vec<_> = env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect();
188188
let mut lib_path = Vec::new();
@@ -1067,7 +1067,7 @@ impl Step for PlainSourceTarball {
10671067
cmd.arg("--sync").arg(manifest_path);
10681068
}
10691069

1070-
let config = cmd.capture().run(builder).stdout();
1070+
let config = cmd.run_capture(builder).stdout();
10711071

10721072
let cargo_config_dir = plain_dst_src.join(".cargo");
10731073
builder.create_dir(&cargo_config_dir);
@@ -2075,7 +2075,7 @@ fn maybe_install_llvm(
20752075
let mut cmd = command(llvm_config);
20762076
cmd.arg("--libfiles");
20772077
builder.verbose(|| println!("running {cmd:?}"));
2078-
let files = cmd.capture_stdout().run(builder).stdout();
2078+
let files = cmd.run_capture_stdout(builder).stdout();
20792079
let build_llvm_out = &builder.llvm_out(builder.config.build);
20802080
let target_llvm_out = &builder.llvm_out(target);
20812081
for file in files.trim_end().split(' ') {

src/bootstrap/src/core/build_steps/format.rs

+4-6
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ fn get_rustfmt_version(build: &Builder<'_>) -> Option<(String, PathBuf)> {
6060
});
6161
cmd.arg("--version");
6262

63-
let output = cmd.capture().allow_failure().run(build);
63+
let output = cmd.allow_failure().run_capture(build);
6464
if output.is_failure() {
6565
return None;
6666
}
@@ -160,25 +160,23 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) {
160160
}
161161
}
162162
let git_available =
163-
helpers::git(None).capture().allow_failure().arg("--version").run(build).is_success();
163+
helpers::git(None).allow_failure().arg("--version").run_capture(build).is_success();
164164

165165
let mut adjective = None;
166166
if git_available {
167167
let in_working_tree = helpers::git(Some(&build.src))
168-
.capture()
169168
.allow_failure()
170169
.arg("rev-parse")
171170
.arg("--is-inside-work-tree")
172-
.run(build)
171+
.run_capture(build)
173172
.is_success();
174173
if in_working_tree {
175174
let untracked_paths_output = helpers::git(Some(&build.src))
176-
.capture_stdout()
177175
.arg("status")
178176
.arg("--porcelain")
179177
.arg("-z")
180178
.arg("--untracked-files=normal")
181-
.run(build)
179+
.run_capture_stdout(build)
182180
.stdout();
183181
let untracked_paths: Vec<_> = untracked_paths_output
184182
.split_terminator('\0')

src/bootstrap/src/core/build_steps/llvm.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ impl Step for Llvm {
471471
builder.ensure(Llvm { target: builder.config.build });
472472
if !builder.config.dry_run() {
473473
let llvm_bindir =
474-
command(&llvm_config).capture_stdout().arg("--bindir").run(builder).stdout();
474+
command(&llvm_config).arg("--bindir").run_capture_stdout(builder).stdout();
475475
let host_bin = Path::new(llvm_bindir.trim());
476476
cfg.define(
477477
"LLVM_TABLEGEN",
@@ -522,7 +522,7 @@ impl Step for Llvm {
522522
// Helper to find the name of LLVM's shared library on darwin and linux.
523523
let find_llvm_lib_name = |extension| {
524524
let version =
525-
command(&res.llvm_config).capture_stdout().arg("--version").run(builder).stdout();
525+
command(&res.llvm_config).arg("--version").run_capture_stdout(builder).stdout();
526526
let major = version.split('.').next().unwrap();
527527

528528
match &llvm_version_suffix {
@@ -578,7 +578,7 @@ fn check_llvm_version(builder: &Builder<'_>, llvm_config: &Path) {
578578
return;
579579
}
580580

581-
let version = command(llvm_config).capture_stdout().arg("--version").run(builder).stdout();
581+
let version = command(llvm_config).arg("--version").run_capture_stdout(builder).stdout();
582582
let mut parts = version.split('.').take(2).filter_map(|s| s.parse::<u32>().ok());
583583
if let (Some(major), Some(_minor)) = (parts.next(), parts.next()) {
584584
if major >= 17 {

src/bootstrap/src/core/build_steps/run.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ impl Step for BuildManifest {
4040
panic!("\n\nfailed to specify `dist.upload-addr` in `config.toml`\n\n")
4141
});
4242

43-
let today = command("date").capture_stdout().arg("+%Y-%m-%d").run(builder).stdout();
43+
let today = command("date").arg("+%Y-%m-%d").run_capture_stdout(builder).stdout();
4444

4545
cmd.arg(sign);
4646
cmd.arg(distdir(builder));

src/bootstrap/src/core/build_steps/setup.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ impl Step for Link {
275275
}
276276

277277
fn rustup_installed(builder: &Builder<'_>) -> bool {
278-
command("rustup").capture_stdout().arg("--version").run(builder).is_success()
278+
command("rustup").arg("--version").run_capture_stdout(builder).is_success()
279279
}
280280

281281
fn stage_dir_exists(stage_path: &str) -> bool {
@@ -313,10 +313,9 @@ fn attempt_toolchain_link(builder: &Builder<'_>, stage_path: &str) {
313313

314314
fn toolchain_is_linked(builder: &Builder<'_>) -> bool {
315315
match command("rustup")
316-
.capture_stdout()
317316
.allow_failure()
318317
.args(["toolchain", "list"])
319-
.run(builder)
318+
.run_capture_stdout(builder)
320319
.stdout_if_ok()
321320
{
322321
Some(toolchain_list) => {
@@ -341,9 +340,8 @@ fn toolchain_is_linked(builder: &Builder<'_>) -> bool {
341340

342341
fn try_link_toolchain(builder: &Builder<'_>, stage_path: &str) -> bool {
343342
command("rustup")
344-
.capture_stdout()
345343
.args(["toolchain", "link", "stage1", stage_path])
346-
.run(builder)
344+
.run_capture_stdout(builder)
347345
.is_success()
348346
}
349347

@@ -481,9 +479,8 @@ impl Step for Hook {
481479
// install a git hook to automatically run tidy, if they want
482480
fn install_git_hook_maybe(builder: &Builder<'_>, config: &Config) -> io::Result<()> {
483481
let git = helpers::git(Some(&config.src))
484-
.capture()
485482
.args(["rev-parse", "--git-common-dir"])
486-
.run(builder)
483+
.run_capture(builder)
487484
.stdout();
488485
let git = PathBuf::from(git.trim());
489486
let hooks_dir = git.join("hooks");

src/bootstrap/src/core/build_steps/suggest.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,9 @@ pub fn suggest(builder: &Builder<'_>, run: bool) {
1414
let git_config = builder.config.git_config();
1515
let suggestions = builder
1616
.tool_cmd(Tool::SuggestTests)
17-
.capture_stdout()
1817
.env("SUGGEST_TESTS_GIT_REPOSITORY", git_config.git_repository)
1918
.env("SUGGEST_TESTS_NIGHTLY_BRANCH", git_config.nightly_branch)
20-
.run(builder)
19+
.run_capture_stdout(builder)
2120
.stdout();
2221

2322
let suggestions = suggestions

src/bootstrap/src/core/build_steps/synthetic_targets.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ fn create_synthetic_target(
6464
// we cannot use nightly features. So `RUSTC_BOOTSTRAP` is needed here.
6565
cmd.env("RUSTC_BOOTSTRAP", "1");
6666

67-
let output = cmd.capture().run(builder).stdout();
67+
let output = cmd.run_capture(builder).stdout();
6868
let mut spec: serde_json::Value = serde_json::from_slice(output.as_bytes()).unwrap();
6969
let spec_map = spec.as_object_mut().unwrap();
7070

0 commit comments

Comments
 (0)