Skip to content

Commit

Permalink
Auto merge of rust-lang#11045 - Alexendoo:extern-flags, r=flip1995
Browse files Browse the repository at this point in the history
Use depinfo to discover UI test dependencies

changelog: none

context: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Building.20test.20dependencies

This restores [the old `EXTERN_FLAGS` method](https://github.com/rust-lang/rust-clippy/blob/4cf5bdc60c7ccec3b5f395ee393615c224e28555/tests/compile-test.rs#L67-L75) of passing `--extern` flags for building UI tests with minor changes

- Unused deps were removed
- It's now a `Vec` of args instead of a command string
- It uses a `BTreeMap` so the extern flags are in alphabetical order and deterministic

I don't know if the `HOST_LIBS` part is still required, but I figured it best to leave it in for now. If the change is accepted we can take a look if it's needed in `rust-lang/rust` after the next sync

This isn't as pleasant as having a `Cargo.toml`, though there is something satisfying about knowing the dependencies are already built and not needing to invoke `cargo`

r? `@flip1995`
  • Loading branch information
bors committed Jul 12, 2023
2 parents 3be3fb7 + 4340a8a commit 53d2938
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 46 deletions.
11 changes: 11 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ walkdir = "2.3"
filetime = "0.2"
itertools = "0.10.1"

# UI test dependencies
clippy_utils = { path = "clippy_utils" }
derive-new = "0.5"
if_chain = "1.0"
quote = "1.0"
serde = { version = "1.0.125", features = ["derive"] }
syn = { version = "2.0", features = ["full"] }
futures = "0.3"
parking_lot = "0.12"
tokio = { version = "1", features = ["io-util"] }

[build-dependencies]
rustc_tools_util = "0.3.0"

Expand Down
23 changes: 0 additions & 23 deletions clippy_test_deps/Cargo.toml

This file was deleted.

14 changes: 0 additions & 14 deletions clippy_test_deps/src/lib.rs

This file was deleted.

118 changes: 109 additions & 9 deletions tests/compile-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,108 @@
#![feature(is_sorted)]
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![warn(rust_2018_idioms, unused_lifetimes)]
#![allow(unused_extern_crates)]

use compiletest::{status_emitter, CommandBuilder};
use ui_test as compiletest;
use ui_test::Mode as TestMode;

use std::collections::BTreeMap;
use std::env::{self, remove_var, set_var, var_os};
use std::ffi::{OsStr, OsString};
use std::fs;
use std::path::{Path, PathBuf};
use std::sync::LazyLock;
use test_utils::IS_RUSTC_TEST_SUITE;

// Test dependencies may need an `extern crate` here to ensure that they show up
// in the depinfo file (otherwise cargo thinks they are unused)
extern crate clippy_lints;
extern crate clippy_utils;
extern crate derive_new;
extern crate futures;
extern crate if_chain;
extern crate itertools;
extern crate parking_lot;
extern crate quote;
extern crate syn;
extern crate tokio;

/// All crates used in UI tests are listed here
static TEST_DEPENDENCIES: &[&str] = &[
"clippy_lints",
"clippy_utils",
"derive_new",
"futures",
"if_chain",
"itertools",
"parking_lot",
"quote",
"regex",
"serde_derive",
"serde",
"syn",
"tokio",
];

/// Produces a string with an `--extern` flag for all UI test crate
/// dependencies.
///
/// The dependency files are located by parsing the depinfo file for this test
/// module. This assumes the `-Z binary-dep-depinfo` flag is enabled. All test
/// dependencies must be added to Cargo.toml at the project root. Test
/// dependencies that are not *directly* used by this test module require an
/// `extern crate` declaration.
static EXTERN_FLAGS: LazyLock<Vec<String>> = LazyLock::new(|| {
let current_exe_depinfo = {
let mut path = env::current_exe().unwrap();
path.set_extension("d");
fs::read_to_string(path).unwrap()
};
let mut crates = BTreeMap::<&str, &str>::new();
for line in current_exe_depinfo.lines() {
// each dependency is expected to have a Makefile rule like `/path/to/crate-hash.rlib:`
let parse_name_path = || {
if line.starts_with(char::is_whitespace) {
return None;
}
let path_str = line.strip_suffix(':')?;
let path = Path::new(path_str);
if !matches!(path.extension()?.to_str()?, "rlib" | "so" | "dylib" | "dll") {
return None;
}
let (name, _hash) = path.file_stem()?.to_str()?.rsplit_once('-')?;
// the "lib" prefix is not present for dll files
let name = name.strip_prefix("lib").unwrap_or(name);
Some((name, path_str))
};
if let Some((name, path)) = parse_name_path() {
if TEST_DEPENDENCIES.contains(&name) {
// A dependency may be listed twice if it is available in sysroot,
// and the sysroot dependencies are listed first. As of the writing,
// this only seems to apply to if_chain.
crates.insert(name, path);
}
}
}
let not_found: Vec<&str> = TEST_DEPENDENCIES
.iter()
.copied()
.filter(|n| !crates.contains_key(n))
.collect();
assert!(
not_found.is_empty(),
"dependencies not found in depinfo: {not_found:?}\n\
help: Make sure the `-Z binary-dep-depinfo` rust flag is enabled\n\
help: Try adding to dev-dependencies in Cargo.toml\n\
help: Be sure to also add `extern crate ...;` to tests/compile-test.rs",
);
crates
.into_iter()
.map(|(name, path)| format!("--extern={name}={path}"))
.collect()
});

mod test_utils;

// whether to run internal tests or not
Expand All @@ -29,7 +120,6 @@ fn base_config(test_dir: &str) -> compiletest::Config {
} else {
compiletest::OutputConflictHandling::Error("cargo test -- -- --bless".into())
},
dependencies_crate_manifest_path: Some("clippy_test_deps/Cargo.toml".into()),
target: None,
out_dir: "target/ui_test".into(),
..compiletest::Config::rustc(Path::new("tests").join(test_dir))
Expand All @@ -44,10 +134,23 @@ fn base_config(test_dir: &str) -> compiletest::Config {
let deps_path = current_exe_path.parent().unwrap();
let profile_path = deps_path.parent().unwrap();

config.program.args.push("--emit=metadata".into());
config.program.args.push("-Aunused".into());
config.program.args.push("-Zui-testing".into());
config.program.args.push("-Dwarnings".into());
config.program.args.extend(
[
"--emit=metadata",
"-Aunused",
"-Zui-testing",
"-Dwarnings",
&format!("-Ldependency={}", deps_path.display()),
]
.map(OsString::from),
);

config.program.args.extend(EXTERN_FLAGS.iter().map(OsString::from));

if let Some(host_libs) = option_env!("HOST_LIBS") {
let dep = format!("-Ldependency={}", Path::new(host_libs).join("deps").display());
config.program.args.push(dep.into());
}

// Normalize away slashes in windows paths.
config.stderr_filter(r"\\", "/");
Expand Down Expand Up @@ -105,9 +208,7 @@ fn run_internal_tests() {
if !RUN_INTERNAL_TESTS {
return;
}
let mut config = base_config("ui-internal");
config.dependency_builder.args.push("--features".into());
config.dependency_builder.args.push("internal".into());
let config = base_config("ui-internal");
compiletest::run_tests(config).unwrap();
}

Expand Down Expand Up @@ -165,7 +266,6 @@ fn run_ui_cargo() {
.push(("RUSTFLAGS".into(), Some("-Dwarnings".into())));
// We need to do this while we still have a rustc in the `program` field.
config.fill_host_and_target().unwrap();
config.dependencies_crate_manifest_path = None;
config.program.program.set_file_name(if cfg!(windows) {
"cargo-clippy.exe"
} else {
Expand Down

0 comments on commit 53d2938

Please sign in to comment.