Skip to content

Commit 597741d

Browse files
borsbors
committed
Auto merge of #127799 - Kobzol:bootstrap-cmd-refactor-7, r=<try>
Bootstrap command refactoring: make command output API more bulletproof (step 7) Continuation of #127680. This PR modifies the API of running commands to make it more explicit when a command is expected to produce programmatically handled output. Now if you call just `run`, you cannot access the stdout/stderr by accident, because it will not be returned to the caller. This API change might be seen as overkill, let me know what do you think. In any case, I'd like to land the second commit, to make it harder to accidentally read stdout/stderr of commands that did not capture output (now you'd get an empty string as a result, but you should probably get a panic instead, if you try to read uncaptured stdout/stderr). Tracking issue: #126819 r? `@onur-ozkan` try-job: x86_64-msvc
2 parents aa877bc + 5980328 commit 597741d

20 files changed

+161
-169
lines changed

‎src/bootstrap/src/bin/main.rs

+6
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,14 @@ use bootstrap::{
1818
find_recent_config_change_ids, human_readable_changes, t, Build, Config, Subcommand,
1919
CONFIG_CHANGE_HISTORY,
2020
};
21+
use build_helper::ci::CiEnv;
2122

2223
fn main() {
24+
// Enable verbose panics on CI
25+
if CiEnv::is_ci() {
26+
std::env::set_var("RUST_BACKTRACE", "1");
27+
}
28+
2329
let args = env::args().skip(1).collect::<Vec<_>>();
2430
let config = Config::parse(&args);
2531

‎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)