From 9341325c73a596292291840c345be09fdb4af083 Mon Sep 17 00:00:00 2001 From: asquared31415 <34665709+asquared31415@users.noreply.github.com> Date: Sun, 22 May 2022 13:53:31 -0400 Subject: [PATCH] make x.py clippy download and use beta clippy --- src/bootstrap/bootstrap.py | 16 ---- src/bootstrap/src/bin/rustc.rs | 22 +++-- src/bootstrap/src/core/builder.rs | 146 ++++++++++++++++------------- src/bootstrap/src/core/download.rs | 26 +++++ src/tools/bump-stage0/src/main.rs | 2 +- 5 files changed, 124 insertions(+), 88 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 2328dd822af..fea194a80ef 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -616,22 +616,6 @@ class RustBuild(object): with output(self.rustc_stamp()) as rust_stamp: rust_stamp.write(key) - def _download_component_helper( - self, filename, pattern, tarball_suffix, rustc_cache, - ): - key = self.stage0_compiler.date - - tarball = os.path.join(rustc_cache, filename) - if not os.path.exists(tarball): - get( - self.download_url, - "dist/{}/{}".format(key, filename), - tarball, - self.checksums_sha256, - verbose=self.verbose, - ) - unpack(tarball, tarball_suffix, self.bin_root(), match=pattern, verbose=self.verbose) - def should_fix_bins_and_dylibs(self): """Whether or not `fix_bin_or_dylib` needs to be run; can only be True on NixOS or if config.toml has `build.patch-binaries-for-nix` set. diff --git a/src/bootstrap/src/bin/rustc.rs b/src/bootstrap/src/bin/rustc.rs index bebdb288d0f..38c55b20344 100644 --- a/src/bootstrap/src/bin/rustc.rs +++ b/src/bootstrap/src/bin/rustc.rs @@ -47,7 +47,8 @@ fn main() { // determine the version of the compiler, the real compiler needs to be // used. Currently, these two states are differentiated based on whether // --target and -vV is/isn't passed. - let (rustc, libdir) = if target.is_none() && version.is_none() { + let is_build_script = target.is_none() && version.is_none(); + let (rustc, libdir) = if is_build_script { ("RUSTC_SNAPSHOT", "RUSTC_SNAPSHOT_LIBDIR") } else { ("RUSTC_REAL", "RUSTC_LIBDIR") @@ -65,19 +66,24 @@ fn main() { // otherwise, substitute whatever cargo thinks rustc should be with RUSTC_REAL. // NOTE: this means we ignore RUSTC in the environment. // FIXME: We might want to consider removing RUSTC_REAL and setting RUSTC directly? - let target_name = target - .map(|s| s.to_owned()) - .unwrap_or_else(|| env::var("CFG_COMPILER_HOST_TRIPLE").unwrap()); - let is_clippy = args[0].to_string_lossy().ends_with(&exe("clippy-driver", &target_name)); + // NOTE: we intentionally pass the name of the host, not the target. + let host = env::var("CFG_COMPILER_BUILD_TRIPLE").unwrap(); + let is_clippy = args[0].to_string_lossy().ends_with(&exe("clippy-driver", &host)); let rustc_driver = if is_clippy { - args.remove(0) + if is_build_script { + // Don't run clippy on build scripts (for one thing, we may not have libstd built with + // the appropriate version yet, e.g. for stage 1 std). + // Also remove the `clippy-driver` param in addition to the RUSTC param. + args.drain(..2); + rustc_real + } else { + args.remove(0) + } } else { // Cargo doesn't respect RUSTC_WRAPPER for version information >:( // don't remove the first arg if we're being run as RUSTC instead of RUSTC_WRAPPER. // Cargo also sometimes doesn't pass the `.exe` suffix on Windows - add it manually. let current_exe = env::current_exe().expect("couldn't get path to rustc shim"); - // NOTE: we intentionally pass the name of the host, not the target. - let host = env::var("CFG_COMPILER_BUILD_TRIPLE").unwrap(); let arg0 = exe(args[0].to_str().expect("only utf8 paths are supported"), &host); if Path::new(&arg0) == current_exe { args.remove(0); diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index ead84ada8f7..e1809644350 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -1152,6 +1152,44 @@ pub fn rustdoc(&self, compiler: Compiler) -> PathBuf { self.ensure(tool::Rustdoc { compiler }) } + pub fn cargo_clippy_cmd(&self, run_compiler: Compiler) -> Command { + let initial_sysroot_bin = self.initial_rustc.parent().unwrap(); + // Set PATH to include the sysroot bin dir so clippy can find cargo. + // FIXME: once rust-clippy#11944 lands on beta, set `CARGO` directly instead. + let path = t!(env::join_paths( + // The sysroot comes first in PATH to avoid using rustup's cargo. + std::iter::once(PathBuf::from(initial_sysroot_bin)) + .chain(env::split_paths(&t!(env::var("PATH")))) + )); + + if run_compiler.stage == 0 { + // `ensure(Clippy { stage: 0 })` *builds* clippy with stage0, it doesn't use the beta clippy. + let cargo_clippy = self.build.config.download_clippy(); + let mut cmd = Command::new(cargo_clippy); + cmd.env("PATH", &path); + return cmd; + } + + let build_compiler = self.compiler(run_compiler.stage - 1, self.build.build); + self.ensure(tool::Clippy { + compiler: build_compiler, + target: self.build.build, + extra_features: vec![], + }); + let cargo_clippy = self.ensure(tool::CargoClippy { + compiler: build_compiler, + target: self.build.build, + extra_features: vec![], + }); + let mut dylib_path = helpers::dylib_path(); + dylib_path.insert(0, self.sysroot(run_compiler).join("lib")); + + let mut cmd = Command::new(cargo_clippy.unwrap()); + cmd.env(helpers::dylib_path_var(), env::join_paths(&dylib_path).unwrap()); + cmd.env("PATH", path); + cmd + } + pub fn rustdoc_cmd(&self, compiler: Compiler) -> Command { let mut cmd = Command::new(&self.bootstrap_out.join("rustdoc")); cmd.env("RUSTC_STAGE", compiler.stage.to_string()) @@ -1200,7 +1238,12 @@ pub fn bare_cargo( target: TargetSelection, cmd: &str, ) -> Command { - let mut cargo = Command::new(&self.initial_cargo); + let mut cargo = if cmd == "clippy" { + self.cargo_clippy_cmd(compiler) + } else { + Command::new(&self.initial_cargo) + }; + // Run cargo from the source root so it can find .cargo/config. // This matters when using vendoring and the working directory is outside the repository. cargo.current_dir(&self.src); @@ -1324,6 +1367,23 @@ pub fn cargo( compiler.stage }; + // We synthetically interpret a stage0 compiler used to build tools as a + // "raw" compiler in that it's the exact snapshot we download. Normally + // the stage0 build means it uses libraries build by the stage0 + // compiler, but for tools we just use the precompiled libraries that + // we've downloaded + let use_snapshot = mode == Mode::ToolBootstrap; + assert!(!use_snapshot || stage == 0 || self.local_rebuild); + + let maybe_sysroot = self.sysroot(compiler); + let sysroot = if use_snapshot { self.rustc_snapshot_sysroot() } else { &maybe_sysroot }; + let libdir = self.rustc_libdir(compiler); + + let sysroot_str = sysroot.as_os_str().to_str().expect("sysroot should be UTF-8"); + if !matches!(self.config.dry_run, DryRun::SelfCheck) { + self.verbose_than(0, &format!("using sysroot {sysroot_str}")); + } + let mut rustflags = Rustflags::new(target); if stage != 0 { if let Ok(s) = env::var("CARGOFLAGS_NOT_BOOTSTRAP") { @@ -1335,41 +1395,16 @@ pub fn cargo( cargo.args(s.split_whitespace()); } rustflags.env("RUSTFLAGS_BOOTSTRAP"); - if cmd == "clippy" { - // clippy overwrites sysroot if we pass it to cargo. - // Pass it directly to clippy instead. - // NOTE: this can't be fixed in clippy because we explicitly don't set `RUSTC`, - // so it has no way of knowing the sysroot. - rustflags.arg("--sysroot"); - rustflags.arg( - self.sysroot(compiler) - .as_os_str() - .to_str() - .expect("sysroot must be valid UTF-8"), - ); - // Only run clippy on a very limited subset of crates (in particular, not build scripts). - cargo.arg("-Zunstable-options"); - // Explicitly does *not* set `--cfg=bootstrap`, since we're using a nightly clippy. - let host_version = Command::new("rustc").arg("--version").output().map_err(|_| ()); - let output = host_version.and_then(|output| { - if output.status.success() { - Ok(output) - } else { - Err(()) - } - }).unwrap_or_else(|_| { - eprintln!( - "ERROR: `x.py clippy` requires a host `rustc` toolchain with the `clippy` component" - ); - eprintln!("HELP: try `rustup component add clippy`"); - crate::exit!(1); - }); - if !t!(std::str::from_utf8(&output.stdout)).contains("nightly") { - rustflags.arg("--cfg=bootstrap"); - } - } else { - rustflags.arg("--cfg=bootstrap"); - } + rustflags.arg("--cfg=bootstrap"); + } + + if cmd == "clippy" { + // clippy overwrites sysroot if we pass it to cargo. + // Pass it directly to clippy instead. + // NOTE: this can't be fixed in clippy because we explicitly don't set `RUSTC`, + // so it has no way of knowing the sysroot. + rustflags.arg("--sysroot"); + rustflags.arg(sysroot_str); } let use_new_symbol_mangling = match self.config.rust_new_symbol_mangling { @@ -1564,18 +1599,6 @@ pub fn cargo( let want_rustdoc = self.doc_tests != DocTests::No; - // We synthetically interpret a stage0 compiler used to build tools as a - // "raw" compiler in that it's the exact snapshot we download. Normally - // the stage0 build means it uses libraries build by the stage0 - // compiler, but for tools we just use the precompiled libraries that - // we've downloaded - let use_snapshot = mode == Mode::ToolBootstrap; - assert!(!use_snapshot || stage == 0 || self.local_rebuild); - - let maybe_sysroot = self.sysroot(compiler); - let sysroot = if use_snapshot { self.rustc_snapshot_sysroot() } else { &maybe_sysroot }; - let libdir = self.rustc_libdir(compiler); - // Clear the output directory if the real rustc we're using has changed; // Cargo cannot detect this as it thinks rustc is bootstrap/debug/rustc. // @@ -1611,22 +1634,19 @@ pub fn cargo( ) .env("RUSTC_ERROR_METADATA_DST", self.extended_error_dir()) .env("RUSTC_BREAK_ON_ICE", "1"); - // Clippy support is a hack and uses the default `cargo-clippy` in path. - // Don't override RUSTC so that the `cargo-clippy` in path will be run. - if cmd != "clippy" { - // Set RUSTC_WRAPPER to the bootstrap shim, which switches between beta and in-tree - // sysroot depending on whether we're building build scripts. - // NOTE: we intentionally use RUSTC_WRAPPER so that we can support clippy - RUSTC is not - // respected by clippy-driver; RUSTC_WRAPPER happens earlier, before clippy runs. - cargo.env("RUSTC_WRAPPER", self.bootstrap_out.join("rustc")); - // NOTE: we also need to set RUSTC so cargo can run `rustc -vV`; apparently that ignores RUSTC_WRAPPER >:( - cargo.env("RUSTC", self.bootstrap_out.join("rustc")); - // Someone might have set some previous rustc wrapper (e.g. - // sccache) before bootstrap overrode it. Respect that variable. - if let Some(existing_wrapper) = env::var_os("RUSTC_WRAPPER") { - cargo.env("RUSTC_WRAPPER_REAL", existing_wrapper); - } + // Set RUSTC_WRAPPER to the bootstrap shim, which switches between beta and in-tree + // sysroot depending on whether we're building build scripts. + // NOTE: we intentionally use RUSTC_WRAPPER so that we can support clippy - RUSTC is not + // respected by clippy-driver; RUSTC_WRAPPER happens earlier, before clippy runs. + cargo.env("RUSTC_WRAPPER", self.bootstrap_out.join("rustc")); + // NOTE: we also need to set RUSTC so cargo can run `rustc -vV`; apparently that ignores RUSTC_WRAPPER >:( + cargo.env("RUSTC", self.bootstrap_out.join("rustc")); + + // Someone might have set some previous rustc wrapper (e.g. + // sccache) before bootstrap overrode it. Respect that variable. + if let Some(existing_wrapper) = env::var_os("RUSTC_WRAPPER") { + cargo.env("RUSTC_WRAPPER_REAL", existing_wrapper); } // Dealing with rpath here is a little special, so let's go into some diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index ebb789c91f6..1f2ee6a6b73 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -378,6 +378,32 @@ enum DownloadSource { /// Functions that are only ever called once, but named for clarify and to avoid thousand-line functions. impl Config { + pub(crate) fn download_clippy(&self) -> PathBuf { + self.verbose("downloading stage0 clippy artifacts"); + + let date = &self.stage0_metadata.compiler.date; + let version = &self.stage0_metadata.compiler.version; + let host = self.build; + + let bin_root = self.out.join(host.triple).join("stage0"); + let clippy_stamp = bin_root.join(".clippy-stamp"); + let cargo_clippy = bin_root.join("bin").join(exe("cargo-clippy", host)); + if cargo_clippy.exists() && !program_out_of_date(&clippy_stamp, &date) { + return cargo_clippy; + } + + let filename = format!("clippy-{version}-{host}.tar.xz"); + self.download_component(DownloadSource::Dist, filename, "clippy-preview", date, "stage0"); + if self.should_fix_bins_and_dylibs() { + self.fix_bin_or_dylib(&cargo_clippy); + self.fix_bin_or_dylib(&cargo_clippy.with_file_name(exe("clippy-driver", host))); + } + + cargo_clippy + } + + /// NOTE: rustfmt is a completely different toolchain than the bootstrap compiler, so it can't + /// reuse target directories or artifacts pub(crate) fn maybe_download_rustfmt(&self) -> Option { let RustfmtMetadata { date, version } = self.stage0_metadata.rustfmt.as_ref()?; let channel = format!("{version}-{date}"); diff --git a/src/tools/bump-stage0/src/main.rs b/src/tools/bump-stage0/src/main.rs index b007f9a22c3..bd97b4eaa3e 100644 --- a/src/tools/bump-stage0/src/main.rs +++ b/src/tools/bump-stage0/src/main.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; const PATH: &str = "src/stage0.json"; -const COMPILER_COMPONENTS: &[&str] = &["rustc", "rust-std", "cargo"]; +const COMPILER_COMPONENTS: &[&str] = &["rustc", "rust-std", "cargo", "clippy-preview"]; const RUSTFMT_COMPONENTS: &[&str] = &["rustfmt-preview", "rustc"]; struct Tool {