Skip to content

Commit

Permalink
Auto merge of #113738 - jyn514:rollup-mjcya4c, r=jyn514
Browse files Browse the repository at this point in the history
Rollup of 3 pull requests

Successful merges:

 - #113643 (bootstrap: Clean up try_run)
 - #113731 (Remove unused `bootstrap::util::CiEnv` enum)
 - #113737 (update mailmap for myself)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jul 16, 2023
2 parents 4124617 + df729c2 commit 2c718d1
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 91 deletions.
6 changes: 5 additions & 1 deletion .mailmap
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,14 @@ Joseph T. Lyons <[email protected]> <[email protected]>
Josh Cotton <[email protected]>
Josh Driver <[email protected]>
Josh Holmer <[email protected]>
Joshua Nelson <[email protected]> <[email protected]>
Julian Knodt <[email protected]>
jumbatm <[email protected]> <[email protected]>
Junyoung Cho <[email protected]>
Jynn Nelson <[email protected]> <[email protected]>
Jynn Nelson <[email protected]> <[email protected]>
Jynn Nelson <[email protected]> <[email protected]>
Jynn Nelson <[email protected]> <[email protected]>
Jynn Nelson <[email protected]>
Jyun-Yan You <[email protected]> <[email protected]>
Kalita Alexey <[email protected]>
Kampfkarren <[email protected]>
Expand Down
12 changes: 12 additions & 0 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1742,6 +1742,18 @@ impl Config {
}
}

/// Runs a command, printing out nice contextual information if it fails.
/// Exits if the command failed to execute at all, otherwise returns its
/// `status.success()`.
#[deprecated = "use `Builder::try_run` instead where possible"]
pub(crate) fn try_run(&self, cmd: &mut Command) -> Result<(), ()> {
if self.dry_run() {
return Ok(());
}
self.verbose(&format!("running: {:?}", cmd));
build_helper::util::try_run(cmd, self.is_verbose())
}

/// A git invocation which runs inside the source directory.
///
/// Use this rather than `Command::new("git")` in order to support out-of-tree builds.
Expand Down
33 changes: 15 additions & 18 deletions src/bootstrap/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{
process::{Command, Stdio},
};

use build_helper::{ci::CiEnv, util::try_run};
use build_helper::ci::CiEnv;
use once_cell::sync::OnceCell;
use xz2::bufread::XzDecoder;

Expand All @@ -21,6 +21,12 @@ use crate::{

static SHOULD_FIX_BINS_AND_DYLIBS: OnceCell<bool> = OnceCell::new();

/// `Config::try_run` wrapper for this module to avoid warnings on `try_run`, since we don't have access to a `builder` yet.
fn try_run(config: &Config, cmd: &mut Command) -> Result<(), ()> {
#[allow(deprecated)]
config.try_run(cmd)
}

/// Generic helpers that are useful anywhere in bootstrap.
impl Config {
pub fn is_verbose(&self) -> bool {
Expand Down Expand Up @@ -51,17 +57,6 @@ impl Config {
tmp
}

/// Runs a command, printing out nice contextual information if it fails.
/// Exits if the command failed to execute at all, otherwise returns its
/// `status.success()`.
pub(crate) fn try_run(&self, cmd: &mut Command) -> Result<(), ()> {
if self.dry_run() {
return Ok(());
}
self.verbose(&format!("running: {:?}", cmd));
try_run(cmd, self.is_verbose())
}

/// Runs a command, printing out nice contextual information if it fails.
/// Returns false if do not execute at all, otherwise returns its
/// `status.success()`.
Expand Down Expand Up @@ -156,14 +151,16 @@ impl Config {
];
}
";
nix_build_succeeded = self
.try_run(Command::new("nix-build").args(&[
nix_build_succeeded = try_run(
self,
Command::new("nix-build").args(&[
Path::new("-E"),
Path::new(NIX_EXPR),
Path::new("-o"),
&nix_deps_dir,
]))
.is_ok();
]),
)
.is_ok();
nix_deps_dir
});
if !nix_build_succeeded {
Expand All @@ -188,7 +185,7 @@ impl Config {
patchelf.args(&["--set-interpreter", dynamic_linker.trim_end()]);
}

let _ = self.try_run(patchelf.arg(fname));
let _ = try_run(self, patchelf.arg(fname));
}

fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) {
Expand Down Expand Up @@ -236,7 +233,7 @@ impl Config {
if self.build.contains("windows-msvc") {
eprintln!("Fallback to PowerShell");
for _ in 0..3 {
if self.try_run(Command::new("PowerShell.exe").args(&[
if try_run(self, Command::new("PowerShell.exe").args(&[
"/nologo",
"-Command",
"[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12;",
Expand Down
33 changes: 29 additions & 4 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@ forward! {
create(path: &Path, s: &str),
remove(f: &Path),
tempdir() -> PathBuf,
try_run(cmd: &mut Command) -> Result<(), ()>,
llvm_link_shared() -> bool,
download_rustc() -> bool,
initial_rustfmt() -> Option<PathBuf>,
Expand Down Expand Up @@ -617,7 +616,9 @@ impl Build {
}

// Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error).
#[allow(deprecated)] // diff-index reports the modifications through the exit status
let has_local_modifications = self
.config
.try_run(
Command::new("git")
.args(&["diff-index", "--quiet", "HEAD"])
Expand Down Expand Up @@ -971,12 +972,36 @@ impl Build {
/// Runs a command, printing out nice contextual information if it fails.
/// Exits if the command failed to execute at all, otherwise returns its
/// `status.success()`.
fn try_run_quiet(&self, cmd: &mut Command) -> bool {
fn run_quiet_delaying_failure(&self, cmd: &mut Command) -> bool {
if self.config.dry_run() {
return true;
}
self.verbose(&format!("running: {:?}", cmd));
try_run_suppressed(cmd)
if !self.fail_fast {
self.verbose(&format!("running: {:?}", cmd));
if !try_run_suppressed(cmd) {
let mut failures = self.delayed_failures.borrow_mut();
failures.push(format!("{:?}", cmd));
return false;
}
} else {
self.run_quiet(cmd);
}
true
}

/// Runs a command, printing out contextual info if it fails, and delaying errors until the build finishes.
pub(crate) fn run_delaying_failure(&self, cmd: &mut Command) -> bool {
if !self.fail_fast {
#[allow(deprecated)] // can't use Build::try_run, that's us
if self.config.try_run(cmd).is_err() {
let mut failures = self.delayed_failures.borrow_mut();
failures.push(format!("{:?}", cmd));
return false;
}
} else {
self.run(cmd);
}
true
}

pub fn is_verbose_than(&self, level: usize) -> bool {
Expand Down
16 changes: 1 addition & 15 deletions src/bootstrap/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ impl Step for ExpandYamlAnchors {
/// anchors in them, since GitHub Actions doesn't support them.
fn run(self, builder: &Builder<'_>) {
builder.info("Expanding YAML anchors in the GitHub Actions configuration");
try_run(
builder,
builder.run_delaying_failure(
&mut builder.tool_cmd(Tool::ExpandYamlAnchors).arg("generate").arg(&builder.src),
);
}
Expand All @@ -39,19 +38,6 @@ impl Step for ExpandYamlAnchors {
}
}

fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
if !builder.fail_fast {
if builder.try_run(cmd).is_err() {
let mut failures = builder.delayed_failures.borrow_mut();
failures.push(format!("{:?}", cmd));
return false;
}
} else {
builder.run(cmd);
}
true
}

#[derive(Debug, PartialOrd, Ord, Copy, Clone, Hash, PartialEq, Eq)]
pub struct BuildManifest;

Expand Down
59 changes: 17 additions & 42 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,32 +48,6 @@ const MIR_OPT_BLESS_TARGET_MAPPING: &[(&str, &str)] = &[
// build for, so there is no entry for "aarch64-apple-darwin" here.
];

fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
if !builder.fail_fast {
if builder.try_run(cmd).is_err() {
let mut failures = builder.delayed_failures.borrow_mut();
failures.push(format!("{:?}", cmd));
return false;
}
} else {
builder.run(cmd);
}
true
}

fn try_run_quiet(builder: &Builder<'_>, cmd: &mut Command) -> bool {
if !builder.fail_fast {
if !builder.try_run_quiet(cmd) {
let mut failures = builder.delayed_failures.borrow_mut();
failures.push(format!("{:?}", cmd));
return false;
}
} else {
builder.run_quiet(cmd);
}
true
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct CrateBootstrap {
path: Interned<PathBuf>,
Expand Down Expand Up @@ -193,7 +167,7 @@ You can skip linkcheck with --exclude src/tools/linkchecker"
let _guard =
builder.msg(Kind::Test, compiler.stage, "Linkcheck", bootstrap_host, bootstrap_host);
let _time = util::timeit(&builder);
try_run(builder, linkchecker.arg(builder.out.join(host.triple).join("doc")));
builder.run_delaying_failure(linkchecker.arg(builder.out.join(host.triple).join("doc")));
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand Down Expand Up @@ -246,7 +220,9 @@ impl Step for HtmlCheck {
builder.default_doc(&[]);
builder.ensure(crate::doc::Rustc::new(builder.top_stage, self.target, builder));

try_run(builder, builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)));
builder.run_delaying_failure(
builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)),
);
}
}

Expand Down Expand Up @@ -285,8 +261,7 @@ impl Step for Cargotest {

let _time = util::timeit(&builder);
let mut cmd = builder.tool_cmd(Tool::CargoTest);
try_run(
builder,
builder.run_delaying_failure(
cmd.arg(&cargo)
.arg(&out_dir)
.args(builder.config.test_args())
Expand Down Expand Up @@ -827,7 +802,8 @@ impl Step for Clippy {

let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host);

if builder.try_run(&mut cargo).is_ok() {
#[allow(deprecated)] // Clippy reports errors if it blessed the outputs
if builder.config.try_run(&mut cargo).is_ok() {
// The tests succeeded; nothing to do.
return;
}
Expand Down Expand Up @@ -887,7 +863,7 @@ impl Step for RustdocTheme {
util::lld_flag_no_threads(self.compiler.host.contains("windows")),
);
}
try_run(builder, &mut cmd);
builder.run_delaying_failure(&mut cmd);
}
}

Expand Down Expand Up @@ -1147,7 +1123,7 @@ help: to skip test's attempt to check tidiness, pass `--exclude src/tools/tidy`
}

builder.info("tidy check");
try_run(builder, &mut cmd);
builder.run_delaying_failure(&mut cmd);

builder.ensure(ExpandYamlAnchors);

Expand Down Expand Up @@ -1192,8 +1168,7 @@ impl Step for ExpandYamlAnchors {
/// by the user before committing CI changes.
fn run(self, builder: &Builder<'_>) {
builder.info("Ensuring the YAML anchors in the GitHub Actions config were expanded");
try_run(
builder,
builder.run_delaying_failure(
&mut builder.tool_cmd(Tool::ExpandYamlAnchors).arg("check").arg(&builder.src),
);
}
Expand Down Expand Up @@ -1982,7 +1957,7 @@ impl BookTest {
compiler.host,
);
let _time = util::timeit(&builder);
let toolstate = if try_run(builder, &mut rustbook_cmd) {
let toolstate = if builder.run_delaying_failure(&mut rustbook_cmd) {
ToolState::TestPass
} else {
ToolState::TestFail
Expand Down Expand Up @@ -2144,9 +2119,9 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) ->
cmd.arg("--test-args").arg(test_args);

if builder.config.verbose_tests {
try_run(builder, &mut cmd)
builder.run_delaying_failure(&mut cmd)
} else {
try_run_quiet(builder, &mut cmd)
builder.run_quiet_delaying_failure(&mut cmd)
}
}

Expand All @@ -2172,7 +2147,7 @@ impl Step for RustcGuide {

let src = builder.src.join(relative_path);
let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook);
let toolstate = if try_run(builder, rustbook_cmd.arg("linkcheck").arg(&src)) {
let toolstate = if builder.run_delaying_failure(rustbook_cmd.arg("linkcheck").arg(&src)) {
ToolState::TestPass
} else {
ToolState::TestFail
Expand Down Expand Up @@ -2725,7 +2700,7 @@ impl Step for Bootstrap {
.current_dir(builder.src.join("src/bootstrap/"));
// NOTE: we intentionally don't pass test_args here because the args for unittest and cargo test are mutually incompatible.
// Use `python -m unittest` manually if you want to pass arguments.
try_run(builder, &mut check_bootstrap);
builder.run_delaying_failure(&mut check_bootstrap);

let mut cmd = Command::new(&builder.initial_cargo);
cmd.arg("test")
Expand Down Expand Up @@ -2801,7 +2776,7 @@ impl Step for TierCheck {
self.compiler.host,
self.compiler.host,
);
try_run(builder, &mut cargo.into());
builder.run_delaying_failure(&mut cargo.into());
}
}

Expand Down Expand Up @@ -2887,7 +2862,7 @@ impl Step for RustInstaller {
cmd.env("CARGO", &builder.initial_cargo);
cmd.env("RUSTC", &builder.initial_rustc);
cmd.env("TMP_DIR", &tmpdir);
try_run(builder, &mut cmd);
builder.run_delaying_failure(&mut cmd);
}

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
Expand Down
3 changes: 2 additions & 1 deletion src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ impl Step for ToolBuild {
);

let mut cargo = Command::from(cargo);
let is_expected = builder.try_run(&mut cargo).is_ok();
#[allow(deprecated)] // we check this in `is_optional_tool` in a second
let is_expected = builder.config.try_run(&mut cargo).is_ok();

builder.save_toolstate(
tool,
Expand Down
10 changes: 0 additions & 10 deletions src/bootstrap/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,6 @@ pub fn symlink_dir(config: &Config, original: &Path, link: &Path) -> io::Result<
}
}

/// The CI environment rustbuild is running in. This mainly affects how the logs
/// are printed.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum CiEnv {
/// Not a CI environment.
None,
/// The GitHub Actions environment, for Linux (including Docker), Windows and macOS builds.
GitHubActions,
}

pub fn forcing_clang_based_tests() -> bool {
if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
match &var.to_string_lossy().to_lowercase()[..] {
Expand Down

0 comments on commit 2c718d1

Please sign in to comment.