Skip to content

Commit ff32394

Browse files
committed
[wip] Fix x test clippy --stage 0
This makes a lot of changes. The basic idea is that clippy cannot depend on the sysroot it is *built* with to be the same as the sysroot it is *run* with. This assumption is hard-coded in lots of places throughout clippy, which is why the changes are so extensive. - Change clippy to use the sysroot of the run compiler (stage1, not stage0-sysroot) when running tests. This fixes an issue where clippy would try to load libraries from the beta compiler, then complain about a version mismatch. - Change clippy to be able to find host macros. Previously it assumed macros and dependencies were stored in the same target directory, which is not true for stage 0 (macros are in `release`, but other dependencies are in `release/x86_64-unknown-linux-gnu`). - Don't build rustc twice. The naive way to get dependencies into the stage1/ sysroot is to just compile everything we were doing before with a later compiler, but that takes a very long while to run. Instead: - Add a new crate in the clippy repo that has only the dependencies needed for UI tests. That allows compiling those crates without also compiling clippy and the whole compiler. - Don't require `clippy_lints` to be present when running tests in rust-lang/rust. That crate is only used for dogfood lints, which aren't even run in-tree. - Change various tests to remove usage of `rustc_*` crates. In all cases they were not testing the rustc crates themselves, they just happened to be conveniently in the sysroot. This currently breaks `--bless` because clippy_dev requires `rustc_lexer`. I hope to fix this shortly when the "build a single crate" PR lands; this PR is a WIP until then.
1 parent 0182fd9 commit ff32394

File tree

11 files changed

+123
-18
lines changed

11 files changed

+123
-18
lines changed

Cargo.lock

+18
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,7 @@ dependencies = [
677677
"termize",
678678
"tester",
679679
"tokio",
680+
"ui-test-dependencies",
680681
]
681682

682683
[[package]]
@@ -5524,6 +5525,23 @@ version = "0.1.3"
55245525
source = "registry+https://github.com/rust-lang/crates.io-index"
55255526
checksum = "56dee185309b50d1f11bfedef0fe6d036842e3fb77413abef29f8f8d1c5d4c1c"
55265527

5528+
[[package]]
5529+
name = "ui-test-dependencies"
5530+
version = "0.1.0"
5531+
dependencies = [
5532+
"derive-new",
5533+
"futures 0.3.19",
5534+
"if_chain",
5535+
"itertools",
5536+
"parking_lot 0.11.2",
5537+
"quote",
5538+
"regex",
5539+
"rustc-semver",
5540+
"serde",
5541+
"syn",
5542+
"tokio",
5543+
]
5544+
55275545
[[package]]
55285546
name = "ui_test"
55295547
version = "0.1.0"

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ members = [
88
"src/rustdoc-json-types",
99
"src/tools/cargotest",
1010
"src/tools/clippy",
11+
"src/tools/clippy/ui-test-dependencies",
1112
"src/tools/clippy/clippy_dev",
1213
"src/tools/compiletest",
1314
"src/tools/error_index_generator",

src/bootstrap/test.rs

+36-8
Original file line numberDiff line numberDiff line change
@@ -642,14 +642,38 @@ impl Step for Clippy {
642642
fn run(self, builder: &Builder<'_>) {
643643
let stage = self.stage;
644644
let host = self.host;
645-
let compiler = builder.compiler(stage, host);
645+
let build_compiler = builder.compiler(stage, host);
646646

647647
builder
648-
.ensure(tool::Clippy { compiler, target: self.host, extra_features: Vec::new() })
648+
.ensure(tool::Clippy {
649+
compiler: build_compiler,
650+
target: self.host,
651+
extra_features: Vec::new(),
652+
})
649653
.expect("in-tree tool");
654+
655+
// We linked Clippy with the previous stage, but now we need to run it with the *next* stage,
656+
// so that it looks at the proper sysroot when loading libstd.
657+
let run_compiler = builder.compiler(stage + 1, host);
658+
builder.ensure(compile::Std { compiler: run_compiler, target: run_compiler.host });
659+
// Clippy uses some of its own dependencies in UI tests. Build those (but only those) with the same toolchain we'll run clippy with.
660+
let mut dummy_bin_cargo = tool::prepare_tool_cargo(
661+
builder,
662+
run_compiler,
663+
// Use ToolRustc to make sure the artifacts are placed in the same directory that clippy looks in during tests.
664+
Mode::ToolRustc,
665+
host,
666+
"test",
667+
"src/tools/clippy/ui-test-dependencies",
668+
SourceType::InTree,
669+
&[],
670+
);
671+
dummy_bin_cargo.arg("-p=ui-test-dependencies");
672+
builder.run(&mut dummy_bin_cargo.into());
673+
650674
let mut cargo = tool::prepare_tool_cargo(
651675
builder,
652-
compiler,
676+
build_compiler,
653677
Mode::ToolRustc,
654678
host,
655679
"test",
@@ -658,14 +682,16 @@ impl Step for Clippy {
658682
&[],
659683
);
660684

661-
cargo.env("RUSTC_TEST_SUITE", builder.rustc(compiler));
662-
cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler));
663-
let host_libs = builder.stage_out(compiler, Mode::ToolRustc).join(builder.cargo_dir());
685+
cargo.env("RUSTC_TEST_SUITE", builder.rustc(run_compiler));
686+
cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(run_compiler));
687+
let host_libs = builder.stage_out(run_compiler, Mode::ToolRustc).join(builder.cargo_dir());
664688
cargo.env("HOST_LIBS", host_libs);
689+
let target_libs = builder.cargo_out(run_compiler, Mode::ToolRustc, run_compiler.host);
690+
cargo.env("TARGET_LIBS", target_libs);
665691

666692
cargo.arg("--").args(builder.config.cmd.test_args());
667693

668-
cargo.add_rustc_lib_path(builder, compiler);
694+
cargo.add_rustc_lib_path(builder, build_compiler);
669695

670696
if builder.try_run(&mut cargo.into()) {
671697
// The tests succeeded; nothing to do.
@@ -676,7 +702,9 @@ impl Step for Clippy {
676702
std::process::exit(1);
677703
}
678704

679-
let mut cargo = builder.cargo(compiler, Mode::ToolRustc, SourceType::InTree, host, "run");
705+
// FIXME: build rustc_lexer so we can support --bless.
706+
let mut cargo =
707+
builder.cargo(run_compiler, Mode::ToolRustc, SourceType::InTree, host, "run");
680708
cargo.arg("-p").arg("clippy_dev");
681709
// clippy_dev gets confused if it can't find `clippy/Cargo.toml`
682710
cargo.current_dir(&builder.src.join("src").join("tools").join("clippy"));

src/bootstrap/tool.rs

+12
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,18 @@ pub fn prepare_tool_cargo(
258258
// own copy
259259
cargo.env("LZMA_API_STATIC", "1");
260260

261+
// clippy tests need to know about the stage sysroot. Set them consistently while building to
262+
// avoid rebuilding when running tests.
263+
if path.ends_with("clippy") {
264+
// NOTE: this uses the compiler it will *run* with, not the compiler it is *built* with.
265+
// See impl Step for test::Clippy for more details.
266+
// NOTE: intentionally does *not* use `builder.compiler` - we don't actually want to build
267+
// the compiler or create the sysroot, just make sure that clippy knows which path to use
268+
// for the sysroot.
269+
let run_compiler = Compiler { stage: compiler.stage + 1, host: compiler.host };
270+
cargo.env("SYSROOT", builder.sysroot(run_compiler));
271+
}
272+
261273
// CFG_RELEASE is needed by rustfmt (and possibly other tools) which
262274
// import rustc-ap-rustc_attr which requires this to be set for the
263275
// `#[cfg(version(...))]` attribute.

src/tools/clippy/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ filetime = "0.2"
4040
rustc-workspace-hack = "1.0"
4141

4242
# UI test dependencies
43+
ui-test-dependencies = { path = "ui-test-dependencies" }
4344
clippy_utils = { path = "clippy_utils" }
4445
derive-new = "0.5"
4546
if_chain = "1.0"

src/tools/clippy/tests/compile-test.rs

+28-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#![feature(test)] // compiletest_rs requires this attribute
22
#![feature(once_cell)]
33
#![feature(is_sorted)]
4+
#![feature(drain_filter)]
45
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
56
#![warn(rust_2018_idioms, unused_lifetimes)]
67

@@ -70,7 +71,23 @@ extern crate tokio;
7071
/// dependencies that are not *directly* used by this test module require an
7172
/// `extern crate` declaration.
7273
static EXTERN_FLAGS: SyncLazy<String> = SyncLazy::new(|| {
73-
let current_exe_depinfo = {
74+
let current_exe_depinfo = if let Some(lib_deps) = option_env!("TARGET_LIBS") {
75+
let mut deps = std::fs::read_dir(Path::new(lib_deps).join("deps")).unwrap().into_iter().map(|x| {
76+
let mut line = x.unwrap().path().into_os_string().into_string().unwrap();
77+
line.push_str(":\n"); // for parse_name_path
78+
line
79+
}).collect();
80+
81+
if let Some(host_deps) = option_env!("HOST_LIBS") {
82+
deps += &*std::fs::read_dir(Path::new(host_deps).join("deps")).unwrap().into_iter().map(|x| {
83+
let mut line = x.unwrap().path().into_os_string().into_string().unwrap();
84+
line.push_str(":\n"); // for parse_name_path
85+
line
86+
}).collect::<String>();
87+
}
88+
89+
deps
90+
} else {
7491
let mut path = env::current_exe().unwrap();
7592
path.set_extension("d");
7693
fs::read_to_string(path).unwrap()
@@ -101,11 +118,15 @@ static EXTERN_FLAGS: SyncLazy<String> = SyncLazy::new(|| {
101118
}
102119
}
103120
}
104-
let not_found: Vec<&str> = TEST_DEPENDENCIES
121+
let mut not_found: Vec<&str> = TEST_DEPENDENCIES
105122
.iter()
106123
.copied()
107124
.filter(|n| !crates.contains_key(n))
108125
.collect();
126+
if option_env!("RUSTC_TEST_SUITE").is_some() {
127+
// Bootstrap doesn't build clippy_utils, but that's ok since clippy only uses it for dogfood tests.
128+
not_found.drain_filter(|x| *x == "clippy_utils");
129+
}
109130
assert!(
110131
not_found.is_empty(),
111132
"dependencies not found in depinfo: {:?}\n\
@@ -146,10 +167,14 @@ fn base_config(test_dir: &str) -> compiletest::Config {
146167
let host_libs = option_env!("HOST_LIBS")
147168
.map(|p| format!(" -L dependency={}", Path::new(p).join("deps").display()))
148169
.unwrap_or_default();
170+
let target_libs = option_env!("TARGET_LIBS")
171+
.map(|p| format!(" -L dependency={}", Path::new(p).join("deps").display()))
172+
.unwrap_or_default();
149173
config.target_rustcflags = Some(format!(
150-
"--emit=metadata -Dwarnings -Zui-testing -L dependency={}{}{}",
174+
"--emit=metadata -Dwarnings -Zui-testing -L dependency={}{}{}{}",
151175
deps_path.display(),
152176
host_libs,
177+
target_libs,
153178
&*EXTERN_FLAGS,
154179
));
155180

src/tools/clippy/tests/ui/auxiliary/proc_macro_derive.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub fn derive(_: TokenStream) -> TokenStream {
2020
let output = quote! {
2121
// Should not trigger `useless_attribute`
2222
#[allow(dead_code)]
23-
extern crate rustc_middle;
23+
extern crate alloc;
2424
};
2525
output
2626
}

src/tools/clippy/tests/ui/useless_attribute.fixed

+3-3
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
#![allow(dead_code)]
99
#![cfg_attr(feature = "cargo-clippy", allow(dead_code))]
1010
#[rustfmt::skip]
11-
#[allow(unused_imports)]
1211
#[allow(unused_extern_crates)]
13-
#[macro_use]
14-
extern crate rustc_middle;
12+
extern crate alloc;
1513

14+
// don't lint on unused_import when `macro_use` is present
15+
#[allow(unused_imports)]
1616
#[macro_use]
1717
extern crate proc_macro_derive;
1818

src/tools/clippy/tests/ui/useless_attribute.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
#[allow(dead_code)]
99
#[cfg_attr(feature = "cargo-clippy", allow(dead_code))]
1010
#[rustfmt::skip]
11-
#[allow(unused_imports)]
1211
#[allow(unused_extern_crates)]
13-
#[macro_use]
14-
extern crate rustc_middle;
12+
extern crate alloc;
1513

14+
// don't lint on unused_import when `macro_use` is present
15+
#[allow(unused_imports)]
1616
#[macro_use]
1717
extern crate proc_macro_derive;
1818

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
[package]
2+
name = "ui-test-dependencies"
3+
version = "0.1.0"
4+
edition = "2021"
5+
6+
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
7+
8+
[dependencies]
9+
# clippy_utils = { path = "../clippy_utils" }
10+
regex = "1.5"
11+
derive-new = "0.5"
12+
if_chain = "1.0"
13+
itertools = "0.10.1"
14+
quote = "1.0"
15+
serde = { version = "1.0.125", features = ["derive"] }
16+
syn = { version = "1.0", features = ["full"] }
17+
futures = "0.3"
18+
parking_lot = "0.11.2"
19+
tokio = { version = "1", features = ["io-util"] }
20+
rustc-semver = "1.1"

src/tools/clippy/ui-test-dependencies/src/lib.rs

Whitespace-only changes.

0 commit comments

Comments
 (0)