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

Make x clippy download and use beta clippy #107628

Closed
Closed
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
26 changes: 26 additions & 0 deletions src/bootstrap/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,25 @@ fn main() {
Err(_) => 0,
};

if verbose > 1 {
eprintln!("target: {target:?}");
eprintln!("version: {version:?}");
}

// Use a different compiler for build scripts, since there may not yet be a
// libstd for the real compiler to use. However, if Cargo is attempting to
// 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() {
if verbose > 1 {
eprintln!("Using snapshot complier");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*compiler, same below

}
("RUSTC_SNAPSHOT", "RUSTC_SNAPSHOT_LIBDIR")
} else {
if verbose > 1 {
eprintln!("Using real complier");
}
("RUSTC_REAL", "RUSTC_LIBDIR")
};
let stage = env::var("RUSTC_STAGE").unwrap_or_else(|_| {
Expand Down Expand Up @@ -75,6 +86,10 @@ fn main() {
cmd.arg("-Ztime-passes");
}
}

if crate_name == "build_script_build" && verbose > 0 {
eprintln!("building build scripts using sysroot {:?}", sysroot);
}
}

// Print backtrace in case of ICE
Expand Down Expand Up @@ -147,6 +162,16 @@ fn main() {
cmd.arg("--check-cfg=values(bootstrap)");
}

if let Ok(command) = env::var("RUSTC_COMMAND") {
if command == "clippy" && target.is_none() {
let libdir_string = libdir.to_string_lossy();
let (sysroot, _) = libdir_string.rsplit_once('/').unwrap();
if !args.iter().any(|arg| arg == "--sysroot") {
cmd.arg("--sysroot").arg(&sysroot);
}
}
}

if let Ok(map) = env::var("RUSTC_DEBUGINFO_MAP") {
cmd.arg("--remap-path-prefix").arg(&map);
}
Expand Down Expand Up @@ -220,6 +245,7 @@ fn main() {
env::join_paths(&dylib_path).unwrap(),
cmd,
);
eprintln!("{} SYSROOT: {:?}", prefix, env::var("SYSROOT"));
eprintln!("{} sysroot: {:?}", prefix, sysroot);
eprintln!("{} libdir: {:?}", prefix, libdir);
}
Expand Down
33 changes: 17 additions & 16 deletions src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,8 @@ def download_toolchain(self):
rustc_channel = self.stage0_compiler.version
bin_root = self.bin_root()

tarball_suffix = '.tar.gz' if lzma is None else '.tar.xz'

key = self.stage0_compiler.date
if self.rustc().startswith(bin_root) and \
(not os.path.exists(self.rustc()) or
Expand Down Expand Up @@ -602,22 +604,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.
Expand Down Expand Up @@ -740,6 +726,17 @@ def rustc_stamp(self):
"""
return os.path.join(self.bin_root(), '.rustc-stamp')

def clippy_stamp(self):
"""Return the path for .clippy-stamp

>>> rb = RustBuild()
>>> rb.build_dir = "build"
>>> rb.clippy_stamp() == os.path.join("build", "stage0", ".clippy-stamp")
True
"""
return os.path.join(self.bin_root(), '.clippy-stamp')


def program_out_of_date(self, stamp_path, key):
"""Check if the given program stamp is out of date"""
if not os.path.exists(stamp_path) or self.clean:
Expand Down Expand Up @@ -810,6 +807,10 @@ def rustc(self):
"""Return config path for rustc"""
return self.program_config('rustc')

def clippy(self):
"""Return config path for clippy"""
return self.program_config('cargo-clippy')

def program_config(self, program):
"""Return config path for the given program at the given stage

Expand Down
158 changes: 103 additions & 55 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::process::Command;
use std::time::{Duration, Instant};

use crate::cache::{Cache, Interned, INTERNER};
use crate::config::{SplitDebuginfo, TargetSelection};
use crate::config::{DryRun, SplitDebuginfo, TargetSelection};
use crate::doc;
use crate::flags::{Color, Subcommand};
use crate::install;
Expand All @@ -22,7 +22,9 @@ use crate::run;
use crate::setup;
use crate::test;
use crate::tool::{self, SourceType};
use crate::util::{self, add_dylib_path, add_link_lib_path, exe, libdir, output, t};
use crate::util::{
self, add_dylib_path, add_link_lib_path, dylib_path, dylib_path_var, exe, libdir, output, t,
};
use crate::EXTRA_CHECK_CFGS;
use crate::{check, compile, Crate};
use crate::{clean, dist};
Expand Down Expand Up @@ -1118,6 +1120,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.
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.initial_rustc.parent().unwrap().join("cargo-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 = dylib_path();
let run_compiler = self.compiler(build_compiler.stage + 1, self.build.build);
dylib_path.insert(0, self.sysroot(run_compiler).join("lib"));

let mut cmd = Command::new(cargo_clippy.unwrap());
cmd.env(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 @@ -1171,7 +1211,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 @@ -1293,6 +1338,24 @@ 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}"));
self.verbose_than(1, &format!("running cargo with mode {mode:?}"));
}

let mut rustflags = Rustflags::new(target);
if stage != 0 {
if let Ok(s) = env::var("CARGOFLAGS_NOT_BOOTSTRAP") {
Expand All @@ -1304,41 +1367,18 @@ 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::detail_exit_macro!(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);
// Only run clippy on a very limited subset of crates (in particular, not build scripts).
cargo.arg("-Zunstable-options");
}

let use_new_symbol_mangling = match self.config.rust_new_symbol_mangling {
Expand Down Expand Up @@ -1520,18 +1560,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 All @@ -1544,6 +1572,28 @@ impl<'a> Builder<'a> {
self.clear_if_dirty(&out_dir, &self.rustc(compiler));
}

// // Cargo doesn't pass `--sysroot` for build scripts and proc-macros, which is exactly when we want to use a different sysroot.
// if

if cmd == "clippy" {
let build_script_sysroot = if stage != 0 {
// Our fake rustc shim passes `-Zmark-unstable-if-unmarked` for stage != 0, which we can't
// replicate because clippy doesn't normally run the shim. We should talk with the clippy
// team about whether there's a way to do this; maybe cargo-clippy can invoke the shim
// which invokes clippy-driver?
cargo.env("RUSTC_CLIPPY_IGNORE_BUILD_SCRIPTS_AND_PROC_MACROS", "1");
self.initial_rustc.ancestors().nth(2).unwrap()
} else {
sysroot.clone()
};
// HACK: clippy will pass `--sysroot` to `RunCompiler` if and only if SYSROOT is set and
// `--sysroot is not already passed. We bypass clippy-driver altogether in stage 1
// because there's no way to set `-Zforce-unstable-if-unmarked` for only the correct
// crates, but for stage 0 build scripts and proc-macros we want to still set the right
// sysroot.
cargo.env("SYSROOT", build_script_sysroot);
}

// Customize the compiler we're running. Specify the compiler to cargo
// as our shim and then pass it some various options used to configure
// how the actual compiler itself is called.
Expand All @@ -1554,6 +1604,7 @@ impl<'a> Builder<'a> {
.env("RUSTBUILD_NATIVE_DIR", self.native_dir(target))
.env("RUSTC_REAL", self.rustc(compiler))
.env("RUSTC_STAGE", stage.to_string())
.env("RUSTC_COMMAND", cmd)
.env("RUSTC_SYSROOT", &sysroot)
.env("RUSTC_LIBDIR", &libdir)
.env("RUSTDOC", self.bootstrap_out.join("rustdoc"))
Expand All @@ -1567,11 +1618,8 @@ 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"));
}

cargo.env("RUSTC", self.bootstrap_out.join("rustc"));

// Dealing with rpath here is a little special, so let's go into some
// detail. First off, `-rpath` is a linker option on Unix platforms
Expand Down
4 changes: 3 additions & 1 deletion src/ci/docker/host-x86_64/x86_64-gnu-llvm-16/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,6 @@ ENV SCRIPT ../x.py --stage 2 test --exclude src/tools/tidy && \
# work.
#
../x.ps1 --stage 2 test tests/ui --pass=check \
--host='' --target=i686-unknown-linux-gnu
--host='' --target=i686-unknown-linux-gnu && \
# Run clippy just to make sure it doesn't error out; we don't actually want to gate on the warnings though.
python3 ../x.py --stage 1 clippy -Awarnings
Loading