From d3cdf27184845afc2c1a9ac2d8d0c2c04015e18d Mon Sep 17 00:00:00 2001
From: jyn <github@jyn.dev>
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 0fd6b46d562..d20bfab4131 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 284efff348d..7704015fbf8 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 4b218d57727..da9d68672c4 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 48fb40d6a6d..da1960fc057 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 893195b69c2..08bc861fd21 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 <github@jyn.dev>
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 691e5ce4eb2..9795f22e2b5 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 <github@jyn.dev>
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 2141bb0ddd9..8c71b7f7fc2 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 <github@jyn.dev>
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 7704015fbf8..28e71e4130b 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 <github@jyn.dev>
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 9c4d0ea265d..1285603a63d 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, &current_toolstates));
+            t!(serde_json::to_writer(&file, &current_toolstates));
+            t!(writeln!(file)); // make sure this ends in a newline
         }
     }
 }

From fff8223584d5c55bfc974772db1856db90e878d8 Mon Sep 17 00:00:00 2001
From: jyn <github@jyn.dev>
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 28e71e4130b..da0c3767c5c 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 <github@jyn.dev>
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 d20bfab4131..8ceaadbefac 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 f5ad4f336a7..7ffd1e1f9c5 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<Kind>,
@@ -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<Kind>,
@@ -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<Kind>,
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index da0c3767c5c..c8ec9f1a8ff 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<Command>,
     libtest_args: &[&str],
     crates: &[Interned<String>],
     primary_crate: &str,
+    description: impl Into<Option<&'a str>>,
     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 <github@jyn.dev>
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 e5a710c0a96..f5ee45c6a27 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 eb1941cd889..d1190adfa19 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 <github@jyn.dev>
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 e8eebdfb5a5..a7beb2a1767 100755
--- a/src/bootstrap/configure.py
+++ b/src/bootstrap/configure.py
@@ -550,6 +550,8 @@ if __name__ == "__main__":
     # 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 @@ if __name__ == "__main__":
 
     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 d1190adfa19..8ee5ed83529 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 <github@jyn.dev>
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 7ffd1e1f9c5..32b66973567 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<Kind>,
@@ -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<Kind>,
@@ -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<Kind>,
@@ -1089,6 +1095,7 @@ impl Build {
         self.group(&msg)
     }
 
+    #[track_caller]
     fn group(&self, msg: &str) -> Option<gha::Group> {
         match self.config.dry_run {
             DryRun::SelfCheck => None,
diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs
index 915dceae389..8b3e8ca9b90 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 08bc861fd21..b70ea5ec113 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 <github@jyn.dev>
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 c8ec9f1a8ff..9ceb2714262 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 <github@jyn.dev>
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 8ceaadbefac..c793b79a97c 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 <github@jyn.dev>
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 c793b79a97c..e58f736d67f 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);
 }