Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x clippy #117595

Merged
merged 6 commits into from
Dec 16, 2023
Merged

x clippy #117595

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,22 +616,6 @@ def download_toolchain(self):
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.
Expand Down
52 changes: 45 additions & 7 deletions src/bootstrap/src/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
//! never get replaced.

use std::env;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::process::{Child, Command};
use std::time::Instant;

use dylib_util::{dylib_path, dylib_path_var};
use dylib_util::{dylib_path, dylib_path_var, exe};

#[path = "../utils/bin_helpers.rs"]
mod bin_helpers;
Expand All @@ -29,8 +29,10 @@ mod bin_helpers;
mod dylib_util;

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

let stage = bin_helpers::parse_rustc_stage();
let verbose = bin_helpers::parse_rustc_verbose();
Expand All @@ -45,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")
Expand All @@ -54,12 +57,47 @@ fn main() {
let sysroot = env::var_os("RUSTC_SYSROOT").expect("RUSTC_SYSROOT was not set");
let on_fail = env::var_os("RUSTC_ON_FAIL").map(Command::new);

let rustc = env::var_os(rustc).unwrap_or_else(|| panic!("{:?} was not set", rustc));
let rustc_real = env::var_os(rustc).unwrap_or_else(|| panic!("{:?} was not set", rustc));
let libdir = env::var_os(libdir).unwrap_or_else(|| panic!("{:?} was not set", libdir));
let mut dylib_path = dylib_path();
dylib_path.insert(0, PathBuf::from(&libdir));

let mut cmd = Command::new(rustc);
// if we're running clippy, trust cargo-clippy to set clippy-driver appropriately (and don't override it with rustc).
// 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?
// 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 {
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");
let arg0 = exe(args[0].to_str().expect("only utf8 paths are supported"), &host);
if Path::new(&arg0) == current_exe {
args.remove(0);
}
rustc_real
};

let mut cmd = if let Some(wrapper) = env::var_os("RUSTC_WRAPPER_REAL") {
let mut cmd = Command::new(wrapper);
cmd.arg(rustc_driver);
cmd
} else {
Command::new(rustc_driver)
};
cmd.args(&args).env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());

// Get the name of the crate we're compiling, if any.
Expand Down
16 changes: 9 additions & 7 deletions src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1809,10 +1809,6 @@ pub fn run_cargo(
is_check: bool,
rlib_only_metadata: bool,
) -> Vec<PathBuf> {
if builder.config.dry_run() {
return Vec::new();
}

// `target_root_dir` looks like $dir/$target/release
let target_root_dir = stamp.parent().unwrap();
// `target_deps_dir` looks like $dir/$target/release/deps
Expand Down Expand Up @@ -1919,6 +1915,10 @@ pub fn run_cargo(
crate::exit!(1);
}

if builder.config.dry_run() {
return Vec::new();
}

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

builder.verbose(&format!("running: {cargo:?}"));

if builder.config.dry_run() {
return true;
}

let mut child = match cargo.spawn() {
Ok(child) => child,
Err(e) => panic!("failed to execute command: {cargo:?}\nERROR: {e}"),
Expand Down
140 changes: 88 additions & 52 deletions src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,44 @@ impl<'a> Builder<'a> {
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())
Expand Down Expand Up @@ -1200,7 +1238,12 @@ impl<'a> Builder<'a> {
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);
Expand Down Expand Up @@ -1324,6 +1367,23 @@ impl<'a> Builder<'a> {
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") {
Expand All @@ -1335,41 +1395,16 @@ impl<'a> Builder<'a> {
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 {
Expand Down Expand Up @@ -1564,18 +1599,6 @@ impl<'a> Builder<'a> {

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.
//
Expand Down Expand Up @@ -1611,10 +1634,19 @@ impl<'a> Builder<'a> {
)
.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" {
cargo.env("RUSTC", self.bootstrap_out.join("rustc"));

// 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);
jyn514 marked this conversation as resolved.
Show resolved Hide resolved
}

// Dealing with rpath here is a little special, so let's go into some
Expand Down Expand Up @@ -1991,7 +2023,11 @@ impl<'a> Builder<'a> {
// Environment variables *required* throughout the build
//
// FIXME: should update code to not require this env var

// The host this new compiler will *run* on.
cargo.env("CFG_COMPILER_HOST_TRIPLE", target.triple);
// The host this new compiler is being *built* on.
cargo.env("CFG_COMPILER_BUILD_TRIPLE", compiler.host.triple);

// Set this for all builds to make sure doc builds also get it.
cargo.env("CFG_RELEASE_CHANNEL", &self.config.channel);
Expand Down
35 changes: 34 additions & 1 deletion src/bootstrap/src/core/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,10 @@ impl Config {
Some(other) => panic!("unsupported protocol {other} in {url}"),
None => panic!("no protocol in {url}"),
}
t!(std::fs::rename(&tempfile, dest_path));
t!(
std::fs::rename(&tempfile, dest_path),
format!("failed to rename {tempfile:?} to {dest_path:?}")
);
}

fn download_http_with_retries(&self, tempfile: &Path, url: &str, help_on_error: &str) {
Expand Down Expand Up @@ -375,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<PathBuf> {
let RustfmtMetadata { date, version } = self.stage0_metadata.rustfmt.as_ref()?;
let channel = format!("{version}-{date}");
Expand Down Expand Up @@ -544,6 +573,10 @@ impl Config {
key: &str,
destination: &str,
) {
if self.dry_run() {
return;
}

let cache_dst = self.out.join("cache");
let cache_dir = cache_dst.join(key);
if !cache_dir.exists() {
Expand Down
Loading
Loading