Skip to content

Commit 2c718d1

Browse files
committed
Auto merge of rust-lang#113738 - jyn514:rollup-mjcya4c, r=jyn514
Rollup of 3 pull requests Successful merges: - rust-lang#113643 (bootstrap: Clean up try_run) - rust-lang#113731 (Remove unused `bootstrap::util::CiEnv` enum) - rust-lang#113737 (update mailmap for myself) r? `@ghost` `@rustbot` modify labels: rollup
2 parents 4124617 + df729c2 commit 2c718d1

File tree

8 files changed

+81
-91
lines changed

8 files changed

+81
-91
lines changed

.mailmap

+5-1
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,14 @@ Joseph T. Lyons <[email protected]> <[email protected]>
299299
Josh Cotton <[email protected]>
300300
Josh Driver <[email protected]>
301301
Josh Holmer <[email protected]>
302-
303302
Julian Knodt <[email protected]>
304303
305304
Junyoung Cho <[email protected]>
305+
306+
307+
308+
309+
Jynn Nelson <[email protected]>
306310
307311
Kalita Alexey <[email protected]>
308312
Kampfkarren <[email protected]>

src/bootstrap/config.rs

+12
Original file line numberDiff line numberDiff line change
@@ -1742,6 +1742,18 @@ impl Config {
17421742
}
17431743
}
17441744

1745+
/// Runs a command, printing out nice contextual information if it fails.
1746+
/// Exits if the command failed to execute at all, otherwise returns its
1747+
/// `status.success()`.
1748+
#[deprecated = "use `Builder::try_run` instead where possible"]
1749+
pub(crate) fn try_run(&self, cmd: &mut Command) -> Result<(), ()> {
1750+
if self.dry_run() {
1751+
return Ok(());
1752+
}
1753+
self.verbose(&format!("running: {:?}", cmd));
1754+
build_helper::util::try_run(cmd, self.is_verbose())
1755+
}
1756+
17451757
/// A git invocation which runs inside the source directory.
17461758
///
17471759
/// Use this rather than `Command::new("git")` in order to support out-of-tree builds.

src/bootstrap/download.rs

+15-18
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::{
77
process::{Command, Stdio},
88
};
99

10-
use build_helper::{ci::CiEnv, util::try_run};
10+
use build_helper::ci::CiEnv;
1111
use once_cell::sync::OnceCell;
1212
use xz2::bufread::XzDecoder;
1313

@@ -21,6 +21,12 @@ use crate::{
2121

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

24+
/// `Config::try_run` wrapper for this module to avoid warnings on `try_run`, since we don't have access to a `builder` yet.
25+
fn try_run(config: &Config, cmd: &mut Command) -> Result<(), ()> {
26+
#[allow(deprecated)]
27+
config.try_run(cmd)
28+
}
29+
2430
/// Generic helpers that are useful anywhere in bootstrap.
2531
impl Config {
2632
pub fn is_verbose(&self) -> bool {
@@ -51,17 +57,6 @@ impl Config {
5157
tmp
5258
}
5359

54-
/// Runs a command, printing out nice contextual information if it fails.
55-
/// Exits if the command failed to execute at all, otherwise returns its
56-
/// `status.success()`.
57-
pub(crate) fn try_run(&self, cmd: &mut Command) -> Result<(), ()> {
58-
if self.dry_run() {
59-
return Ok(());
60-
}
61-
self.verbose(&format!("running: {:?}", cmd));
62-
try_run(cmd, self.is_verbose())
63-
}
64-
6560
/// Runs a command, printing out nice contextual information if it fails.
6661
/// Returns false if do not execute at all, otherwise returns its
6762
/// `status.success()`.
@@ -156,14 +151,16 @@ impl Config {
156151
];
157152
}
158153
";
159-
nix_build_succeeded = self
160-
.try_run(Command::new("nix-build").args(&[
154+
nix_build_succeeded = try_run(
155+
self,
156+
Command::new("nix-build").args(&[
161157
Path::new("-E"),
162158
Path::new(NIX_EXPR),
163159
Path::new("-o"),
164160
&nix_deps_dir,
165-
]))
166-
.is_ok();
161+
]),
162+
)
163+
.is_ok();
167164
nix_deps_dir
168165
});
169166
if !nix_build_succeeded {
@@ -188,7 +185,7 @@ impl Config {
188185
patchelf.args(&["--set-interpreter", dynamic_linker.trim_end()]);
189186
}
190187

191-
let _ = self.try_run(patchelf.arg(fname));
188+
let _ = try_run(self, patchelf.arg(fname));
192189
}
193190

194191
fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) {
@@ -236,7 +233,7 @@ impl Config {
236233
if self.build.contains("windows-msvc") {
237234
eprintln!("Fallback to PowerShell");
238235
for _ in 0..3 {
239-
if self.try_run(Command::new("PowerShell.exe").args(&[
236+
if try_run(self, Command::new("PowerShell.exe").args(&[
240237
"/nologo",
241238
"-Command",
242239
"[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12;",

src/bootstrap/lib.rs

+29-4
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,6 @@ forward! {
333333
create(path: &Path, s: &str),
334334
remove(f: &Path),
335335
tempdir() -> PathBuf,
336-
try_run(cmd: &mut Command) -> Result<(), ()>,
337336
llvm_link_shared() -> bool,
338337
download_rustc() -> bool,
339338
initial_rustfmt() -> Option<PathBuf>,
@@ -617,7 +616,9 @@ impl Build {
617616
}
618617

619618
// Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error).
619+
#[allow(deprecated)] // diff-index reports the modifications through the exit status
620620
let has_local_modifications = self
621+
.config
621622
.try_run(
622623
Command::new("git")
623624
.args(&["diff-index", "--quiet", "HEAD"])
@@ -971,12 +972,36 @@ impl Build {
971972
/// Runs a command, printing out nice contextual information if it fails.
972973
/// Exits if the command failed to execute at all, otherwise returns its
973974
/// `status.success()`.
974-
fn try_run_quiet(&self, cmd: &mut Command) -> bool {
975+
fn run_quiet_delaying_failure(&self, cmd: &mut Command) -> bool {
975976
if self.config.dry_run() {
976977
return true;
977978
}
978-
self.verbose(&format!("running: {:?}", cmd));
979-
try_run_suppressed(cmd)
979+
if !self.fail_fast {
980+
self.verbose(&format!("running: {:?}", cmd));
981+
if !try_run_suppressed(cmd) {
982+
let mut failures = self.delayed_failures.borrow_mut();
983+
failures.push(format!("{:?}", cmd));
984+
return false;
985+
}
986+
} else {
987+
self.run_quiet(cmd);
988+
}
989+
true
990+
}
991+
992+
/// Runs a command, printing out contextual info if it fails, and delaying errors until the build finishes.
993+
pub(crate) fn run_delaying_failure(&self, cmd: &mut Command) -> bool {
994+
if !self.fail_fast {
995+
#[allow(deprecated)] // can't use Build::try_run, that's us
996+
if self.config.try_run(cmd).is_err() {
997+
let mut failures = self.delayed_failures.borrow_mut();
998+
failures.push(format!("{:?}", cmd));
999+
return false;
1000+
}
1001+
} else {
1002+
self.run(cmd);
1003+
}
1004+
true
9801005
}
9811006

9821007
pub fn is_verbose_than(&self, level: usize) -> bool {

src/bootstrap/run.rs

+1-15
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ impl Step for ExpandYamlAnchors {
2424
/// anchors in them, since GitHub Actions doesn't support them.
2525
fn run(self, builder: &Builder<'_>) {
2626
builder.info("Expanding YAML anchors in the GitHub Actions configuration");
27-
try_run(
28-
builder,
27+
builder.run_delaying_failure(
2928
&mut builder.tool_cmd(Tool::ExpandYamlAnchors).arg("generate").arg(&builder.src),
3029
);
3130
}
@@ -39,19 +38,6 @@ impl Step for ExpandYamlAnchors {
3938
}
4039
}
4140

42-
fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
43-
if !builder.fail_fast {
44-
if builder.try_run(cmd).is_err() {
45-
let mut failures = builder.delayed_failures.borrow_mut();
46-
failures.push(format!("{:?}", cmd));
47-
return false;
48-
}
49-
} else {
50-
builder.run(cmd);
51-
}
52-
true
53-
}
54-
5541
#[derive(Debug, PartialOrd, Ord, Copy, Clone, Hash, PartialEq, Eq)]
5642
pub struct BuildManifest;
5743

src/bootstrap/test.rs

+17-42
Original file line numberDiff line numberDiff line change
@@ -48,32 +48,6 @@ const MIR_OPT_BLESS_TARGET_MAPPING: &[(&str, &str)] = &[
4848
// build for, so there is no entry for "aarch64-apple-darwin" here.
4949
];
5050

51-
fn try_run(builder: &Builder<'_>, cmd: &mut Command) -> bool {
52-
if !builder.fail_fast {
53-
if builder.try_run(cmd).is_err() {
54-
let mut failures = builder.delayed_failures.borrow_mut();
55-
failures.push(format!("{:?}", cmd));
56-
return false;
57-
}
58-
} else {
59-
builder.run(cmd);
60-
}
61-
true
62-
}
63-
64-
fn try_run_quiet(builder: &Builder<'_>, cmd: &mut Command) -> bool {
65-
if !builder.fail_fast {
66-
if !builder.try_run_quiet(cmd) {
67-
let mut failures = builder.delayed_failures.borrow_mut();
68-
failures.push(format!("{:?}", cmd));
69-
return false;
70-
}
71-
} else {
72-
builder.run_quiet(cmd);
73-
}
74-
true
75-
}
76-
7751
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
7852
pub struct CrateBootstrap {
7953
path: Interned<PathBuf>,
@@ -193,7 +167,7 @@ You can skip linkcheck with --exclude src/tools/linkchecker"
193167
let _guard =
194168
builder.msg(Kind::Test, compiler.stage, "Linkcheck", bootstrap_host, bootstrap_host);
195169
let _time = util::timeit(&builder);
196-
try_run(builder, linkchecker.arg(builder.out.join(host.triple).join("doc")));
170+
builder.run_delaying_failure(linkchecker.arg(builder.out.join(host.triple).join("doc")));
197171
}
198172

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

249-
try_run(builder, builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)));
223+
builder.run_delaying_failure(
224+
builder.tool_cmd(Tool::HtmlChecker).arg(builder.doc_out(self.target)),
225+
);
250226
}
251227
}
252228

@@ -285,8 +261,7 @@ impl Step for Cargotest {
285261

286262
let _time = util::timeit(&builder);
287263
let mut cmd = builder.tool_cmd(Tool::CargoTest);
288-
try_run(
289-
builder,
264+
builder.run_delaying_failure(
290265
cmd.arg(&cargo)
291266
.arg(&out_dir)
292267
.args(builder.config.test_args())
@@ -827,7 +802,8 @@ impl Step for Clippy {
827802

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

830-
if builder.try_run(&mut cargo).is_ok() {
805+
#[allow(deprecated)] // Clippy reports errors if it blessed the outputs
806+
if builder.config.try_run(&mut cargo).is_ok() {
831807
// The tests succeeded; nothing to do.
832808
return;
833809
}
@@ -887,7 +863,7 @@ impl Step for RustdocTheme {
887863
util::lld_flag_no_threads(self.compiler.host.contains("windows")),
888864
);
889865
}
890-
try_run(builder, &mut cmd);
866+
builder.run_delaying_failure(&mut cmd);
891867
}
892868
}
893869

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

11491125
builder.info("tidy check");
1150-
try_run(builder, &mut cmd);
1126+
builder.run_delaying_failure(&mut cmd);
11511127

11521128
builder.ensure(ExpandYamlAnchors);
11531129

@@ -1192,8 +1168,7 @@ impl Step for ExpandYamlAnchors {
11921168
/// by the user before committing CI changes.
11931169
fn run(self, builder: &Builder<'_>) {
11941170
builder.info("Ensuring the YAML anchors in the GitHub Actions config were expanded");
1195-
try_run(
1196-
builder,
1171+
builder.run_delaying_failure(
11971172
&mut builder.tool_cmd(Tool::ExpandYamlAnchors).arg("check").arg(&builder.src),
11981173
);
11991174
}
@@ -1982,7 +1957,7 @@ impl BookTest {
19821957
compiler.host,
19831958
);
19841959
let _time = util::timeit(&builder);
1985-
let toolstate = if try_run(builder, &mut rustbook_cmd) {
1960+
let toolstate = if builder.run_delaying_failure(&mut rustbook_cmd) {
19861961
ToolState::TestPass
19871962
} else {
19881963
ToolState::TestFail
@@ -2144,9 +2119,9 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) ->
21442119
cmd.arg("--test-args").arg(test_args);
21452120

21462121
if builder.config.verbose_tests {
2147-
try_run(builder, &mut cmd)
2122+
builder.run_delaying_failure(&mut cmd)
21482123
} else {
2149-
try_run_quiet(builder, &mut cmd)
2124+
builder.run_quiet_delaying_failure(&mut cmd)
21502125
}
21512126
}
21522127

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

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

27302705
let mut cmd = Command::new(&builder.initial_cargo);
27312706
cmd.arg("test")
@@ -2801,7 +2776,7 @@ impl Step for TierCheck {
28012776
self.compiler.host,
28022777
self.compiler.host,
28032778
);
2804-
try_run(builder, &mut cargo.into());
2779+
builder.run_delaying_failure(&mut cargo.into());
28052780
}
28062781
}
28072782

@@ -2887,7 +2862,7 @@ impl Step for RustInstaller {
28872862
cmd.env("CARGO", &builder.initial_cargo);
28882863
cmd.env("RUSTC", &builder.initial_rustc);
28892864
cmd.env("TMP_DIR", &tmpdir);
2890-
try_run(builder, &mut cmd);
2865+
builder.run_delaying_failure(&mut cmd);
28912866
}
28922867

28932868
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {

src/bootstrap/tool.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ impl Step for ToolBuild {
108108
);
109109

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

113114
builder.save_toolstate(
114115
tool,

src/bootstrap/util.rs

-10
Original file line numberDiff line numberDiff line change
@@ -153,16 +153,6 @@ pub fn symlink_dir(config: &Config, original: &Path, link: &Path) -> io::Result<
153153
}
154154
}
155155

156-
/// The CI environment rustbuild is running in. This mainly affects how the logs
157-
/// are printed.
158-
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
159-
pub enum CiEnv {
160-
/// Not a CI environment.
161-
None,
162-
/// The GitHub Actions environment, for Linux (including Docker), Windows and macOS builds.
163-
GitHubActions,
164-
}
165-
166156
pub fn forcing_clang_based_tests() -> bool {
167157
if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
168158
match &var.to_string_lossy().to_lowercase()[..] {

0 commit comments

Comments
 (0)