From 616469aa8aa84585bbaeaee0c9e5b9e06ec15149 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Jun 2023 15:50:20 +0200 Subject: [PATCH 1/3] Fix rustdoc-gui tester --- src/tools/rustdoc-gui-test/src/main.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/tools/rustdoc-gui-test/src/main.rs b/src/tools/rustdoc-gui-test/src/main.rs index dc902f8cb0242..ed91763420a59 100644 --- a/src/tools/rustdoc-gui-test/src/main.rs +++ b/src/tools/rustdoc-gui-test/src/main.rs @@ -67,7 +67,7 @@ fn find_librs>(path: P) -> Option { None } -fn main() { +fn main() -> Result<(), ()> { let config = Arc::new(Config::from_args(env::args().collect())); // The goal here is to check if the necessary packages are installed, and if not, we @@ -128,7 +128,10 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse } } - try_run(&mut cargo, config.verbose); + if !try_run(&mut cargo, config.verbose) { + eprintln!("failed to document `{}`", entry.path().display()); + panic!("Cannot run rustdoc-gui tests"); + } } } @@ -158,5 +161,5 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse command.args(&config.test_args); - try_run(&mut command, config.verbose); + if try_run(&mut command, config.verbose) { Ok(()) } else { Err(()) } } From d70faf3645d826bd8b0d76107d7b54db639fcec9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Jun 2023 15:50:30 +0200 Subject: [PATCH 2/3] Fix failing rustdoc GUI test --- tests/rustdoc-gui/search-result-color.goml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/rustdoc-gui/search-result-color.goml b/tests/rustdoc-gui/search-result-color.goml index 193a3eb3fd134..e3c628b366fbd 100644 --- a/tests/rustdoc-gui/search-result-color.goml +++ b/tests/rustdoc-gui/search-result-color.goml @@ -31,7 +31,7 @@ define-function: ( // color of the typename (struct, module, method, ...) before search results assert-css: ( ".result-name .typename", - {"color": "#888"}, + {"color": |grey|}, ALL, ) }, @@ -75,6 +75,7 @@ store-value: (entry_color, "#0096cf") // color of the search entry store-value: (hover_entry_color, "#fff") // color of the hovered/focused search entry store-value: (background_color, "transparent") // background color store-value: (hover_background_color, "#3c3c3c") // hover background color +store-value: (grey, "#999") call-function: ( "check-result-color", ( @@ -186,6 +187,7 @@ store-value: (entry_color, "#ddd") // color of the search entry store-value: (hover_entry_color, "#ddd") // color of the hovered/focused search entry store-value: (background_color, "transparent") // background color store-value: (hover_background_color, "#616161") // hover background color +store-value: (grey, "#ccc") call-function: ( "check-result-color", ( @@ -282,6 +284,7 @@ store-value: (entry_color, "#000") // color of the search entry store-value: (hover_entry_color, "#000") // color of the hovered/focused search entry store-value: (background_color, "transparent") // background color store-value: (hover_background_color, "#ccc") // hover background color +store-value: (grey, "#999") call-function: ( "check-result-color", ( From 7b5577985df7e7f3132c1e8db1eb6e1022f31276 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Jun 2023 16:33:59 +0200 Subject: [PATCH 3/3] Make `try_run` return a `Result<(), ()>` instead of a boolean --- src/bootstrap/download.rs | 22 ++++++++------- src/bootstrap/lib.rs | 14 ++++++---- src/bootstrap/run.rs | 11 ++++---- src/bootstrap/test.rs | 38 ++++++++++++++------------ src/bootstrap/tool.rs | 2 +- src/bootstrap/util.rs | 2 +- src/tools/build_helper/src/util.rs | 20 ++++++++------ src/tools/rustdoc-gui-test/src/main.rs | 4 +-- 8 files changed, 63 insertions(+), 50 deletions(-) diff --git a/src/bootstrap/download.rs b/src/bootstrap/download.rs index 06f479808b975..cb40521dda763 100644 --- a/src/bootstrap/download.rs +++ b/src/bootstrap/download.rs @@ -54,9 +54,9 @@ impl Config { /// Runs a command, printing out nice contextual information if it fails. /// Exits if the command failed to execute at all, otherwise returns its /// `status.success()`. - pub(crate) fn try_run(&self, cmd: &mut Command) -> bool { + pub(crate) fn try_run(&self, cmd: &mut Command) -> Result<(), ()> { if self.dry_run() { - return true; + return Ok(()); } self.verbose(&format!("running: {:?}", cmd)); try_run(cmd, self.is_verbose()) @@ -156,12 +156,14 @@ impl Config { ]; } "; - nix_build_succeeded = self.try_run(Command::new("nix-build").args(&[ - Path::new("-E"), - Path::new(NIX_EXPR), - Path::new("-o"), - &nix_deps_dir, - ])); + nix_build_succeeded = self + .try_run(Command::new("nix-build").args(&[ + Path::new("-E"), + Path::new(NIX_EXPR), + Path::new("-o"), + &nix_deps_dir, + ])) + .is_ok(); nix_deps_dir }); if !nix_build_succeeded { @@ -186,7 +188,7 @@ impl Config { patchelf.args(&["--set-interpreter", dynamic_linker.trim_end()]); } - self.try_run(patchelf.arg(fname)); + self.try_run(patchelf.arg(fname)).unwrap(); } fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) { @@ -237,7 +239,7 @@ impl Config { "(New-Object System.Net.WebClient).DownloadFile('{}', '{}')", url, tempfile.to_str().expect("invalid UTF-8 not supported with powershell downloads"), ), - ])) { + ])).is_err() { return; } eprintln!("\nspurious failure, trying again"); diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index d7e77aeb338f6..d389b568f563d 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -333,7 +333,7 @@ forward! { create(path: &Path, s: &str), remove(f: &Path), tempdir() -> PathBuf, - try_run(cmd: &mut Command) -> bool, + try_run(cmd: &mut Command) -> Result<(), ()>, llvm_link_shared() -> bool, download_rustc() -> bool, initial_rustfmt() -> Option, @@ -614,11 +614,13 @@ impl Build { } // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error). - let has_local_modifications = !self.try_run( - Command::new("git") - .args(&["diff-index", "--quiet", "HEAD"]) - .current_dir(&absolute_path), - ); + let has_local_modifications = self + .try_run( + Command::new("git") + .args(&["diff-index", "--quiet", "HEAD"]) + .current_dir(&absolute_path), + ) + .is_err(); if has_local_modifications { self.run(Command::new("git").args(&["stash", "push"]).current_dir(&absolute_path)); } diff --git a/src/bootstrap/run.rs b/src/bootstrap/run.rs index ec01f744b8250..c97b759273717 100644 --- a/src/bootstrap/run.rs +++ b/src/bootstrap/run.rs @@ -27,7 +27,8 @@ 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<'_> { @@ -39,17 +40,17 @@ impl Step for ExpandYamlAnchors { } } -fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool { +fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> Result<(), ()> { if !builder.fail_fast { - if !builder.try_run(cmd) { + if let Err(e) = builder.try_run(cmd) { let mut failures = builder.delayed_failures.borrow_mut(); failures.push(format!("{:?}", cmd)); - return false; + return Err(e); } } else { builder.run(cmd); } - true + Ok(()) } #[derive(Debug, PartialOrd, Ord, Copy, Clone, Hash, PartialEq, Eq)] diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 9212362f6c9e1..873ed61daf33c 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -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) -> bool { +fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> Result<(), ()> { if !builder.fail_fast { - if !builder.try_run(cmd) { + if let Err(e) = builder.try_run(cmd) { let mut failures = builder.delayed_failures.borrow_mut(); failures.push(format!("{:?}", cmd)); - return false; + return Err(e); } } else { builder.run(cmd); } - true + Ok(()) } fn try_run_quiet(builder: &Builder<'_>, cmd: &mut Command) -> bool { @@ -187,7 +187,8 @@ 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<'_> { @@ -240,7 +241,8 @@ 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))); + try_run(builder, builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target))) + .unwrap(); } } @@ -286,7 +288,8 @@ impl Step for Cargotest { .args(builder.config.test_args()) .env("RUSTC", builder.rustc(compiler)) .env("RUSTDOC", builder.rustdoc(compiler)), - ); + ) + .unwrap(); } } @@ -785,7 +788,7 @@ impl Step for Clippy { cargo.add_rustc_lib_path(builder, compiler); let mut cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder); - if builder.try_run(&mut cargo) { + if builder.try_run(&mut cargo).is_ok() { // The tests succeeded; nothing to do. return; } @@ -858,7 +861,7 @@ impl Step for RustdocTheme { util::lld_flag_no_threads(self.compiler.host.contains("windows")), ); } - try_run(builder, &mut cmd); + try_run(builder, &mut cmd).unwrap(); } } @@ -1109,7 +1112,7 @@ help: to skip test's attempt to check tidiness, pass `--exclude src/tools/tidy` } builder.info("tidy check"); - try_run(builder, &mut cmd); + try_run(builder, &mut cmd).unwrap(); builder.ensure(ExpandYamlAnchors); @@ -1157,7 +1160,8 @@ 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<'_> { @@ -1936,7 +1940,7 @@ impl BookTest { compiler.host, ); let _time = util::timeit(&builder); - let toolstate = if try_run(builder, &mut rustbook_cmd) { + let toolstate = if try_run(builder, &mut rustbook_cmd).is_ok() { ToolState::TestPass } else { ToolState::TestFail @@ -2094,7 +2098,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) + try_run(builder, &mut cmd).is_ok() } else { try_run_quiet(builder, &mut cmd) } @@ -2122,7 +2126,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)) { + let toolstate = if try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src)).is_ok() { ToolState::TestPass } else { ToolState::TestFail @@ -2661,7 +2665,7 @@ impl Step for Bootstrap { fn run(self, builder: &Builder<'_>) { let mut check_bootstrap = Command::new(&builder.python()); check_bootstrap.arg("bootstrap_test.py").current_dir(builder.src.join("src/bootstrap/")); - try_run(builder, &mut check_bootstrap); + try_run(builder, &mut check_bootstrap).unwrap(); let host = builder.config.build; let compiler = builder.compiler(0, host); @@ -2733,7 +2737,7 @@ impl Step for TierCheck { } builder.info("platform support check"); - try_run(builder, &mut cargo.into()); + try_run(builder, &mut cargo.into()).unwrap(); } } @@ -2813,7 +2817,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); + try_run(builder, &mut cmd).unwrap(); } fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 96341b69df046..126f0b1eb31a4 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -107,7 +107,7 @@ impl Step for ToolBuild { ); let mut cargo = Command::from(cargo); - let is_expected = builder.try_run(&mut cargo); + let is_expected = builder.try_run(&mut cargo).is_ok(); builder.save_toolstate( tool, diff --git a/src/bootstrap/util.rs b/src/bootstrap/util.rs index 1ec49f80d632e..b291584b300ce 100644 --- a/src/bootstrap/util.rs +++ b/src/bootstrap/util.rs @@ -228,7 +228,7 @@ pub fn is_valid_test_suite_arg<'a, P: AsRef>( } pub fn run(cmd: &mut Command, print_cmd_on_fail: bool) { - if !try_run(cmd, print_cmd_on_fail) { + if try_run(cmd, print_cmd_on_fail).is_err() { crate::detail_exit_macro!(1); } } diff --git a/src/tools/build_helper/src/util.rs b/src/tools/build_helper/src/util.rs index 731095023a96e..11b8a228b8a8e 100644 --- a/src/tools/build_helper/src/util.rs +++ b/src/tools/build_helper/src/util.rs @@ -25,17 +25,21 @@ pub fn fail(s: &str) -> ! { detail_exit(1, cfg!(test)); } -pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> bool { +pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> Result<(), ()> { let status = match cmd.status() { Ok(status) => status, Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}", cmd, e)), }; - if !status.success() && print_cmd_on_fail { - println!( - "\n\ncommand did not execute successfully: {:?}\n\ - expected success, got: {}\n\n", - cmd, status - ); + if !status.success() { + if print_cmd_on_fail { + println!( + "\n\ncommand did not execute successfully: {:?}\n\ + expected success, got: {}\n\n", + cmd, status + ); + } + Err(()) + } else { + Ok(()) } - status.success() } diff --git a/src/tools/rustdoc-gui-test/src/main.rs b/src/tools/rustdoc-gui-test/src/main.rs index ed91763420a59..0ddd2c66cf9ea 100644 --- a/src/tools/rustdoc-gui-test/src/main.rs +++ b/src/tools/rustdoc-gui-test/src/main.rs @@ -128,7 +128,7 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse } } - if !try_run(&mut cargo, config.verbose) { + if try_run(&mut cargo, config.verbose).is_err() { eprintln!("failed to document `{}`", entry.path().display()); panic!("Cannot run rustdoc-gui tests"); } @@ -161,5 +161,5 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse command.args(&config.test_args); - if try_run(&mut command, config.verbose) { Ok(()) } else { Err(()) } + try_run(&mut command, config.verbose) }