Skip to content

Commit 4451777

Browse files
borsbors
committed
Auto merge of #117595 - jyn514:x-clippy, r=albertlarsan68
x clippy thanks to `@asquared31415` `@albertlarsan68` for all their help, most of this pr is their work note that this also adds x clippy --stage 0 -Awarnings to x86_64-gnu-llvm-15 to make sure it stays working; that won't gate on any clippy warnings, just enforce that clippy doesn't give a hard error. we can't add --stage 1 until clippy fixes its debug assertions not to panic. note that `x clippy --stage 1` currently breaks when combined with download-rustc. unlike the previous prs, this doesn't require changes to clippy (it works by using RUSTC_WRAPPER instead), and supports stage 0 read this commit-by-commit closes #107628; see also #106394, #97443. fixes #95988. helps with #76495. r? bootstrap
2 parents 02ad667 + a078c3a commit 4451777

File tree

10 files changed

+245
-93
lines changed

10 files changed

+245
-93
lines changed

‎src/bootstrap/bootstrap.py

-16
Original file line numberDiff line numberDiff line change
@@ -616,22 +616,6 @@ def download_toolchain(self):
616616
with output(self.rustc_stamp()) as rust_stamp:
617617
rust_stamp.write(key)
618618

619-
def _download_component_helper(
620-
self, filename, pattern, tarball_suffix, rustc_cache,
621-
):
622-
key = self.stage0_compiler.date
623-
624-
tarball = os.path.join(rustc_cache, filename)
625-
if not os.path.exists(tarball):
626-
get(
627-
self.download_url,
628-
"dist/{}/{}".format(key, filename),
629-
tarball,
630-
self.checksums_sha256,
631-
verbose=self.verbose,
632-
)
633-
unpack(tarball, tarball_suffix, self.bin_root(), match=pattern, verbose=self.verbose)
634-
635619
def should_fix_bins_and_dylibs(self):
636620
"""Whether or not `fix_bin_or_dylib` needs to be run; can only be True
637621
on NixOS or if config.toml has `build.patch-binaries-for-nix` set.

‎src/bootstrap/src/bin/rustc.rs

+45-7
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@
1616
//! never get replaced.
1717
1818
use std::env;
19-
use std::path::PathBuf;
19+
use std::path::{Path, PathBuf};
2020
use std::process::{Child, Command};
2121
use std::time::Instant;
2222

23-
use dylib_util::{dylib_path, dylib_path_var};
23+
use dylib_util::{dylib_path, dylib_path_var, exe};
2424

2525
#[path = "../utils/bin_helpers.rs"]
2626
mod bin_helpers;
@@ -29,8 +29,10 @@ mod bin_helpers;
2929
mod dylib_util;
3030

3131
fn main() {
32-
let args = env::args_os().skip(1).collect::<Vec<_>>();
33-
let arg = |name| args.windows(2).find(|args| args[0] == name).and_then(|args| args[1].to_str());
32+
let orig_args = env::args_os().skip(1).collect::<Vec<_>>();
33+
let mut args = orig_args.clone();
34+
let arg =
35+
|name| orig_args.windows(2).find(|args| args[0] == name).and_then(|args| args[1].to_str());
3436

3537
let stage = bin_helpers::parse_rustc_stage();
3638
let verbose = bin_helpers::parse_rustc_verbose();
@@ -45,7 +47,8 @@ fn main() {
4547
// determine the version of the compiler, the real compiler needs to be
4648
// used. Currently, these two states are differentiated based on whether
4749
// --target and -vV is/isn't passed.
48-
let (rustc, libdir) = if target.is_none() && version.is_none() {
50+
let is_build_script = target.is_none() && version.is_none();
51+
let (rustc, libdir) = if is_build_script {
4952
("RUSTC_SNAPSHOT", "RUSTC_SNAPSHOT_LIBDIR")
5053
} else {
5154
("RUSTC_REAL", "RUSTC_LIBDIR")
@@ -54,12 +57,47 @@ fn main() {
5457
let sysroot = env::var_os("RUSTC_SYSROOT").expect("RUSTC_SYSROOT was not set");
5558
let on_fail = env::var_os("RUSTC_ON_FAIL").map(Command::new);
5659

57-
let rustc = env::var_os(rustc).unwrap_or_else(|| panic!("{:?} was not set", rustc));
60+
let rustc_real = env::var_os(rustc).unwrap_or_else(|| panic!("{:?} was not set", rustc));
5861
let libdir = env::var_os(libdir).unwrap_or_else(|| panic!("{:?} was not set", libdir));
5962
let mut dylib_path = dylib_path();
6063
dylib_path.insert(0, PathBuf::from(&libdir));
6164

62-
let mut cmd = Command::new(rustc);
65+
// if we're running clippy, trust cargo-clippy to set clippy-driver appropriately (and don't override it with rustc).
66+
// otherwise, substitute whatever cargo thinks rustc should be with RUSTC_REAL.
67+
// NOTE: this means we ignore RUSTC in the environment.
68+
// FIXME: We might want to consider removing RUSTC_REAL and setting RUSTC directly?
69+
// NOTE: we intentionally pass the name of the host, not the target.
70+
let host = env::var("CFG_COMPILER_BUILD_TRIPLE").unwrap();
71+
let is_clippy = args[0].to_string_lossy().ends_with(&exe("clippy-driver", &host));
72+
let rustc_driver = if is_clippy {
73+
if is_build_script {
74+
// Don't run clippy on build scripts (for one thing, we may not have libstd built with
75+
// the appropriate version yet, e.g. for stage 1 std).
76+
// Also remove the `clippy-driver` param in addition to the RUSTC param.
77+
args.drain(..2);
78+
rustc_real
79+
} else {
80+
args.remove(0)
81+
}
82+
} else {
83+
// Cargo doesn't respect RUSTC_WRAPPER for version information >:(
84+
// don't remove the first arg if we're being run as RUSTC instead of RUSTC_WRAPPER.
85+
// Cargo also sometimes doesn't pass the `.exe` suffix on Windows - add it manually.
86+
let current_exe = env::current_exe().expect("couldn't get path to rustc shim");
87+
let arg0 = exe(args[0].to_str().expect("only utf8 paths are supported"), &host);
88+
if Path::new(&arg0) == current_exe {
89+
args.remove(0);
90+
}
91+
rustc_real
92+
};
93+
94+
let mut cmd = if let Some(wrapper) = env::var_os("RUSTC_WRAPPER_REAL") {
95+
let mut cmd = Command::new(wrapper);
96+
cmd.arg(rustc_driver);
97+
cmd
98+
} else {
99+
Command::new(rustc_driver)
100+
};
63101
cmd.args(&args).env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());
64102

65103
// Get the name of the crate we're compiling, if any.

‎src/bootstrap/src/core/build_steps/compile.rs

+9-7
Original file line numberDiff line numberDiff line change
@@ -1809,10 +1809,6 @@ pub fn run_cargo(
18091809
is_check: bool,
18101810
rlib_only_metadata: bool,
18111811
) -> Vec<PathBuf> {
1812-
if builder.config.dry_run() {
1813-
return Vec::new();
1814-
}
1815-
18161812
// `target_root_dir` looks like $dir/$target/release
18171813
let target_root_dir = stamp.parent().unwrap();
18181814
// `target_deps_dir` looks like $dir/$target/release/deps
@@ -1919,6 +1915,10 @@ pub fn run_cargo(
19191915
crate::exit!(1);
19201916
}
19211917

1918+
if builder.config.dry_run() {
1919+
return Vec::new();
1920+
}
1921+
19221922
// Ok now we need to actually find all the files listed in `toplevel`. We've
19231923
// got a list of prefix/extensions and we basically just need to find the
19241924
// most recent file in the `deps` folder corresponding to each one.
@@ -1974,9 +1974,6 @@ pub fn stream_cargo(
19741974
cb: &mut dyn FnMut(CargoMessage<'_>),
19751975
) -> bool {
19761976
let mut cargo = Command::from(cargo);
1977-
if builder.config.dry_run() {
1978-
return true;
1979-
}
19801977
// Instruct Cargo to give us json messages on stdout, critically leaving
19811978
// stderr as piped so we can get those pretty colors.
19821979
let mut message_format = if builder.config.json_output {
@@ -1995,6 +1992,11 @@ pub fn stream_cargo(
19951992
}
19961993

19971994
builder.verbose(&format!("running: {cargo:?}"));
1995+
1996+
if builder.config.dry_run() {
1997+
return true;
1998+
}
1999+
19982000
let mut child = match cargo.spawn() {
19992001
Ok(child) => child,
20002002
Err(e) => panic!("failed to execute command: {cargo:?}\nERROR: {e}"),

‎src/bootstrap/src/core/builder.rs

+88-52
Original file line numberDiff line numberDiff line change
@@ -1152,6 +1152,44 @@ impl<'a> Builder<'a> {
11521152
self.ensure(tool::Rustdoc { compiler })
11531153
}
11541154

1155+
pub fn cargo_clippy_cmd(&self, run_compiler: Compiler) -> Command {
1156+
let initial_sysroot_bin = self.initial_rustc.parent().unwrap();
1157+
// Set PATH to include the sysroot bin dir so clippy can find cargo.
1158+
// FIXME: once rust-clippy#11944 lands on beta, set `CARGO` directly instead.
1159+
let path = t!(env::join_paths(
1160+
// The sysroot comes first in PATH to avoid using rustup's cargo.
1161+
std::iter::once(PathBuf::from(initial_sysroot_bin))
1162+
.chain(env::split_paths(&t!(env::var("PATH"))))
1163+
));
1164+
1165+
if run_compiler.stage == 0 {
1166+
// `ensure(Clippy { stage: 0 })` *builds* clippy with stage0, it doesn't use the beta clippy.
1167+
let cargo_clippy = self.build.config.download_clippy();
1168+
let mut cmd = Command::new(cargo_clippy);
1169+
cmd.env("PATH", &path);
1170+
return cmd;
1171+
}
1172+
1173+
let build_compiler = self.compiler(run_compiler.stage - 1, self.build.build);
1174+
self.ensure(tool::Clippy {
1175+
compiler: build_compiler,
1176+
target: self.build.build,
1177+
extra_features: vec![],
1178+
});
1179+
let cargo_clippy = self.ensure(tool::CargoClippy {
1180+
compiler: build_compiler,
1181+
target: self.build.build,
1182+
extra_features: vec![],
1183+
});
1184+
let mut dylib_path = helpers::dylib_path();
1185+
dylib_path.insert(0, self.sysroot(run_compiler).join("lib"));
1186+
1187+
let mut cmd = Command::new(cargo_clippy.unwrap());
1188+
cmd.env(helpers::dylib_path_var(), env::join_paths(&dylib_path).unwrap());
1189+
cmd.env("PATH", path);
1190+
cmd
1191+
}
1192+
11551193
pub fn rustdoc_cmd(&self, compiler: Compiler) -> Command {
11561194
let mut cmd = Command::new(&self.bootstrap_out.join("rustdoc"));
11571195
cmd.env("RUSTC_STAGE", compiler.stage.to_string())
@@ -1200,7 +1238,12 @@ impl<'a> Builder<'a> {
12001238
target: TargetSelection,
12011239
cmd: &str,
12021240
) -> Command {
1203-
let mut cargo = Command::new(&self.initial_cargo);
1241+
let mut cargo = if cmd == "clippy" {
1242+
self.cargo_clippy_cmd(compiler)
1243+
} else {
1244+
Command::new(&self.initial_cargo)
1245+
};
1246+
12041247
// Run cargo from the source root so it can find .cargo/config.
12051248
// This matters when using vendoring and the working directory is outside the repository.
12061249
cargo.current_dir(&self.src);
@@ -1324,6 +1367,23 @@ impl<'a> Builder<'a> {
13241367
compiler.stage
13251368
};
13261369

1370+
// We synthetically interpret a stage0 compiler used to build tools as a
1371+
// "raw" compiler in that it's the exact snapshot we download. Normally
1372+
// the stage0 build means it uses libraries build by the stage0
1373+
// compiler, but for tools we just use the precompiled libraries that
1374+
// we've downloaded
1375+
let use_snapshot = mode == Mode::ToolBootstrap;
1376+
assert!(!use_snapshot || stage == 0 || self.local_rebuild);
1377+
1378+
let maybe_sysroot = self.sysroot(compiler);
1379+
let sysroot = if use_snapshot { self.rustc_snapshot_sysroot() } else { &maybe_sysroot };
1380+
let libdir = self.rustc_libdir(compiler);
1381+
1382+
let sysroot_str = sysroot.as_os_str().to_str().expect("sysroot should be UTF-8");
1383+
if !matches!(self.config.dry_run, DryRun::SelfCheck) {
1384+
self.verbose_than(0, &format!("using sysroot {sysroot_str}"));
1385+
}
1386+
13271387
let mut rustflags = Rustflags::new(target);
13281388
if stage != 0 {
13291389
if let Ok(s) = env::var("CARGOFLAGS_NOT_BOOTSTRAP") {
@@ -1335,41 +1395,16 @@ impl<'a> Builder<'a> {
13351395
cargo.args(s.split_whitespace());
13361396
}
13371397
rustflags.env("RUSTFLAGS_BOOTSTRAP");
1338-
if cmd == "clippy" {
1339-
// clippy overwrites sysroot if we pass it to cargo.
1340-
// Pass it directly to clippy instead.
1341-
// NOTE: this can't be fixed in clippy because we explicitly don't set `RUSTC`,
1342-
// so it has no way of knowing the sysroot.
1343-
rustflags.arg("--sysroot");
1344-
rustflags.arg(
1345-
self.sysroot(compiler)
1346-
.as_os_str()
1347-
.to_str()
1348-
.expect("sysroot must be valid UTF-8"),
1349-
);
1350-
// Only run clippy on a very limited subset of crates (in particular, not build scripts).
1351-
cargo.arg("-Zunstable-options");
1352-
// Explicitly does *not* set `--cfg=bootstrap`, since we're using a nightly clippy.
1353-
let host_version = Command::new("rustc").arg("--version").output().map_err(|_| ());
1354-
let output = host_version.and_then(|output| {
1355-
if output.status.success() {
1356-
Ok(output)
1357-
} else {
1358-
Err(())
1359-
}
1360-
}).unwrap_or_else(|_| {
1361-
eprintln!(
1362-
"ERROR: `x.py clippy` requires a host `rustc` toolchain with the `clippy` component"
1363-
);
1364-
eprintln!("HELP: try `rustup component add clippy`");
1365-
crate::exit!(1);
1366-
});
1367-
if !t!(std::str::from_utf8(&output.stdout)).contains("nightly") {
1368-
rustflags.arg("--cfg=bootstrap");
1369-
}
1370-
} else {
1371-
rustflags.arg("--cfg=bootstrap");
1372-
}
1398+
rustflags.arg("--cfg=bootstrap");
1399+
}
1400+
1401+
if cmd == "clippy" {
1402+
// clippy overwrites sysroot if we pass it to cargo.
1403+
// Pass it directly to clippy instead.
1404+
// NOTE: this can't be fixed in clippy because we explicitly don't set `RUSTC`,
1405+
// so it has no way of knowing the sysroot.
1406+
rustflags.arg("--sysroot");
1407+
rustflags.arg(sysroot_str);
13731408
}
13741409

13751410
let use_new_symbol_mangling = match self.config.rust_new_symbol_mangling {
@@ -1564,18 +1599,6 @@ impl<'a> Builder<'a> {
15641599

15651600
let want_rustdoc = self.doc_tests != DocTests::No;
15661601

1567-
// We synthetically interpret a stage0 compiler used to build tools as a
1568-
// "raw" compiler in that it's the exact snapshot we download. Normally
1569-
// the stage0 build means it uses libraries build by the stage0
1570-
// compiler, but for tools we just use the precompiled libraries that
1571-
// we've downloaded
1572-
let use_snapshot = mode == Mode::ToolBootstrap;
1573-
assert!(!use_snapshot || stage == 0 || self.local_rebuild);
1574-
1575-
let maybe_sysroot = self.sysroot(compiler);
1576-
let sysroot = if use_snapshot { self.rustc_snapshot_sysroot() } else { &maybe_sysroot };
1577-
let libdir = self.rustc_libdir(compiler);
1578-
15791602
// Clear the output directory if the real rustc we're using has changed;
15801603
// Cargo cannot detect this as it thinks rustc is bootstrap/debug/rustc.
15811604
//
@@ -1611,10 +1634,19 @@ impl<'a> Builder<'a> {
16111634
)
16121635
.env("RUSTC_ERROR_METADATA_DST", self.extended_error_dir())
16131636
.env("RUSTC_BREAK_ON_ICE", "1");
1614-
// Clippy support is a hack and uses the default `cargo-clippy` in path.
1615-
// Don't override RUSTC so that the `cargo-clippy` in path will be run.
1616-
if cmd != "clippy" {
1617-
cargo.env("RUSTC", self.bootstrap_out.join("rustc"));
1637+
1638+
// Set RUSTC_WRAPPER to the bootstrap shim, which switches between beta and in-tree
1639+
// sysroot depending on whether we're building build scripts.
1640+
// NOTE: we intentionally use RUSTC_WRAPPER so that we can support clippy - RUSTC is not
1641+
// respected by clippy-driver; RUSTC_WRAPPER happens earlier, before clippy runs.
1642+
cargo.env("RUSTC_WRAPPER", self.bootstrap_out.join("rustc"));
1643+
// NOTE: we also need to set RUSTC so cargo can run `rustc -vV`; apparently that ignores RUSTC_WRAPPER >:(
1644+
cargo.env("RUSTC", self.bootstrap_out.join("rustc"));
1645+
1646+
// Someone might have set some previous rustc wrapper (e.g.
1647+
// sccache) before bootstrap overrode it. Respect that variable.
1648+
if let Some(existing_wrapper) = env::var_os("RUSTC_WRAPPER") {
1649+
cargo.env("RUSTC_WRAPPER_REAL", existing_wrapper);
16181650
}
16191651

16201652
// Dealing with rpath here is a little special, so let's go into some
@@ -1991,7 +2023,11 @@ impl<'a> Builder<'a> {
19912023
// Environment variables *required* throughout the build
19922024
//
19932025
// FIXME: should update code to not require this env var
2026+
2027+
// The host this new compiler will *run* on.
19942028
cargo.env("CFG_COMPILER_HOST_TRIPLE", target.triple);
2029+
// The host this new compiler is being *built* on.
2030+
cargo.env("CFG_COMPILER_BUILD_TRIPLE", compiler.host.triple);
19952031

19962032
// Set this for all builds to make sure doc builds also get it.
19972033
cargo.env("CFG_RELEASE_CHANNEL", &self.config.channel);

‎src/bootstrap/src/core/download.rs

+34-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,10 @@ impl Config {
208208
Some(other) => panic!("unsupported protocol {other} in {url}"),
209209
None => panic!("no protocol in {url}"),
210210
}
211-
t!(std::fs::rename(&tempfile, dest_path));
211+
t!(
212+
std::fs::rename(&tempfile, dest_path),
213+
format!("failed to rename {tempfile:?} to {dest_path:?}")
214+
);
212215
}
213216

214217
fn download_http_with_retries(&self, tempfile: &Path, url: &str, help_on_error: &str) {
@@ -375,6 +378,32 @@ enum DownloadSource {
375378

376379
/// Functions that are only ever called once, but named for clarify and to avoid thousand-line functions.
377380
impl Config {
381+
pub(crate) fn download_clippy(&self) -> PathBuf {
382+
self.verbose("downloading stage0 clippy artifacts");
383+
384+
let date = &self.stage0_metadata.compiler.date;
385+
let version = &self.stage0_metadata.compiler.version;
386+
let host = self.build;
387+
388+
let bin_root = self.out.join(host.triple).join("stage0");
389+
let clippy_stamp = bin_root.join(".clippy-stamp");
390+
let cargo_clippy = bin_root.join("bin").join(exe("cargo-clippy", host));
391+
if cargo_clippy.exists() && !program_out_of_date(&clippy_stamp, &date) {
392+
return cargo_clippy;
393+
}
394+
395+
let filename = format!("clippy-{version}-{host}.tar.xz");
396+
self.download_component(DownloadSource::Dist, filename, "clippy-preview", date, "stage0");
397+
if self.should_fix_bins_and_dylibs() {
398+
self.fix_bin_or_dylib(&cargo_clippy);
399+
self.fix_bin_or_dylib(&cargo_clippy.with_file_name(exe("clippy-driver", host)));
400+
}
401+
402+
cargo_clippy
403+
}
404+
405+
/// NOTE: rustfmt is a completely different toolchain than the bootstrap compiler, so it can't
406+
/// reuse target directories or artifacts
378407
pub(crate) fn maybe_download_rustfmt(&self) -> Option<PathBuf> {
379408
let RustfmtMetadata { date, version } = self.stage0_metadata.rustfmt.as_ref()?;
380409
let channel = format!("{version}-{date}");
@@ -544,6 +573,10 @@ impl Config {
544573
key: &str,
545574
destination: &str,
546575
) {
576+
if self.dry_run() {
577+
return;
578+
}
579+
547580
let cache_dst = self.out.join("cache");
548581
let cache_dir = cache_dst.join(key);
549582
if !cache_dir.exists() {

0 commit comments

Comments
 (0)