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.py clippy download and use beta clippy #106394

Closed
wants to merge 5 commits into from
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");
}
("RUSTC_SNAPSHOT", "RUSTC_SNAPSHOT_LIBDIR")
} else {
if verbose > 1 {
eprintln!("Using real complier");
}
("RUSTC_REAL", "RUSTC_LIBDIR")
};
let stage = env::var("RUSTC_STAGE").expect("RUSTC_STAGE was not set");
Expand All @@ -70,6 +81,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 @@ -142,6 +157,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 @@ -215,6 +240,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
41 changes: 40 additions & 1 deletion src/bootstrap/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,14 @@ 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
self.program_out_of_date(self.rustc_stamp(), key)):
if os.path.exists(bin_root):
shutil.rmtree(bin_root)
tarball_suffix = '.tar.gz' if lzma is None else '.tar.xz'
filename = "rust-std-{}-{}{}".format(
rustc_channel, self.build, tarball_suffix)
pattern = "rust-std-{}".format(self.build)
Expand All @@ -451,6 +452,29 @@ def download_toolchain(self):
with output(self.rustc_stamp()) as rust_stamp:
rust_stamp.write(key)

clippy_path = self.clippy()
clippy_needs_download = not os.path.exists(clippy_path) \
or self.program_out_of_date(self.clippy_stamp(), key)
if clippy_path.startswith(bin_root) and clippy_needs_download:
# download Clippy
# the component name is clippy, but the bin containing folder name is clippy-preview
filename = self._format_component_filename(
"clippy",
rustc_channel,
self.build,
tarball_suffix
)
self._download_component_helper(filename, "clippy-preview", tarball_suffix)
if self.should_fix_bins_and_dylibs():
self.fix_bin_or_dylib("{}/bin/clippy-driver".format(bin_root))
self.fix_bin_or_dylib("{}/bin/cargo-clippy".format(bin_root))

with output(self.clippy_stamp()) as clippy_stamp:
clippy_stamp.write(key)

def _format_component_filename(self, component_name, version, build, tarball_suffix):
return "{}-{}-{}{}".format(component_name, version, build, tarball_suffix)

def _download_component_helper(
self, filename, pattern, tarball_suffix,
):
Expand Down Expand Up @@ -592,6 +616,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 @@ -665,6 +700,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 @@ -12,7 +12,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 @@ -21,7 +21,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 @@ -1061,6 +1063,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 @@ -1114,7 +1154,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 @@ -1236,6 +1281,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 @@ -1247,41 +1310,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(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 @@ -1455,18 +1495,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 @@ -1479,6 +1507,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 @@ -1489,6 +1539,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 @@ -1502,11 +1553,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
2 changes: 2 additions & 0 deletions src/ci/docker/host-x86_64/x86_64-gnu-llvm-13/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,7 @@ ENV SCRIPT ../x.py --stage 2 test --exclude src/tools/tidy && \
#
../x.ps1 --stage 2 test tests/ui --pass=check \
--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 && \
# Run tidy at the very end, after all the other tests.
python2.7 ../x.py --stage 2 test src/tools/tidy
Loading