From d3cdf27184845afc2c1a9ac2d8d0c2c04015e18d Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 17:17:22 -0500 Subject: [PATCH 01/13] Add even more GHA log groups This also adds a dynamic check that we don't emit nested groups, since GHA currently doesn't support them. --- src/bootstrap/doc.rs | 9 ++++++ src/bootstrap/test.rs | 51 ++++++++++++++++++++++---------- src/ci/docker/run.sh | 4 ++- src/ci/run.sh | 8 +++-- src/tools/build_helper/src/ci.rs | 13 ++++++++ 5 files changed, 67 insertions(+), 18 deletions(-) diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index 0fd6b46d562bd..d20bfab4131cc 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -1060,7 +1060,16 @@ impl Step for RustcBook { // config.toml), then this needs to explicitly update the dylib search // path. builder.add_rustc_lib_path(self.compiler, &mut cmd); + let doc_generator_guard = builder.msg( + Kind::Run, + self.compiler.stage, + "lint-docs", + self.compiler.host, + self.target, + ); builder.run(&mut cmd); + drop(doc_generator_guard); + // Run rustbook/mdbook to generate the HTML pages. builder.ensure(RustbookSrc { target: self.target, diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 284efff348d40..7704015fbf8c2 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -117,12 +117,7 @@ impl Step for CrateBootstrap { SourceType::InTree, &[], ); - builder.info(&format!( - "{} {} stage0 ({})", - builder.kind.description(), - path, - bootstrap_host, - )); + let _group = builder.msg(Kind::Test, compiler.stage, path, compiler.host, bootstrap_host); let crate_name = path.rsplit_once('/').unwrap().1; run_cargo_test(cargo, &[], &[], crate_name, compiler, bootstrap_host, builder); } @@ -163,6 +158,15 @@ You can skip linkcheck with --exclude src/tools/linkchecker" // Test the linkchecker itself. let bootstrap_host = builder.config.build; let compiler = builder.compiler(0, bootstrap_host); + + let self_test_group = builder.msg( + Kind::Test, + compiler.stage, + "linkchecker self tests", + bootstrap_host, + bootstrap_host, + ); + let cargo = tool::prepare_tool_cargo( builder, compiler, @@ -174,6 +178,7 @@ You can skip linkcheck with --exclude src/tools/linkchecker" &[], ); run_cargo_test(cargo, &[], &[], "linkchecker", compiler, bootstrap_host, builder); + drop(self_test_group); if builder.doc_tests == DocTests::No { return; @@ -182,12 +187,14 @@ You can skip linkcheck with --exclude src/tools/linkchecker" // Build all the default documentation. builder.default_doc(&[]); + // Build the linkchecker before calling `msg`, since GHA doesn't support nested groups. + let mut linkchecker = builder.tool_cmd(Tool::Linkchecker); + // Run the linkchecker. + let _guard = + builder.msg(Kind::Test, compiler.stage, "Linkcheck", bootstrap_host, bootstrap_host); let _time = util::timeit(&builder); - try_run( - builder, - builder.tool_cmd(Tool::Linkchecker).arg(builder.out.join(host.triple).join("doc")), - ); + try_run(builder, linkchecker.arg(builder.out.join(host.triple).join("doc"))); } fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { @@ -2669,6 +2676,10 @@ impl Step for Bootstrap { /// Tests the build system itself. fn run(self, builder: &Builder<'_>) { + let host = builder.config.build; + let compiler = builder.compiler(0, host); + let _guard = builder.msg(Kind::Test, 0, "bootstrap", host, host); + let mut check_bootstrap = Command::new(&builder.python()); check_bootstrap .args(["-m", "unittest", "bootstrap_test.py"]) @@ -2679,8 +2690,6 @@ impl Step for Bootstrap { // Use `python -m unittest` manually if you want to pass arguments. try_run(builder, &mut check_bootstrap); - let host = builder.config.build; - let compiler = builder.compiler(0, host); let mut cmd = Command::new(&builder.initial_cargo); cmd.arg("test") .current_dir(builder.src.join("src/bootstrap")) @@ -2748,7 +2757,13 @@ impl Step for TierCheck { cargo.arg("--verbose"); } - builder.info("platform support check"); + let _guard = builder.msg( + Kind::Test, + self.compiler.stage, + "platform support check", + self.compiler.host, + self.compiler.host, + ); try_run(builder, &mut cargo.into()); } } @@ -2796,8 +2811,6 @@ impl Step for RustInstaller { /// Ensure the version placeholder replacement tool builds fn run(self, builder: &Builder<'_>) { - builder.info("test rust-installer"); - let bootstrap_host = builder.config.build; let compiler = builder.compiler(0, bootstrap_host); let cargo = tool::prepare_tool_cargo( @@ -2810,6 +2823,14 @@ impl Step for RustInstaller { SourceType::InTree, &[], ); + + let _guard = builder.msg( + Kind::Test, + compiler.stage, + "rust-installer", + bootstrap_host, + bootstrap_host, + ); run_cargo_test(cargo, &[], &[], "installer", compiler, bootstrap_host, builder); // We currently don't support running the test.sh script outside linux(?) environments. diff --git a/src/ci/docker/run.sh b/src/ci/docker/run.sh index 4b218d5772769..da9d68672c4b7 100755 --- a/src/ci/docker/run.sh +++ b/src/ci/docker/run.sh @@ -79,7 +79,7 @@ if [ -f "$docker_dir/$image/Dockerfile" ]; then loaded_images=$(/usr/bin/timeout -k 720 600 docker load -i /tmp/rustci_docker_cache \ | sed 's/.* sha/sha/') set -e - echo "Downloaded containers:\n$loaded_images" + printf "Downloaded containers:\n$loaded_images\n" fi dockerfile="$docker_dir/$image/Dockerfile" @@ -89,12 +89,14 @@ if [ -f "$docker_dir/$image/Dockerfile" ]; then else context="$script_dir" fi + echo "::group::Building docker image for $image" retry docker \ build \ --rm \ -t rust-ci \ -f "$dockerfile" \ "$context" + echo "::endgroup::" if [ "$CI" != "" ]; then s3url="s3://$SCCACHE_BUCKET/docker/$cksum" diff --git a/src/ci/run.sh b/src/ci/run.sh index 48fb40d6a6dd8..da1960fc05712 100755 --- a/src/ci/run.sh +++ b/src/ci/run.sh @@ -154,13 +154,13 @@ fi # check for clock drifts. An HTTP URL is used instead of HTTPS since on Azure # Pipelines it happened that the certificates were marked as expired. datecheck() { - echo "== clock drift check ==" + echo "::group::Clock drift check" echo -n " local time: " date echo -n " network time: " curl -fs --head http://ci-caches.rust-lang.org | grep ^Date: \ | sed 's/Date: //g' || true - echo "== end clock drift check ==" + echo "::endgroup::" } datecheck trap datecheck EXIT @@ -177,6 +177,7 @@ retry make prepare # Display the CPU and memory information. This helps us know why the CI timing # is fluctuating. +echo "::group::Display CPU and Memory information" if isMacOS; then system_profiler SPHardwareDataType || true sysctl hw || true @@ -186,6 +187,7 @@ else cat /proc/meminfo || true ncpus=$(grep processor /proc/cpuinfo | wc -l) fi +echo "::endgroup::" if [ ! -z "$SCRIPT" ]; then echo "Executing ${SCRIPT}" @@ -218,4 +220,6 @@ if [ "$RUN_CHECK_WITH_PARALLEL_QUERIES" != "" ]; then CARGO_INCREMENTAL=0 ../x check fi +echo "::group::sccache stats" sccache --show-stats || true +echo "::endgroup::" diff --git a/src/tools/build_helper/src/ci.rs b/src/tools/build_helper/src/ci.rs index 893195b69c219..08bc861fd2195 100644 --- a/src/tools/build_helper/src/ci.rs +++ b/src/tools/build_helper/src/ci.rs @@ -36,6 +36,10 @@ impl CiEnv { } pub mod gha { + use std::sync::atomic::{AtomicBool, Ordering}; + + static GROUP_ACTIVE: AtomicBool = AtomicBool::new(false); + /// All github actions log messages from this call to the Drop of the return value /// will be grouped and hidden by default in logs. Note that nesting these does /// not really work. @@ -45,6 +49,11 @@ pub mod gha { } else { eprintln!("{name}") } + // https://github.com/actions/toolkit/issues/1001 + assert!( + !GROUP_ACTIVE.swap(true, Ordering::Relaxed), + "nested groups are not supported by GHA!" + ); Group(()) } @@ -57,6 +66,10 @@ pub mod gha { if std::env::var_os("GITHUB_ACTIONS").is_some() { eprintln!("::endgroup::"); } + assert!( + GROUP_ACTIVE.swap(false, Ordering::Relaxed), + "group dropped but no group active!" + ); } } } From 2b3db1cd5a406c8f66942c1926f28f836296affb Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 18:59:23 -0500 Subject: [PATCH 02/13] Don't nest GHA groups in `check::Std` --- src/bootstrap/check.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 691e5ce4eb26d..9795f22e2b5d1 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -135,6 +135,7 @@ impl Step for Std { let hostdir = builder.sysroot_libdir(compiler, compiler.host); add_to_sysroot(&builder, &libdir, &hostdir, &libstd_stamp(builder, compiler, target)); } + drop(_guard); // don't run on std twice with x.py clippy // don't check test dependencies if we haven't built libtest From fb3ac44dd839010b0bb91fdb76d42c6b01b300de Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 18:59:48 -0500 Subject: [PATCH 03/13] Don't checkout the LLVM submodule in `x dist --dry-run` We don't actually need it and it's quite slow. --- src/bootstrap/dist.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index 2141bb0ddd9ba..8c71b7f7fc22f 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -902,7 +902,9 @@ impl Step for Src { /// Creates the `rust-src` installer component fn run(self, builder: &Builder<'_>) -> GeneratedTarball { - builder.update_submodule(&Path::new("src/llvm-project")); + if !builder.config.dry_run() { + builder.update_submodule(&Path::new("src/llvm-project")); + } let tarball = Tarball::new_targetless(builder, "rust-src"); From df5cc59a68b8af5a45c657edef29a6f1d10a5a2e Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 20:28:47 -0500 Subject: [PATCH 04/13] fix nested GHA groups (redux) --- src/bootstrap/test.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 7704015fbf8c2..28e71e4130b82 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1893,14 +1893,6 @@ impl Step for BookTest { /// /// This uses the `rustdoc` that sits next to `compiler`. fn run(self, builder: &Builder<'_>) { - let host = self.compiler.host; - let _guard = builder.msg( - Kind::Test, - self.compiler.stage, - &format!("book {}", self.name), - host, - host, - ); // External docs are different from local because: // - Some books need pre-processing by mdbook before being tested. // - They need to save their state to toolstate. @@ -1943,7 +1935,7 @@ impl BookTest { let _guard = builder.msg( Kind::Test, compiler.stage, - format_args!("rustbook {}", self.path.display()), + format_args!("mdbook {}", self.path.display()), compiler.host, compiler.host, ); @@ -1959,8 +1951,12 @@ impl BookTest { /// This runs `rustdoc --test` on all `.md` files in the path. fn run_local_doc(self, builder: &Builder<'_>) { let compiler = self.compiler; + let host = self.compiler.host; - builder.ensure(compile::Std::new(compiler, compiler.host)); + builder.ensure(compile::Std::new(compiler, host)); + + let _guard = + builder.msg(Kind::Test, compiler.stage, &format!("book {}", self.name), host, host); // Do a breadth-first traversal of the `src/doc` directory and just run // tests for all files that end in `*.md` From a5de56a95eb9d49e945e68f736fa7599ef8b5482 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 21:03:01 -0500 Subject: [PATCH 05/13] Make sure toolstates.json ends in a newline This avoids the following broken logging in CI: ``` {"book":"test-pass","reference":"test-pass","rustbook":"test-fail","rust-by-example":"test-pass","nomicon":"test-pass","embedded-book":"test-pass","edition-guide":"test-pass"}::group::Building bootstrap ``` --- src/bootstrap/toolstate.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs index 9c4d0ea265ddf..1285603a63da5 100644 --- a/src/bootstrap/toolstate.rs +++ b/src/bootstrap/toolstate.rs @@ -262,6 +262,8 @@ impl Builder<'_> { /// `rust.save-toolstates` in `config.toml`. If unspecified, nothing will be /// done. The file is updated immediately after this function completes. pub fn save_toolstate(&self, tool: &str, state: ToolState) { + use std::io::Write; + // If we're in a dry run setting we don't want to save toolstates as // that means if we e.g. panic down the line it'll look like we tested // everything (but we actually haven't). @@ -286,7 +288,8 @@ impl Builder<'_> { current_toolstates.insert(tool.into(), state); t!(file.seek(SeekFrom::Start(0))); t!(file.set_len(0)); - t!(serde_json::to_writer(file, ¤t_toolstates)); + t!(serde_json::to_writer(&file, ¤t_toolstates)); + t!(writeln!(file)); // make sure this ends in a newline } } } From fff8223584d5c55bfc974772db1856db90e878d8 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 21:26:11 -0500 Subject: [PATCH 06/13] Add GHA log groups for tool tests --- src/bootstrap/test.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 28e71e4130b82..da0c3767c5ce2 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -413,6 +413,7 @@ impl Step for RustAnalyzer { cargo.env("SKIP_SLOW_TESTS", "1"); cargo.add_rustc_lib_path(builder, compiler); + builder.msg_sysroot_tool(Kind::Test, compiler.stage, "rust-analyzer", host, host); run_cargo_test(cargo, &[], &[], "rust-analyzer", compiler, host, builder); } } @@ -462,6 +463,7 @@ impl Step for Rustfmt { cargo.add_rustc_lib_path(builder, compiler); + builder.msg_sysroot_tool(Kind::Test, compiler.stage, "rustfmt", host, host); run_cargo_test(cargo, &[], &[], "rustfmt", compiler, host, builder); } } @@ -554,6 +556,13 @@ impl Miri { cargo.env("RUST_BACKTRACE", "1"); let mut cargo = Command::from(cargo); + let _guard = builder.msg( + Kind::Build, + compiler.stage + 1, + "miri sysroot", + compiler.host, + compiler.host, + ); builder.run(&mut cargo); // # Determine where Miri put its sysroot. @@ -631,6 +640,8 @@ impl Step for Miri { SourceType::InTree, &[], ); + let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "miri", host, host); + cargo.add_rustc_lib_path(builder, compiler); // miri tests need to know about the stage sysroot @@ -799,6 +810,8 @@ impl Step for Clippy { cargo.env("BLESS", "Gesundheit"); } + builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host); + if builder.try_run(&mut cargo).is_ok() { // The tests succeeded; nothing to do. return; @@ -1049,6 +1062,13 @@ impl Step for RustdocGUI { } let _time = util::timeit(&builder); + let _guard = builder.msg_sysroot_tool( + Kind::Test, + self.compiler.stage, + "rustdoc-gui", + self.compiler.host, + self.target, + ); crate::render_tests::try_run_tests(builder, &mut cmd, true); } } From 26cdf7566cba986045b856d26c253df1150bff93 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 21:59:27 -0500 Subject: [PATCH 07/13] Add must_use to `msg_` functions This caught several places which weren't waiting until the command finished to drop the Group. I also took the liberty of calling `msg_sysroot_tool` from `run_cargo_test` to reduce code duplication and make errors like this less likely in the future. --- src/bootstrap/doc.rs | 8 ++-- src/bootstrap/lib.rs | 6 +++ src/bootstrap/test.rs | 98 +++++++++++++++++++++++++------------------ 3 files changed, 67 insertions(+), 45 deletions(-) diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index d20bfab4131cc..8ceaadbefacf5 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -221,7 +221,7 @@ impl Step for TheBook { let shared_assets = builder.ensure(SharedAssets { target }); // build the redirect pages - builder.msg_doc(compiler, "book redirect pages", target); + let _guard = builder.msg_doc(compiler, "book redirect pages", target); for file in t!(fs::read_dir(builder.src.join(&relative_path).join("redirects"))) { let file = t!(file); let path = file.path(); @@ -305,7 +305,7 @@ impl Step for Standalone { fn run(self, builder: &Builder<'_>) { let target = self.target; let compiler = self.compiler; - builder.msg_doc(compiler, "standalone", target); + let _guard = builder.msg_doc(compiler, "standalone", target); let out = builder.doc_out(target); t!(fs::create_dir_all(&out)); @@ -799,8 +799,6 @@ macro_rules! tool_doc { SourceType::Submodule }; - builder.msg_doc(compiler, stringify!($tool).to_lowercase(), target); - // Symlink compiler docs to the output directory of rustdoc documentation. let out_dirs = [ builder.stage_out(compiler, Mode::ToolRustc).join(target.triple).join("doc"), @@ -839,6 +837,8 @@ macro_rules! tool_doc { cargo.rustdocflag("--show-type-layout"); cargo.rustdocflag("--generate-link-to-definition"); cargo.rustdocflag("-Zunstable-options"); + + let _guard = builder.msg_doc(compiler, stringify!($tool).to_lowercase(), target); builder.run(&mut cargo.into()); } } diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index f5ad4f336a7e3..7ffd1e1f9c57f 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -999,6 +999,7 @@ impl Build { } } + #[must_use = "Groups should not be dropped until the Step finishes running"] fn msg_check( &self, what: impl Display, @@ -1007,6 +1008,7 @@ impl Build { self.msg(Kind::Check, self.config.stage, what, self.config.build, target) } + #[must_use = "Groups should not be dropped until the Step finishes running"] fn msg_doc( &self, compiler: Compiler, @@ -1016,6 +1018,7 @@ impl Build { self.msg(Kind::Doc, compiler.stage, what, compiler.host, target.into()) } + #[must_use = "Groups should not be dropped until the Step finishes running"] fn msg_build( &self, compiler: Compiler, @@ -1028,6 +1031,7 @@ impl Build { /// Return a `Group` guard for a [`Step`] that is built for each `--stage`. /// /// [`Step`]: crate::builder::Step + #[must_use = "Groups should not be dropped until the Step finishes running"] fn msg( &self, action: impl Into, @@ -1054,6 +1058,7 @@ impl Build { /// Return a `Group` guard for a [`Step`] that is only built once and isn't affected by `--stage`. /// /// [`Step`]: crate::builder::Step + #[must_use = "Groups should not be dropped until the Step finishes running"] fn msg_unstaged( &self, action: impl Into, @@ -1065,6 +1070,7 @@ impl Build { self.group(&msg) } + #[must_use = "Groups should not be dropped until the Step finishes running"] fn msg_sysroot_tool( &self, action: impl Into, diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index da0c3767c5ce2..c8ec9f1a8ffc5 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -117,9 +117,8 @@ impl Step for CrateBootstrap { SourceType::InTree, &[], ); - let _group = builder.msg(Kind::Test, compiler.stage, path, compiler.host, bootstrap_host); let crate_name = path.rsplit_once('/').unwrap().1; - run_cargo_test(cargo, &[], &[], crate_name, compiler, bootstrap_host, builder); + run_cargo_test(cargo, &[], &[], crate_name, crate_name, compiler, bootstrap_host, builder); } } @@ -159,14 +158,6 @@ You can skip linkcheck with --exclude src/tools/linkchecker" let bootstrap_host = builder.config.build; let compiler = builder.compiler(0, bootstrap_host); - let self_test_group = builder.msg( - Kind::Test, - compiler.stage, - "linkchecker self tests", - bootstrap_host, - bootstrap_host, - ); - let cargo = tool::prepare_tool_cargo( builder, compiler, @@ -177,8 +168,16 @@ You can skip linkcheck with --exclude src/tools/linkchecker" SourceType::InTree, &[], ); - run_cargo_test(cargo, &[], &[], "linkchecker", compiler, bootstrap_host, builder); - drop(self_test_group); + run_cargo_test( + cargo, + &[], + &[], + "linkchecker", + "linkchecker self tests", + compiler, + bootstrap_host, + builder, + ); if builder.doc_tests == DocTests::No { return; @@ -413,8 +412,7 @@ impl Step for RustAnalyzer { cargo.env("SKIP_SLOW_TESTS", "1"); cargo.add_rustc_lib_path(builder, compiler); - builder.msg_sysroot_tool(Kind::Test, compiler.stage, "rust-analyzer", host, host); - run_cargo_test(cargo, &[], &[], "rust-analyzer", compiler, host, builder); + run_cargo_test(cargo, &[], &[], "rust-analyzer", "rust-analyzer", compiler, host, builder); } } @@ -463,8 +461,7 @@ impl Step for Rustfmt { cargo.add_rustc_lib_path(builder, compiler); - builder.msg_sysroot_tool(Kind::Test, compiler.stage, "rustfmt", host, host); - run_cargo_test(cargo, &[], &[], "rustfmt", compiler, host, builder); + run_cargo_test(cargo, &[], &[], "rustfmt", "rustfmt", compiler, host, builder); } } @@ -512,7 +509,16 @@ impl Step for RustDemangler { cargo.env("RUST_DEMANGLER_DRIVER_PATH", rust_demangler); cargo.add_rustc_lib_path(builder, compiler); - run_cargo_test(cargo, &[], &[], "rust-demangler", compiler, host, builder); + run_cargo_test( + cargo, + &[], + &[], + "rust-demangler", + "rust-demangler", + compiler, + host, + builder, + ); } } @@ -754,7 +760,16 @@ impl Step for CompiletestTest { &[], ); cargo.allow_features("test"); - run_cargo_test(cargo, &[], &[], "compiletest", compiler, host, builder); + run_cargo_test( + cargo, + &[], + &[], + "compiletest", + "compiletest self test", + compiler, + host, + builder, + ); } } @@ -810,7 +825,7 @@ impl Step for Clippy { cargo.env("BLESS", "Gesundheit"); } - builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host); + let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host); if builder.try_run(&mut cargo).is_ok() { // The tests succeeded; nothing to do. @@ -2201,11 +2216,12 @@ impl Step for CrateLibrustc { /// Given a `cargo test` subcommand, add the appropriate flags and run it. /// /// Returns whether the test succeeded. -fn run_cargo_test( +fn run_cargo_test<'a>( cargo: impl Into, libtest_args: &[&str], crates: &[Interned], primary_crate: &str, + description: impl Into>, compiler: Compiler, target: TargetSelection, builder: &Builder<'_>, @@ -2213,6 +2229,9 @@ fn run_cargo_test( let mut cargo = prepare_cargo_test(cargo, libtest_args, crates, primary_crate, compiler, target, builder); let _time = util::timeit(&builder); + let _group = description.into().and_then(|what| { + builder.msg_sysroot_tool(Kind::Test, compiler.stage, what, compiler.host, target) + }); #[cfg(feature = "build-metrics")] builder.metrics.begin_test_suite( @@ -2378,14 +2397,16 @@ impl Step for Crate { _ => panic!("can only test libraries"), }; - let _guard = builder.msg( - builder.kind, - compiler.stage, - crate_description(&self.crates), - compiler.host, + run_cargo_test( + cargo, + &[], + &self.crates, + &self.crates[0], + &*crate_description(&self.crates), + compiler, target, + builder, ); - run_cargo_test(cargo, &[], &self.crates, &self.crates[0], compiler, target, builder); } } @@ -2478,18 +2499,12 @@ impl Step for CrateRustdoc { dylib_path.insert(0, PathBuf::from(&*libdir)); cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap()); - let _guard = builder.msg_sysroot_tool( - builder.kind, - compiler.stage, - "rustdoc", - compiler.host, - target, - ); run_cargo_test( cargo, &[], &[INTERNER.intern_str("rustdoc:0.0.0")], "rustdoc", + "rustdoc", compiler, target, builder, @@ -2545,13 +2560,12 @@ impl Step for CrateRustdocJsonTypes { &[] }; - let _guard = - builder.msg(builder.kind, compiler.stage, "rustdoc-json-types", compiler.host, target); run_cargo_test( cargo, libtest_args, &[INTERNER.intern_str("rustdoc-json-types")], "rustdoc-json-types", + "rustdoc-json-types", compiler, target, builder, @@ -2722,7 +2736,7 @@ impl Step for Bootstrap { } // rustbuild tests are racy on directory creation so just run them one at a time. // Since there's not many this shouldn't be a problem. - run_cargo_test(cmd, &["--test-threads=1"], &[], "bootstrap", compiler, host, builder); + run_cargo_test(cmd, &["--test-threads=1"], &[], "bootstrap", None, compiler, host, builder); } fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { @@ -2840,14 +2854,16 @@ impl Step for RustInstaller { &[], ); - let _guard = builder.msg( - Kind::Test, - compiler.stage, + run_cargo_test( + cargo, + &[], + &[], + "installer", "rust-installer", + compiler, bootstrap_host, - bootstrap_host, + builder, ); - run_cargo_test(cargo, &[], &[], "installer", compiler, bootstrap_host, builder); // We currently don't support running the test.sh script outside linux(?) environments. // Eventually this should likely migrate to #[test]s in rust-installer proper rather than a From dcd8d376cbe7d8dc40cb6d810a10914f98a868c2 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 22:12:39 -0500 Subject: [PATCH 08/13] don't print download progress in CI --- src/bootstrap/bootstrap.py | 2 +- src/bootstrap/download.rs | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index e5a710c0a9646..f5ee45c6a27d1 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -97,7 +97,7 @@ def _download(path, url, probably_big, verbose, exception): print("downloading {}".format(url), file=sys.stderr) try: - if probably_big or verbose: + if (probably_big or verbose) and "GITHUB_ACTIONS" not in os.environ: option = "-#" else: option = "-s" diff --git a/src/bootstrap/download.rs b/src/bootstrap/download.rs index eb1941cd889c4..d1190adfa19a5 100644 --- a/src/bootstrap/download.rs +++ b/src/bootstrap/download.rs @@ -7,7 +7,7 @@ use std::{ process::{Command, Stdio}, }; -use build_helper::util::try_run; +use build_helper::{util::try_run, ci::CiEnv}; use once_cell::sync::OnceCell; use xz2::bufread::XzDecoder; @@ -213,7 +213,6 @@ impl Config { // Try curl. If that fails and we are on windows, fallback to PowerShell. let mut curl = Command::new("curl"); curl.args(&[ - "-#", "-y", "30", "-Y", @@ -224,6 +223,12 @@ impl Config { "3", "-SRf", ]); + // Don't print progress in CI; the \r wrapping looks bad and downloads don't take long enough for progress to be useful. + if CiEnv::is_ci() { + curl.arg("-s"); + } else { + curl.arg("--progress-bar"); + } curl.arg(url); let f = File::create(tempfile).unwrap(); curl.stdout(Stdio::from(f)); From 9851a141a32a4a878d868a8187a7d2c8f69d7dbd Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 22:14:48 -0500 Subject: [PATCH 09/13] put configure behind a group --- src/bootstrap/configure.py | 4 ++++ src/bootstrap/download.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/configure.py b/src/bootstrap/configure.py index e8eebdfb5a581..a7beb2a176759 100755 --- a/src/bootstrap/configure.py +++ b/src/bootstrap/configure.py @@ -550,6 +550,8 @@ def quit_if_file_exists(file): # If 'config.toml' already exists, exit the script at this point quit_if_file_exists('config.toml') + if "GITHUB_ACTIONS" in os.environ: + print("::group::Configure the build") p("processing command line") # Parse all known arguments into a configuration structure that reflects the # TOML we're going to write out @@ -572,3 +574,5 @@ def quit_if_file_exists(file): p("") p("run `python {}/x.py --help`".format(rust_dir)) + if "GITHUB_ACTIONS" in os.environ: + print("::endgroup::") diff --git a/src/bootstrap/download.rs b/src/bootstrap/download.rs index d1190adfa19a5..8ee5ed83529a5 100644 --- a/src/bootstrap/download.rs +++ b/src/bootstrap/download.rs @@ -7,7 +7,7 @@ use std::{ process::{Command, Stdio}, }; -use build_helper::{util::try_run, ci::CiEnv}; +use build_helper::{ci::CiEnv, util::try_run}; use once_cell::sync::OnceCell; use xz2::bufread::XzDecoder; From 3e306c2ddb743491147e0a36fc531692d9c0d30d Mon Sep 17 00:00:00 2001 From: jyn Date: Fri, 14 Jul 2023 17:32:05 -0500 Subject: [PATCH 10/13] Add `track_caller` to builder.msg this makes the panics on nested GHA groups more useful --- src/bootstrap/lib.rs | 7 +++++++ src/bootstrap/tool.rs | 1 + src/tools/build_helper/src/ci.rs | 1 + 3 files changed, 9 insertions(+) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 7ffd1e1f9c57f..32b6697356777 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -1000,6 +1000,7 @@ impl Build { } #[must_use = "Groups should not be dropped until the Step finishes running"] + #[track_caller] fn msg_check( &self, what: impl Display, @@ -1009,6 +1010,7 @@ impl Build { } #[must_use = "Groups should not be dropped until the Step finishes running"] + #[track_caller] fn msg_doc( &self, compiler: Compiler, @@ -1019,6 +1021,7 @@ impl Build { } #[must_use = "Groups should not be dropped until the Step finishes running"] + #[track_caller] fn msg_build( &self, compiler: Compiler, @@ -1032,6 +1035,7 @@ impl Build { /// /// [`Step`]: crate::builder::Step #[must_use = "Groups should not be dropped until the Step finishes running"] + #[track_caller] fn msg( &self, action: impl Into, @@ -1059,6 +1063,7 @@ impl Build { /// /// [`Step`]: crate::builder::Step #[must_use = "Groups should not be dropped until the Step finishes running"] + #[track_caller] fn msg_unstaged( &self, action: impl Into, @@ -1071,6 +1076,7 @@ impl Build { } #[must_use = "Groups should not be dropped until the Step finishes running"] + #[track_caller] fn msg_sysroot_tool( &self, action: impl Into, @@ -1089,6 +1095,7 @@ impl Build { self.group(&msg) } + #[track_caller] fn group(&self, msg: &str) -> Option { match self.config.dry_run { DryRun::SelfCheck => None, diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 915dceae3893a..8b3e8ca9b908e 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -34,6 +34,7 @@ struct ToolBuild { } impl Builder<'_> { + #[track_caller] fn msg_tool( &self, mode: Mode, diff --git a/src/tools/build_helper/src/ci.rs b/src/tools/build_helper/src/ci.rs index 08bc861fd2195..b70ea5ec11390 100644 --- a/src/tools/build_helper/src/ci.rs +++ b/src/tools/build_helper/src/ci.rs @@ -43,6 +43,7 @@ pub mod gha { /// All github actions log messages from this call to the Drop of the return value /// will be grouped and hidden by default in logs. Note that nesting these does /// not really work. + #[track_caller] pub fn group(name: impl std::fmt::Display) -> Group { if std::env::var_os("GITHUB_ACTIONS").is_some() { eprintln!("::group::{name}"); From ce843aa24cf8dc6a26746f86179d8453b1b76426 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 9 Jul 2023 22:57:17 -0500 Subject: [PATCH 11/13] add a couple more groups - group rustdoc-js-std - group rust-installer/test.sh --- src/bootstrap/test.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index c8ec9f1a8ffc5..9ceb2714262ea 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -939,6 +939,13 @@ impl Step for RustdocJSStd { builder, DocumentationFormat::HTML, )); + let _guard = builder.msg( + Kind::Test, + builder.top_stage, + "rustdoc-js-std", + builder.config.build, + self.target, + ); builder.run(&mut command); } } @@ -2854,16 +2861,14 @@ impl Step for RustInstaller { &[], ); - run_cargo_test( - cargo, - &[], - &[], - "installer", + let _guard = builder.msg( + Kind::Test, + compiler.stage, "rust-installer", - compiler, bootstrap_host, - builder, + bootstrap_host, ); + run_cargo_test(cargo, &[], &[], "installer", None, compiler, bootstrap_host, builder); // We currently don't support running the test.sh script outside linux(?) environments. // Eventually this should likely migrate to #[test]s in rust-installer proper rather than a From 02ae14c9725eae0919007b16a0e48422e3ba9715 Mon Sep 17 00:00:00 2001 From: jyn Date: Thu, 13 Jul 2023 00:41:00 -0500 Subject: [PATCH 12/13] fix another GHA log panic --- src/bootstrap/doc.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index 8ceaadbefacf5..c793b79a97c13 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -220,6 +220,9 @@ impl Step for TheBook { // build the version info page and CSS let shared_assets = builder.ensure(SharedAssets { target }); + // build the command first so we don't nest GHA groups + builder.rustdoc_cmd(compiler); + // build the redirect pages let _guard = builder.msg_doc(compiler, "book redirect pages", target); for file in t!(fs::read_dir(builder.src.join(&relative_path).join("redirects"))) { From 3a0caed188df8abe1a60d50100df69d17b1fb798 Mon Sep 17 00:00:00 2001 From: jyn Date: Fri, 14 Jul 2023 17:34:27 -0500 Subject: [PATCH 13/13] fix another nesting issue --- src/bootstrap/doc.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index c793b79a97c13..e58f736d67f56 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -566,10 +566,6 @@ fn doc_std( let compiler = builder.compiler(stage, builder.config.build); - let description = - format!("library{} in {} format", crate_description(&requested_crates), format.as_str()); - let _guard = builder.msg_doc(compiler, &description, target); - let target_doc_dir_name = if format == DocumentationFormat::JSON { "json-doc" } else { "doc" }; let target_dir = builder.stage_out(compiler, Mode::Std).join(target.triple).join(target_doc_dir_name); @@ -606,6 +602,10 @@ fn doc_std( cargo.arg("-p").arg(krate); } + let description = + format!("library{} in {} format", crate_description(&requested_crates), format.as_str()); + let _guard = builder.msg_doc(compiler, &description, target); + builder.run(&mut cargo.into()); builder.cp_r(&out_dir, &out); }