Group entire build steps in the gha logs

9 files changed, 190 insertions, 177 deletions

diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py
index e6788ee6fee..680a8da6adf 100644
--- a/src/bootstrap/bootstrap.py
+++ b/src/bootstrap/bootstrap.py
@@ -722,11 +722,14 @@ class RustBuild(object):
 
     def build_bootstrap(self, color, verbose_count):
         """Build bootstrap"""
-        print("Building bootstrap")
+        env = os.environ.copy()
+        if "GITHUB_ACTIONS" in env:
+            print("::group::Building bootstrap")
+        else:
+            print("Building bootstrap")
         build_dir = os.path.join(self.build_dir, "bootstrap")
         if self.clean and os.path.exists(build_dir):
             shutil.rmtree(build_dir)
-        env = os.environ.copy()
         # `CARGO_BUILD_TARGET` breaks bootstrap build.
         # See also: <https://github.com/rust-lang/rust/issues/70208>.
         if "CARGO_BUILD_TARGET" in env:
@@ -798,6 +801,9 @@ class RustBuild(object):
         # Run this from the source directory so cargo finds .cargo/config
         run(args, env=env, verbose=self.verbose, cwd=self.rust_root)
 
+        if "GITHUB_ACTIONS" in env:
+            print("::endgroup::")
+
     def build_triple(self):
         """Build triple as in LLVM
 
diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs
index 44efc502e39..60de46ce64c 100644
--- a/src/bootstrap/check.rs
+++ b/src/bootstrap/check.rs
@@ -105,15 +105,7 @@ impl Step for Std {
             cargo.arg("--lib");
         }
 
-        let msg = if compiler.host == target {
-            format!("Checking stage{} library artifacts ({target})", builder.top_stage)
-        } else {
-            format!(
-                "Checking stage{} library artifacts ({} -> {})",
-                builder.top_stage, &compiler.host, target
-            )
-        };
-        builder.info(&msg);
+        let _guard = builder.msg_check("library artifacts", target);
         run_cargo(
             builder,
             cargo,
@@ -167,18 +159,7 @@ impl Step for Std {
             cargo.arg("-p").arg(krate.name);
         }
 
-        let msg = if compiler.host == target {
-            format!(
-                "Checking stage{} library test/bench/example targets ({target})",
-                builder.top_stage
-            )
-        } else {
-            format!(
-                "Checking stage{} library test/bench/example targets ({} -> {})",
-                builder.top_stage, &compiler.host, target
-            )
-        };
-        builder.info(&msg);
+        let _guard = builder.msg_check("library test/bench/example targets", target);
         run_cargo(
             builder,
             cargo,
@@ -252,15 +233,7 @@ impl Step for Rustc {
             cargo.arg("-p").arg(krate.name);
         }
 
-        let msg = if compiler.host == target {
-            format!("Checking stage{} compiler artifacts ({target})", builder.top_stage)
-        } else {
-            format!(
-                "Checking stage{} compiler artifacts ({} -> {})",
-                builder.top_stage, &compiler.host, target
-            )
-        };
-        builder.info(&msg);
+        let _guard = builder.msg_check("compiler artifacts", target);
         run_cargo(
             builder,
             cargo,
@@ -317,15 +290,7 @@ impl Step for CodegenBackend {
             .arg(builder.src.join(format!("compiler/rustc_codegen_{}/Cargo.toml", backend)));
         rustc_cargo_env(builder, &mut cargo, target, compiler.stage);
 
-        let msg = if compiler.host == target {
-            format!("Checking stage{} {} artifacts ({target})", builder.top_stage, backend)
-        } else {
-            format!(
-                "Checking stage{} {} library ({} -> {})",
-                builder.top_stage, backend, &compiler.host.triple, target.triple
-            )
-        };
-        builder.info(&msg);
+        let _guard = builder.msg_check(&backend, target);
 
         run_cargo(
             builder,
@@ -385,15 +350,7 @@ impl Step for RustAnalyzer {
             cargo.arg("--benches");
         }
 
-        let msg = if compiler.host == target {
-            format!("Checking stage{} {} artifacts ({target})", compiler.stage, "rust-analyzer")
-        } else {
-            format!(
-                "Checking stage{} {} artifacts ({} -> {})",
-                compiler.stage, "rust-analyzer", &compiler.host.triple, target.triple
-            )
-        };
-        builder.info(&msg);
+        let _guard = builder.msg_check("rust-analyzer artifacts", target);
         run_cargo(
             builder,
             cargo,
@@ -460,18 +417,7 @@ macro_rules! tool_check_step {
                 // NOTE: this doesn't enable lints for any other tools unless they explicitly add `#![warn(rustc::internal)]`
                 // See https://github.com/rust-lang/rust/pull/80573#issuecomment-754010776
                 cargo.rustflag("-Zunstable-options");
-                let msg = if compiler.host == target {
-                    format!("Checking stage{} {} artifacts ({target})", builder.top_stage, stringify!($name).to_lowercase())
-                } else {
-                    format!(
-                        "Checking stage{} {} artifacts ({} -> {})",
-                        builder.top_stage,
-                        stringify!($name).to_lowercase(),
-                        &compiler.host.triple,
-                        target.triple
-                    )
-                };
-                builder.info(&msg);
+                let _guard = builder.msg_check(&concat!(stringify!($name), " artifacts").to_lowercase(), target);
                 run_cargo(
                     builder,
                     cargo,
diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs
index 8af5927e27f..51c1cab7001 100644
--- a/src/bootstrap/compile.rs
+++ b/src/bootstrap/compile.rs
@@ -143,23 +143,13 @@ impl Step for Std {
             cargo.arg("-p").arg(krate);
         }
 
-        let msg = if compiler.host == target {
-            format!(
-                "Building{} stage{} library artifacts ({}) ",
-                crate_description(&self.crates),
-                compiler.stage,
-                compiler.host
-            )
-        } else {
-            format!(
-                "Building{} stage{} library artifacts ({} -> {})",
-                crate_description(&self.crates),
-                compiler.stage,
-                compiler.host,
-                target,
-            )
-        };
-        builder.info(&msg);
+        let _guard = builder.msg(
+            Kind::Build,
+            compiler.stage,
+            format_args!("library artifacts{}", crate_description(&self.crates)),
+            compiler.host,
+            target,
+        );
         run_cargo(
             builder,
             cargo,
@@ -790,24 +780,13 @@ impl Step for Rustc {
             cargo.arg("-p").arg(krate);
         }
 
-        let msg = if compiler.host == target {
-            format!(
-                "Building{} compiler artifacts (stage{} -> stage{})",
-                crate_description(&self.crates),
-                compiler.stage,
-                compiler.stage + 1
-            )
-        } else {
-            format!(
-                "Building{} compiler artifacts (stage{}:{} -> stage{}:{})",
-                crate_description(&self.crates),
-                compiler.stage,
-                compiler.host,
-                compiler.stage + 1,
-                target,
-            )
-        };
-        builder.info(&msg);
+        let _guard = builder.msg_sysroot_tool(
+            Kind::Build,
+            compiler.stage,
+            format_args!("compiler artifacts{}", crate_description(&self.crates)),
+            compiler.host,
+            target,
+        );
         run_cargo(
             builder,
             cargo,
@@ -1114,15 +1093,7 @@ impl Step for CodegenBackend {
 
         let tmp_stamp = out_dir.join(".tmp.stamp");
 
-        let msg = if compiler.host == target {
-            format!("Building stage{} codegen backend {}", compiler.stage, backend)
-        } else {
-            format!(
-                "Building stage{} codegen backend {} ({} -> {})",
-                compiler.stage, backend, compiler.host, target
-            )
-        };
-        builder.info(&msg);
+        let _guard = builder.msg_build(compiler, format_args!("codegen backend {backend}"), target);
         let files = run_cargo(builder, cargo, vec![], &tmp_stamp, vec![], false, false);
         if builder.config.dry_run() {
             return;
diff --git a/src/bootstrap/install.rs b/src/bootstrap/install.rs
index 42d895a3413..b62aa999246 100644
--- a/src/bootstrap/install.rs
+++ b/src/bootstrap/install.rs
@@ -12,7 +12,7 @@ use crate::util::t;
 
 use crate::dist;
 use crate::tarball::GeneratedTarball;
-use crate::Compiler;
+use crate::{Compiler, Kind};
 
 use crate::builder::{Builder, RunConfig, ShouldRun, Step};
 use crate::config::{Config, TargetSelection};
@@ -52,7 +52,7 @@ fn install_sh(
     host: Option<TargetSelection>,
     tarball: &GeneratedTarball,
 ) {
-    builder.info(&format!("Install {} stage{} ({:?})", package, stage, host));
+    let _guard = builder.msg(Kind::Install, stage, package, host, host);
 
     let prefix = default_path(&builder.config.prefix, "/usr/local");
     let sysconfdir = prefix.join(default_path(&builder.config.sysconfdir, "/etc"));
diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs
index bfdb029951f..238d167c4c2 100644
--- a/src/bootstrap/lib.rs
+++ b/src/bootstrap/lib.rs
@@ -19,13 +19,14 @@
 use std::cell::{Cell, RefCell};
 use std::collections::{HashMap, HashSet};
 use std::env;
+use std::fmt::Display;
 use std::fs::{self, File};
 use std::io;
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 use std::str;
 
-use build_helper::ci::CiEnv;
+use build_helper::ci::{gha, CiEnv};
 use channel::GitInfo;
 use config::{DryRun, Target};
 use filetime::FileTime;
@@ -993,6 +994,85 @@ impl Build {
         }
     }
 
+    fn msg_check(
+        &self,
+        what: impl Display,
+        target: impl Into<Option<TargetSelection>>,
+    ) -> Option<gha::Group> {
+        self.msg(Kind::Check, self.config.stage, what, self.config.build, target)
+    }
+
+    fn msg_build(
+        &self,
+        compiler: Compiler,
+        what: impl Display,
+        target: impl Into<Option<TargetSelection>>,
+    ) -> Option<gha::Group> {
+        self.msg(Kind::Build, compiler.stage, what, compiler.host, target)
+    }
+
+    /// Return a `Group` guard for a [`Step`] that is built for each `--stage`.
+    fn msg(
+        &self,
+        action: impl Into<Kind>,
+        stage: u32,
+        what: impl Display,
+        host: impl Into<Option<TargetSelection>>,
+        target: impl Into<Option<TargetSelection>>,
+    ) -> Option<gha::Group> {
+        let action = action.into();
+        let msg = |fmt| format!("{action:?}ing stage{stage} {what}{fmt}");
+        let msg = if let Some(target) = target.into() {
+            let host = host.into().unwrap();
+            if host == target {
+                msg(format_args!(" ({target})"))
+            } else {
+                msg(format_args!(" ({host} -> {target})"))
+            }
+        } else {
+            msg(format_args!(""))
+        };
+        self.group(&msg)
+    }
+
+    /// Return a `Group` guard for a [`Step`] that is only built once and isn't affected by `--stage`.
+    fn msg_unstaged(
+        &self,
+        action: impl Into<Kind>,
+        what: impl Display,
+        target: TargetSelection,
+    ) -> Option<gha::Group> {
+        let action = action.into();
+        let msg = format!("{action:?}ing {what} for {target}");
+        self.group(&msg)
+    }
+
+    fn msg_sysroot_tool(
+        &self,
+        action: impl Into<Kind>,
+        stage: u32,
+        what: impl Display,
+        host: TargetSelection,
+        target: TargetSelection,
+    ) -> Option<gha::Group> {
+        let action = action.into();
+        let msg = |fmt| format!("{action:?}ing {what} {fmt}");
+        let msg = if host == target {
+            msg(format_args!("(stage{stage} -> stage{}, {target})", stage + 1))
+        } else {
+            msg(format_args!("(stage{stage}:{host} -> stage{}:{target})", stage + 1))
+        };
+        self.group(&msg)
+    }
+
+    fn group(&self, msg: &str) -> Option<gha::Group> {
+        self.info(&msg);
+        match self.config.dry_run {
+            DryRun::SelfCheck => None,
+            DryRun::Disabled | DryRun::UserSelected => Some(gha::group(&msg)),
+        }
+    }
+
     /// Returns the number of parallel jobs that have been configured for this
     /// build.
     fn jobs(&self) -> u32 {
diff --git a/src/bootstrap/llvm.rs b/src/bootstrap/llvm.rs
index cfc74186313..67cb8837391 100644
--- a/src/bootstrap/llvm.rs
+++ b/src/bootstrap/llvm.rs
@@ -21,7 +21,7 @@ use crate::channel;
 use crate::config::{Config, TargetSelection};
 use crate::util::get_clang_cl_resource_dir;
 use crate::util::{self, exe, output, t, up_to_date};
-use crate::{CLang, GitRepo};
+use crate::{CLang, GitRepo, Kind};
 
 use build_helper::ci::CiEnv;
 
@@ -271,7 +271,7 @@ impl Step for Llvm {
             panic!("shared linking to LLVM is not currently supported on {}", target.triple);
         }
 
-        builder.info(&format!("Building LLVM for {}", target));
+        let _guard = builder.msg_unstaged(Kind::Build, "LLVM", target);
         t!(stamp.remove());
         let _time = util::timeit(&builder);
         t!(fs::create_dir_all(&out_dir));
@@ -813,7 +813,7 @@ impl Step for Lld {
             return out_dir;
         }
 
-        builder.info(&format!("Building LLD for {}", target));
+        let _guard = builder.msg_unstaged(Kind::Build, "LLD", target);
         let _time = util::timeit(&builder);
         t!(fs::create_dir_all(&out_dir));
 
@@ -911,7 +911,7 @@ impl Step for Sanitizers {
             return runtimes;
         }
 
-        builder.info(&format!("Building sanitizers for {}", self.target));
+        let _guard = builder.msg_unstaged(Kind::Build, "sanitizers", self.target);
         t!(stamp.remove());
         let _time = util::timeit(&builder);
 
@@ -1103,7 +1103,7 @@ impl Step for CrtBeginEnd {
             return out_dir;
         }
 
-        builder.info("Building crtbegin.o and crtend.o");
+        let _guard = builder.msg_unstaged(Kind::Build, "crtbegin.o and crtend.o", self.target);
         t!(fs::create_dir_all(&out_dir));
 
         let mut cfg = cc::Build::new();
@@ -1168,7 +1168,7 @@ impl Step for Libunwind {
             return out_dir;
         }
 
-        builder.info(&format!("Building libunwind.a for {}", self.target.triple));
+        let _guard = builder.msg_unstaged(Kind::Build, "libunwind.a", self.target);
         t!(fs::create_dir_all(&out_dir));
 
         let mut cc_cfg = cc::Build::new();
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index ccf83974b8c..601351ea8e3 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -5,7 +5,6 @@
 
 use std::env;
 use std::ffi::OsString;
-use std::fmt;
 use std::fs;
 use std::iter;
 use std::path::{Path, PathBuf};
@@ -57,12 +56,12 @@ impl TestKind {
     }
 }
 
-impl fmt::Display for TestKind {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(match *self {
-            TestKind::Test => "Testing",
-            TestKind::Bench => "Benchmarking",
-        })
+impl Into<Kind> for TestKind {
+    fn into(self) -> Kind {
+        match self {
+            TestKind::Test => Kind::Test,
+            TestKind::Bench => Kind::Bench,
+        }
     }
 }
 
@@ -1905,7 +1904,13 @@ impl BookTest {
         rustbook_cmd.env("RUSTC_BOOTSTRAP", "1");
         rustbook_cmd.env("PATH", new_path).arg("test").arg(path);
         builder.add_rust_test_threads(&mut rustbook_cmd);
-        builder.info(&format!("Testing rustbook {}", self.path.display()));
+        let _guard = builder.msg(
+            Kind::Test,
+            compiler.stage,
+            format_args!("rustbook {}", self.path.display()),
+            compiler.host,
+            compiler.host,
+        );
         let _time = util::timeit(&builder);
         let toolstate = if try_run(builder, &mut rustbook_cmd) {
             ToolState::TestPass
@@ -2033,7 +2038,8 @@ impl Step for ErrorIndex {
         let mut tool = tool::ErrorIndex::command(builder);
         tool.arg("markdown").arg(&output);
 
-        builder.info(&format!("Testing error-index stage{}", compiler.stage));
+        let _guard =
+            builder.msg(Kind::Test, compiler.stage, "error-index", compiler.host, compiler.host);
         let _time = util::timeit(&builder);
         builder.run_quiet(&mut tool);
         // The tests themselves need to link to std, so make sure it is
@@ -2263,14 +2269,13 @@ impl Step for Crate {
             );
         }
 
-        builder.info(&format!(
-            "{}{} stage{} ({} -> {})",
+        let _guard = builder.msg(
             test_kind,
-            crate_description(&self.crates),
             compiler.stage,
-            &compiler.host,
-            target
-        ));
+            crate_description(&self.crates),
+            compiler.host,
+            target,
+        );
         let _time = util::timeit(&builder);
         crate::render_tests::try_run_tests(builder, &mut cargo.into());
     }
@@ -2386,10 +2391,8 @@ impl Step for CrateRustdoc {
             cargo.arg("--quiet");
         }
 
-        builder.info(&format!(
-            "{} rustdoc stage{} ({} -> {})",
-            test_kind, compiler.stage, &compiler.host, target
-        ));
+        let _guard = builder.msg(test_kind, compiler.stage, "rustdoc", compiler.host, target);
+
         let _time = util::timeit(&builder);
 
         add_flags_and_try_run_tests(builder, &mut cargo.into());
@@ -2453,10 +2456,8 @@ impl Step for CrateRustdocJsonTypes {
             cargo.arg("'-Ctarget-feature=-crt-static'");
         }
 
-        builder.info(&format!(
-            "{} rustdoc-json-types stage{} ({} -> {})",
-            test_kind, compiler.stage, &compiler.host, target
-        ));
+        let _guard =
+            builder.msg(test_kind, compiler.stage, "rustdoc-json-types", compiler.host, target);
         let _time = util::timeit(&builder);
 
         add_flags_and_try_run_tests(builder, &mut cargo.into());
@@ -2845,7 +2846,7 @@ impl Step for TestHelpers {
             return;
         }
 
-        builder.info("Building test helpers");
+        let _guard = builder.msg_unstaged(Kind::Build, "test helpers", target);
         t!(fs::create_dir_all(&dst));
         let mut cfg = cc::Build::new();
         // FIXME: Workaround for https://github.com/emscripten-core/emscripten/issues/9013
diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs
index 79eec6f848c..5faef76fafe 100644
--- a/src/bootstrap/tool.rs
+++ b/src/bootstrap/tool.rs
@@ -11,6 +11,7 @@ use crate::toolstate::ToolState;
 use crate::util::{add_dylib_path, exe, t};
 use crate::Compiler;
 use crate::Mode;
+use crate::{gha, Kind};
 
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
 pub enum SourceType {
@@ -32,41 +33,27 @@ struct ToolBuild {
     allow_features: &'static str,
 }
 
-fn tooling_output(
-    mode: Mode,
-    tool: &str,
-    build_stage: u32,
-    host: &TargetSelection,
-    target: &TargetSelection,
-) -> String {
-    match mode {
-        // depends on compiler stage, different to host compiler
-        Mode::ToolRustc => {
-            if host == target {
-                format!("Building tool {} (stage{} -> stage{})", tool, build_stage, build_stage + 1)
-            } else {
-                format!(
-                    "Building tool {} (stage{}:{} -> stage{}:{})",
-                    tool,
-                    build_stage,
-                    host,
-                    build_stage + 1,
-                    target
-                )
-            }
-        }
-        // doesn't depend on compiler, same as host compiler
-        Mode::ToolStd => {
-            if host == target {
-                format!("Building tool {} (stage{})", tool, build_stage)
-            } else {
-                format!(
-                    "Building tool {} (stage{}:{} -> stage{}:{})",
-                    tool, build_stage, host, build_stage, target
-                )
-            }
+impl Builder<'_> {
+    fn msg_tool(
+        &self,
+        mode: Mode,
+        tool: &str,
+        build_stage: u32,
+        host: &TargetSelection,
+        target: &TargetSelection,
+    ) -> Option<gha::Group> {
+        match mode {
+            // depends on compiler stage, different to host compiler
+            Mode::ToolRustc => self.msg_sysroot_tool(
+                Kind::Build,
+                build_stage,
+                format_args!("tool {tool}"),
+                *host,
+                *target,
+            ),
+            // doesn't depend on compiler, same as host compiler
+            _ => self.msg(Kind::Build, build_stage, format_args!("tool {tool}"), *host, *target),
         }
-        _ => format!("Building tool {} (stage{})", tool, build_stage),
     }
 }
 
@@ -111,14 +98,13 @@ impl Step for ToolBuild {
         if !self.allow_features.is_empty() {
             cargo.allow_features(self.allow_features);
         }
-        let msg = tooling_output(
+        let _guard = builder.msg_tool(
             self.mode,
             self.tool,
             self.compiler.stage,
             &self.compiler.host,
             &self.target,
         );
-        builder.info(&msg);
 
         let mut cargo = Command::from(cargo);
         let is_expected = builder.try_run(&mut cargo);
@@ -492,14 +478,13 @@ impl Step for Rustdoc {
             cargo.rustflag("--cfg=parallel_compiler");
         }
 
-        let msg = tooling_output(
+        let _guard = builder.msg_tool(
             Mode::ToolRustc,
             "rustdoc",
             build_compiler.stage,
             &self.compiler.host,
             &target,
         );
-        builder.info(&msg);
         builder.run(&mut cargo.into());
 
         // Cargo adds a number of paths to the dylib search path on windows, which results in
diff --git a/src/tools/build_helper/src/ci.rs b/src/tools/build_helper/src/ci.rs
index 9f113c72b93..d2e9c324af8 100644
--- a/src/tools/build_helper/src/ci.rs
+++ b/src/tools/build_helper/src/ci.rs
@@ -38,3 +38,27 @@ impl CiEnv {
         }
     }
 }
+
+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.
+    pub fn group(name: impl std::fmt::Display) -> Group {
+        if std::env::var_os("GITHUB_ACTIONS").is_some() {
+            eprintln!("::group::{name}");
+        }
+        Group(())
+    }
+
+    /// A guard that closes the current github actions log group on drop.
+    #[must_use]
+    pub struct Group(());
+
+    impl Drop for Group {
+        fn drop(&mut self) {
+            if std::env::var_os("GITHUB_ACTIONS").is_some() {
+                eprintln!("::endgroup::");
+            }
+        }
+    }
+}