Auto merge of #113514 - jyn514:more-gha-groups, r=oli-obk

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.

r? `@oli-obk`
This commit is contained in:
bors 2023-07-15 13:52:04 +00:00
commit 4d6e4260b2
13 changed files with 184 additions and 65 deletions

View File

@ -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"

View File

@ -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

View File

@ -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::")

View File

@ -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");

View File

@ -220,8 +220,11 @@ 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
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 +308,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));
@ -563,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);
@ -603,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);
}
@ -799,8 +802,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 +840,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());
}
}
@ -1060,7 +1063,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,

View File

@ -7,7 +7,7 @@ use std::{
process::{Command, Stdio},
};
use build_helper::util::try_run;
use build_helper::{ci::CiEnv, util::try_run};
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));

View File

@ -999,6 +999,8 @@ impl Build {
}
}
#[must_use = "Groups should not be dropped until the Step finishes running"]
#[track_caller]
fn msg_check(
&self,
what: impl Display,
@ -1007,6 +1009,8 @@ 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"]
#[track_caller]
fn msg_doc(
&self,
compiler: Compiler,
@ -1016,6 +1020,8 @@ 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"]
#[track_caller]
fn msg_build(
&self,
compiler: Compiler,
@ -1028,6 +1034,8 @@ 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"]
#[track_caller]
fn msg(
&self,
action: impl Into<Kind>,
@ -1054,6 +1062,8 @@ 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"]
#[track_caller]
fn msg_unstaged(
&self,
action: impl Into<Kind>,
@ -1065,6 +1075,8 @@ impl Build {
self.group(&msg)
}
#[must_use = "Groups should not be dropped until the Step finishes running"]
#[track_caller]
fn msg_sysroot_tool(
&self,
action: impl Into<Kind>,
@ -1083,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,

View File

@ -117,14 +117,8 @@ impl Step for CrateBootstrap {
SourceType::InTree,
&[],
);
builder.info(&format!(
"{} {} stage0 ({})",
builder.kind.description(),
path,
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);
}
}
@ -163,6 +157,7 @@ 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 cargo = tool::prepare_tool_cargo(
builder,
compiler,
@ -173,7 +168,16 @@ You can skip linkcheck with --exclude src/tools/linkchecker"
SourceType::InTree,
&[],
);
run_cargo_test(cargo, &[], &[], "linkchecker", compiler, bootstrap_host, builder);
run_cargo_test(
cargo,
&[],
&[],
"linkchecker",
"linkchecker self tests",
compiler,
bootstrap_host,
builder,
);
if builder.doc_tests == DocTests::No {
return;
@ -182,12 +186,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<'_> {
@ -406,7 +412,7 @@ impl Step for RustAnalyzer {
cargo.env("SKIP_SLOW_TESTS", "1");
cargo.add_rustc_lib_path(builder, compiler);
run_cargo_test(cargo, &[], &[], "rust-analyzer", compiler, host, builder);
run_cargo_test(cargo, &[], &[], "rust-analyzer", "rust-analyzer", compiler, host, builder);
}
}
@ -455,7 +461,7 @@ impl Step for Rustfmt {
cargo.add_rustc_lib_path(builder, compiler);
run_cargo_test(cargo, &[], &[], "rustfmt", compiler, host, builder);
run_cargo_test(cargo, &[], &[], "rustfmt", "rustfmt", compiler, host, builder);
}
}
@ -503,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,
);
}
}
@ -547,6 +562,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.
@ -624,6 +646,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
@ -736,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,
);
}
}
@ -792,6 +825,8 @@ impl Step for Clippy {
cargo.env("BLESS", "Gesundheit");
}
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.
return;
@ -904,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);
}
}
@ -1042,6 +1084,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);
}
}
@ -1886,14 +1935,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.
@ -1936,7 +1977,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,
);
@ -1952,8 +1993,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`
@ -2178,11 +2223,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<'_>,
@ -2190,6 +2236,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(
@ -2355,14 +2404,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);
}
}
@ -2455,18 +2506,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,
@ -2522,13 +2567,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,
@ -2669,6 +2713,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 +2727,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"))
@ -2697,7 +2743,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<'_> {
@ -2748,7 +2794,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 +2848,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,7 +2860,15 @@ impl Step for RustInstaller {
SourceType::InTree,
&[],
);
run_cargo_test(cargo, &[], &[], "installer", compiler, bootstrap_host, builder);
let _guard = builder.msg(
Kind::Test,
compiler.stage,
"rust-installer",
bootstrap_host,
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

View File

@ -34,6 +34,7 @@ struct ToolBuild {
}
impl Builder<'_> {
#[track_caller]
fn msg_tool(
&self,
mode: Mode,

View File

@ -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
}
}
}

View File

@ -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"

View File

@ -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::"

View File

@ -36,15 +36,25 @@ 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.
#[track_caller]
pub fn group(name: impl std::fmt::Display) -> Group {
if std::env::var_os("GITHUB_ACTIONS").is_some() {
eprintln!("::group::{name}");
} 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 +67,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!"
);
}
}
}