Skip to content

Commit

Permalink
Auto merge of #113214 - GuillaumeGomez:try-run-fix, r=ozkanonur,jyn514
Browse files Browse the repository at this point in the history
Don't fail early if `try_run` returns an error

Fixes #113208.

Follow-up of #112962.

r? `@jyn514`
  • Loading branch information
bors committed Jul 12, 2023
2 parents da1d099 + 98336f8 commit 1e6db34
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ impl Config {
patchelf.args(&["--set-interpreter", dynamic_linker.trim_end()]);
}

self.try_run(patchelf.arg(fname)).unwrap();
let _ = self.try_run(patchelf.arg(fname));
}

fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) {
Expand Down
11 changes: 5 additions & 6 deletions src/bootstrap/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ impl Step for ExpandYamlAnchors {
try_run(
builder,
&mut builder.tool_cmd(Tool::ExpandYamlAnchors).arg("generate").arg(&builder.src),
)
.unwrap();
);
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand All @@ -40,17 +39,17 @@ impl Step for ExpandYamlAnchors {
}
}

fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> Result<(), ()> {
fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
if !builder.fail_fast {
if let Err(e) = builder.try_run(cmd) {
if builder.try_run(cmd).is_err() {
let mut failures = builder.delayed_failures.borrow_mut();
failures.push(format!("{:?}", cmd));
return Err(e);
return false;
}
} else {
builder.run(cmd);
}
Ok(())
true
}

#[derive(Debug, PartialOrd, Ord, Copy, Clone, Hash, PartialEq, Eq)]
Expand Down
36 changes: 16 additions & 20 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,17 @@ const MIR_OPT_BLESS_TARGET_MAPPING: &[(&str, &str)] = &[
// build for, so there is no entry for "aarch64-apple-darwin" here.
];

fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> Result<(), ()> {
fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
if !builder.fail_fast {
if let Err(e) = builder.try_run(cmd) {
if builder.try_run(cmd).is_err() {
let mut failures = builder.delayed_failures.borrow_mut();
failures.push(format!("{:?}", cmd));
return Err(e);
return false;
}
} else {
builder.run(cmd);
}
Ok(())
true
}

fn try_run_quiet(builder: &Builder<'_>, cmd: &mut Command) -> bool {
Expand Down Expand Up @@ -187,8 +187,7 @@ You can skip linkcheck with --exclude src/tools/linkchecker"
try_run(
builder,
builder.tool_cmd(Tool::Linkchecker).arg(builder.out.join(host.triple).join("doc")),
)
.unwrap();
);
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand Down Expand Up @@ -241,8 +240,7 @@ impl Step for HtmlCheck {
builder.default_doc(&[]);
builder.ensure(crate::doc::Rustc::new(builder.top_stage, self.target, builder));

try_run(builder, builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)))
.unwrap();
try_run(builder, builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)));
}
}

Expand Down Expand Up @@ -288,8 +286,7 @@ impl Step for Cargotest {
.args(builder.config.test_args())
.env("RUSTC", builder.rustc(compiler))
.env("RUSTDOC", builder.rustdoc(compiler)),
)
.unwrap();
);
}
}

Expand Down Expand Up @@ -855,7 +852,7 @@ impl Step for RustdocTheme {
util::lld_flag_no_threads(self.compiler.host.contains("windows")),
);
}
try_run(builder, &mut cmd).unwrap();
try_run(builder, &mut cmd);
}
}

Expand Down Expand Up @@ -1106,7 +1103,7 @@ help: to skip test's attempt to check tidiness, pass `--exclude src/tools/tidy`
}

builder.info("tidy check");
try_run(builder, &mut cmd).unwrap();
try_run(builder, &mut cmd);

builder.ensure(ExpandYamlAnchors);

Expand Down Expand Up @@ -1154,8 +1151,7 @@ impl Step for ExpandYamlAnchors {
try_run(
builder,
&mut builder.tool_cmd(Tool::ExpandYamlAnchors).arg("check").arg(&builder.src),
)
.unwrap();
);
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand Down Expand Up @@ -1948,7 +1944,7 @@ impl BookTest {
compiler.host,
);
let _time = util::timeit(&builder);
let toolstate = if try_run(builder, &mut rustbook_cmd).is_ok() {
let toolstate = if try_run(builder, &mut rustbook_cmd) {
ToolState::TestPass
} else {
ToolState::TestFail
Expand Down Expand Up @@ -2106,7 +2102,7 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) ->
cmd.arg("--test-args").arg(test_args);

if builder.config.verbose_tests {
try_run(builder, &mut cmd).is_ok()
try_run(builder, &mut cmd)
} else {
try_run_quiet(builder, &mut cmd)
}
Expand Down Expand Up @@ -2134,7 +2130,7 @@ impl Step for RustcGuide {

let src = builder.src.join(relative_path);
let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook);
let toolstate = if try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src)).is_ok() {
let toolstate = if try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src)) {
ToolState::TestPass
} else {
ToolState::TestFail
Expand Down Expand Up @@ -2684,7 +2680,7 @@ impl Step for Bootstrap {
.current_dir(builder.src.join("src/bootstrap/"));
// NOTE: we intentionally don't pass test_args here because the args for unittest and cargo test are mutually incompatible.
// Use `python -m unittest` manually if you want to pass arguments.
try_run(builder, &mut check_bootstrap).unwrap();
try_run(builder, &mut check_bootstrap);

let host = builder.config.build;
let compiler = builder.compiler(0, host);
Expand Down Expand Up @@ -2756,7 +2752,7 @@ impl Step for TierCheck {
}

builder.info("platform support check");
try_run(builder, &mut cargo.into()).unwrap();
try_run(builder, &mut cargo.into());
}
}

Expand Down Expand Up @@ -2836,7 +2832,7 @@ impl Step for RustInstaller {
cmd.env("CARGO", &builder.initial_cargo);
cmd.env("RUSTC", &builder.initial_rustc);
cmd.env("TMP_DIR", &tmpdir);
try_run(builder, &mut cmd).unwrap();
try_run(builder, &mut cmd);
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand Down

0 comments on commit 1e6db34

Please sign in to comment.