Skip to content

Commit 7b55779

Browse files
GuillaumeGomezGuillaumeGomez
committed
Make try_run return a Result<(), ()> instead of a boolean
1 parent d70faf3 commit 7b55779

File tree

8 files changed

+63
-50
lines changed

8 files changed

+63
-50
lines changed

‎src/bootstrap/download.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ impl Config {
5454
/// Runs a command, printing out nice contextual information if it fails.
5555
/// Exits if the command failed to execute at all, otherwise returns its
5656
/// `status.success()`.
57-
pub(crate) fn try_run(&self, cmd: &mut Command) -> bool {
57+
pub(crate) fn try_run(&self, cmd: &mut Command) -> Result<(), ()> {
5858
if self.dry_run() {
59-
return true;
59+
return Ok(());
6060
}
6161
self.verbose(&format!("running: {:?}", cmd));
6262
try_run(cmd, self.is_verbose())
@@ -156,12 +156,14 @@ impl Config {
156156
];
157157
}
158158
";
159-
nix_build_succeeded = self.try_run(Command::new("nix-build").args(&[
160-
Path::new("-E"),
161-
Path::new(NIX_EXPR),
162-
Path::new("-o"),
163-
&nix_deps_dir,
164-
]));
159+
nix_build_succeeded = self
160+
.try_run(Command::new("nix-build").args(&[
161+
Path::new("-E"),
162+
Path::new(NIX_EXPR),
163+
Path::new("-o"),
164+
&nix_deps_dir,
165+
]))
166+
.is_ok();
165167
nix_deps_dir
166168
});
167169
if !nix_build_succeeded {
@@ -186,7 +188,7 @@ impl Config {
186188
patchelf.args(&["--set-interpreter", dynamic_linker.trim_end()]);
187189
}
188190

189-
self.try_run(patchelf.arg(fname));
191+
self.try_run(patchelf.arg(fname)).unwrap();
190192
}
191193

192194
fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) {
@@ -237,7 +239,7 @@ impl Config {
237239
"(New-Object System.Net.WebClient).DownloadFile('{}', '{}')",
238240
url, tempfile.to_str().expect("invalid UTF-8 not supported with powershell downloads"),
239241
),
240-
])) {
242+
])).is_err() {
241243
return;
242244
}
243245
eprintln!("\nspurious failure, trying again");

‎src/bootstrap/lib.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ forward! {
333333
create(path: &Path, s: &str),
334334
remove(f: &Path),
335335
tempdir() -> PathBuf,
336-
try_run(cmd: &mut Command) -> bool,
336+
try_run(cmd: &mut Command) -> Result<(), ()>,
337337
llvm_link_shared() -> bool,
338338
download_rustc() -> bool,
339339
initial_rustfmt() -> Option<PathBuf>,
@@ -614,11 +614,13 @@ impl Build {
614614
}
615615

616616
// Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error).
617-
let has_local_modifications = !self.try_run(
618-
Command::new("git")
619-
.args(&["diff-index", "--quiet", "HEAD"])
620-
.current_dir(&absolute_path),
621-
);
617+
let has_local_modifications = self
618+
.try_run(
619+
Command::new("git")
620+
.args(&["diff-index", "--quiet", "HEAD"])
621+
.current_dir(&absolute_path),
622+
)
623+
.is_err();
622624
if has_local_modifications {
623625
self.run(Command::new("git").args(&["stash", "push"]).current_dir(&absolute_path));
624626
}

‎src/bootstrap/run.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ impl Step for ExpandYamlAnchors {
2727
try_run(
2828
builder,
2929
&mut builder.tool_cmd(Tool::ExpandYamlAnchors).arg("generate").arg(&builder.src),
30-
);
30+
)
31+
.unwrap();
3132
}
3233

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

42-
fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
43+
fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> Result<(), ()> {
4344
if !builder.fail_fast {
44-
if !builder.try_run(cmd) {
45+
if let Err(e) = builder.try_run(cmd) {
4546
let mut failures = builder.delayed_failures.borrow_mut();
4647
failures.push(format!("{:?}", cmd));
47-
return false;
48+
return Err(e);
4849
}
4950
} else {
5051
builder.run(cmd);
5152
}
52-
true
53+
Ok(())
5354
}
5455

5556
#[derive(Debug, PartialOrd, Ord, Copy, Clone, Hash, PartialEq, Eq)]

‎src/bootstrap/test.rs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,17 @@ const MIR_OPT_BLESS_TARGET_MAPPING: &[(&str, &str)] = &[
4848
// build for, so there is no entry for "aarch64-apple-darwin" here.
4949
];
5050

51-
fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
51+
fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> Result<(), ()> {
5252
if !builder.fail_fast {
53-
if !builder.try_run(cmd) {
53+
if let Err(e) = builder.try_run(cmd) {
5454
let mut failures = builder.delayed_failures.borrow_mut();
5555
failures.push(format!("{:?}", cmd));
56-
return false;
56+
return Err(e);
5757
}
5858
} else {
5959
builder.run(cmd);
6060
}
61-
true
61+
Ok(())
6262
}
6363

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

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

243-
try_run(builder, builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)));
244+
try_run(builder, builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)))
245+
.unwrap();
244246
}
245247
}
246248

@@ -286,7 +288,8 @@ impl Step for Cargotest {
286288
.args(builder.config.test_args())
287289
.env("RUSTC", builder.rustc(compiler))
288290
.env("RUSTDOC", builder.rustdoc(compiler)),
289-
);
291+
)
292+
.unwrap();
290293
}
291294
}
292295

@@ -785,7 +788,7 @@ impl Step for Clippy {
785788
cargo.add_rustc_lib_path(builder, compiler);
786789
let mut cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder);
787790

788-
if builder.try_run(&mut cargo) {
791+
if builder.try_run(&mut cargo).is_ok() {
789792
// The tests succeeded; nothing to do.
790793
return;
791794
}
@@ -858,7 +861,7 @@ impl Step for RustdocTheme {
858861
util::lld_flag_no_threads(self.compiler.host.contains("windows")),
859862
);
860863
}
861-
try_run(builder, &mut cmd);
864+
try_run(builder, &mut cmd).unwrap();
862865
}
863866
}
864867

@@ -1109,7 +1112,7 @@ help: to skip test's attempt to check tidiness, pass `--exclude src/tools/tidy`
11091112
}
11101113

11111114
builder.info("tidy check");
1112-
try_run(builder, &mut cmd);
1115+
try_run(builder, &mut cmd).unwrap();
11131116

11141117
builder.ensure(ExpandYamlAnchors);
11151118

@@ -1157,7 +1160,8 @@ impl Step for ExpandYamlAnchors {
11571160
try_run(
11581161
builder,
11591162
&mut builder.tool_cmd(Tool::ExpandYamlAnchors).arg("check").arg(&builder.src),
1160-
);
1163+
)
1164+
.unwrap();
11611165
}
11621166

11631167
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -1936,7 +1940,7 @@ impl BookTest {
19361940
compiler.host,
19371941
);
19381942
let _time = util::timeit(&builder);
1939-
let toolstate = if try_run(builder, &mut rustbook_cmd) {
1943+
let toolstate = if try_run(builder, &mut rustbook_cmd).is_ok() {
19401944
ToolState::TestPass
19411945
} else {
19421946
ToolState::TestFail
@@ -2094,7 +2098,7 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) ->
20942098
cmd.arg("--test-args").arg(test_args);
20952099

20962100
if builder.config.verbose_tests {
2097-
try_run(builder, &mut cmd)
2101+
try_run(builder, &mut cmd).is_ok()
20982102
} else {
20992103
try_run_quiet(builder, &mut cmd)
21002104
}
@@ -2122,7 +2126,7 @@ impl Step for RustcGuide {
21222126

21232127
let src = builder.src.join(relative_path);
21242128
let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook);
2125-
let toolstate = if try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src)) {
2129+
let toolstate = if try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src)).is_ok() {
21262130
ToolState::TestPass
21272131
} else {
21282132
ToolState::TestFail
@@ -2661,7 +2665,7 @@ impl Step for Bootstrap {
26612665
fn run(self, builder: &Builder<'_>) {
26622666
let mut check_bootstrap = Command::new(&builder.python());
26632667
check_bootstrap.arg("bootstrap_test.py").current_dir(builder.src.join("src/bootstrap/"));
2664-
try_run(builder, &mut check_bootstrap);
2668+
try_run(builder, &mut check_bootstrap).unwrap();
26652669

26662670
let host = builder.config.build;
26672671
let compiler = builder.compiler(0, host);
@@ -2733,7 +2737,7 @@ impl Step for TierCheck {
27332737
}
27342738

27352739
builder.info("platform support check");
2736-
try_run(builder, &mut cargo.into());
2740+
try_run(builder, &mut cargo.into()).unwrap();
27372741
}
27382742
}
27392743

@@ -2813,7 +2817,7 @@ impl Step for RustInstaller {
28132817
cmd.env("CARGO", &builder.initial_cargo);
28142818
cmd.env("RUSTC", &builder.initial_rustc);
28152819
cmd.env("TMP_DIR", &tmpdir);
2816-
try_run(builder, &mut cmd);
2820+
try_run(builder, &mut cmd).unwrap();
28172821
}
28182822

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

‎src/bootstrap/tool.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ impl Step for ToolBuild {
107107
);
108108

109109
let mut cargo = Command::from(cargo);
110-
let is_expected = builder.try_run(&mut cargo);
110+
let is_expected = builder.try_run(&mut cargo).is_ok();
111111

112112
builder.save_toolstate(
113113
tool,

‎src/bootstrap/util.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ pub fn is_valid_test_suite_arg<'a, P: AsRef<Path>>(
228228
}
229229

230230
pub fn run(cmd: &mut Command, print_cmd_on_fail: bool) {
231-
if !try_run(cmd, print_cmd_on_fail) {
231+
if try_run(cmd, print_cmd_on_fail).is_err() {
232232
crate::detail_exit_macro!(1);
233233
}
234234
}

‎src/tools/build_helper/src/util.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,21 @@ pub fn fail(s: &str) -> ! {
2525
detail_exit(1, cfg!(test));
2626
}
2727

28-
pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> bool {
28+
pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> Result<(), ()> {
2929
let status = match cmd.status() {
3030
Ok(status) => status,
3131
Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", cmd, e)),
3232
};
33-
if !status.success() && print_cmd_on_fail {
34-
println!(
35-
"\n\ncommand did not execute successfully: {:?}\n\
36-
expected success, got: {}\n\n",
37-
cmd, status
38-
);
33+
if !status.success() {
34+
if print_cmd_on_fail {
35+
println!(
36+
"\n\ncommand did not execute successfully: {:?}\n\
37+
expected success, got: {}\n\n",
38+
cmd, status
39+
);
40+
}
41+
Err(())
42+
} else {
43+
Ok(())
3944
}
40-
status.success()
4145
}

‎src/tools/rustdoc-gui-test/src/main.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse
128128
}
129129
}
130130

131-
if !try_run(&mut cargo, config.verbose) {
131+
if try_run(&mut cargo, config.verbose).is_err() {
132132
eprintln!("failed to document `{}`", entry.path().display());
133133
panic!("Cannot run rustdoc-gui tests");
134134
}
@@ -161,5 +161,5 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse
161161

162162
command.args(&config.test_args);
163163

164-
if try_run(&mut command, config.verbose) { Ok(()) } else { Err(()) }
164+
try_run(&mut command, config.verbose)
165165
}

0 commit comments

Comments
 (0)