Skip to content

Commit

Permalink
Use binary-dep-depinfo to resolve UI dependencies
Browse files Browse the repository at this point in the history
  • Loading branch information
camsteffen committed Sep 7, 2021
1 parent 01d3816 commit fec8628
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 79 deletions.
3 changes: 2 additions & 1 deletion .cargo/config
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ lintcheck = "run --target-dir lintcheck/target --package lintcheck --bin lintche
collect-metadata = "test --test dogfood --features metadata-collector-lint -- run_metadata_collection_lint --ignored"

[build]
rustflags = ["-Zunstable-options"]
# -Zbinary-dep-depinfo allows us to track which rlib files to use for compiling UI tests
rustflags = ["-Zunstable-options", "-Zbinary-dep-depinfo"]
13 changes: 9 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,7 @@ tempfile = { version = "3.1.0", optional = true }
cargo_metadata = "0.12"
compiletest_rs = { version = "0.6.0", features = ["tmp"] }
tester = "0.9"
serde = { version = "1.0", features = ["derive"] }
derive-new = "0.5"
regex = "1.4"
quote = "1"
syn = { version = "1", features = ["full"] }
# This is used by the `collect-metadata` alias.
filetime = "0.2"

Expand All @@ -45,6 +41,15 @@ filetime = "0.2"
# for more information.
rustc-workspace-hack = "1.0.0"

# UI test dependencies
clippy_utils = { path = "clippy_utils" }
derive-new = "0.5"
if_chain = "1.0"
itertools = "0.10.1"
quote = "1"
serde = { version = "1.0", features = ["derive"] }
syn = { version = "1", features = ["full"] }

[build-dependencies]
rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" }

Expand Down
144 changes: 70 additions & 74 deletions tests/compile-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use compiletest_rs as compiletest;
use compiletest_rs::common::Mode as TestMode;

use std::collections::HashMap;
use std::env::{self, remove_var, set_var, var_os};
use std::ffi::{OsStr, OsString};
use std::fs;
Expand All @@ -16,6 +17,28 @@ mod cargo;
// whether to run internal tests or not
const RUN_INTERNAL_TESTS: bool = cfg!(feature = "internal-lints");

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

// 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_utils;
extern crate derive_new;
extern crate if_chain;
extern crate itertools;
extern crate quote;
extern crate syn;

fn host_lib() -> PathBuf {
option_env!("HOST_LIBS").map_or(cargo::CARGO_TARGET_DIR.join(env!("PROFILE")), PathBuf::from)
}
Expand All @@ -24,72 +47,51 @@ fn clippy_driver_path() -> PathBuf {
option_env!("CLIPPY_DRIVER_PATH").map_or(cargo::TARGET_LIB.join("clippy-driver"), PathBuf::from)
}

// When we'll want to use `extern crate ..` for a dependency that is used
// both by the crate and the compiler itself, we can't simply pass -L flags
// as we'll get a duplicate matching versions. Instead, disambiguate with
// `--extern dep=path`.
// See https://github.com/rust-lang/rust-clippy/issues/4015.
//
// FIXME: We cannot use `cargo build --message-format=json` to resolve to dependency files.
// Because it would force-rebuild if the options passed to `build` command is not the same
// as what we manually pass to `cargo` invocation
fn third_party_crates() -> String {
use std::collections::HashMap;
static CRATES: &[&str] = &[
"clippy_lints",
"clippy_utils",
"if_chain",
"itertools",
"quote",
"regex",
"serde",
"serde_derive",
"syn",
];
let dep_dir = cargo::TARGET_LIB.join("deps");
let mut crates: HashMap<&str, Vec<PathBuf>> = HashMap::with_capacity(CRATES.len());
let mut flags = String::new();
for entry in fs::read_dir(dep_dir).unwrap().flatten() {
let path = entry.path();
if let Some(name) = try {
let name = path.file_name()?.to_str()?;
let (name, _) = name.strip_suffix(".rlib")?.strip_prefix("lib")?.split_once('-')?;
CRATES.iter().copied().find(|&c| c == name)?
} {
flags += &format!(" --extern {}={}", name, path.display());
crates.entry(name).or_default().push(path.clone());
/// 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.
fn extern_flags() -> String {
let current_exe_depinfo = {
let mut path = env::current_exe().unwrap();
path.set_extension("d");
std::fs::read_to_string(path).unwrap()
};
let mut crates: HashMap<&str, &str> = HashMap::with_capacity(TEST_DEPENDENCIES.len());
for line in current_exe_depinfo.lines() {
let name_path = try {
// each dependency is expected to have a Makefile rule like `/path/to/crate-hash.rlib:`
let path = line.strip_suffix(':')?;
let exts = ["rlib", "so", "dylib", "dll"];
let part = exts.iter().find_map(|ext| path.strip_suffix(ext)?.strip_suffix('.'))?;
let part = part.rsplit_once('-')?.0.rsplit_once(|c| matches!(c, '/' | '\\'))?.1;
// the "lib" prefix is not present for dll files
let name = part.strip_prefix("lib").unwrap_or(part);
TEST_DEPENDENCIES.contains(&name).then(|| (name, path))?
};
if let Some((name, path)) = name_path {
// 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);
}
}
crates.retain(|_, paths| paths.len() > 1);
if !crates.is_empty() {
let crate_names = crates.keys().map(|s| format!("`{}`", s)).collect::<Vec<_>>().join(", ");
// add backslashes for an easy copy-paste `rm` command
let paths = crates
.into_values()
.flatten()
.map(|p| strip_current_dir(&p).display().to_string())
.collect::<Vec<_>>()
.join(" \\\n");
// Check which action should be done in order to remove compiled deps.
// If pre-installed version of compiler is used, `cargo clean` will do.
// Otherwise (for bootstrapped compiler), the dependencies directory
// must be removed manually.
let suggested_action = if std::env::var_os("RUSTC_BOOTSTRAP").is_some() {
"removing the stageN-tools directory"
} else {
"running `cargo clean`"
};

panic!(
"\n----------------------------------------------------------------------\n\
ERROR: Found multiple rlibs for crates: {}\n\
Try {} or remove the following files:\n\n{}\n\n\
For details on this error see https://github.com/rust-lang/rust-clippy/issues/7343\n\
----------------------------------------------------------------------\n",
crate_names, suggested_action, paths
);
let not_found: Vec<&str> = TEST_DEPENDENCIES
.iter()
.copied()
.filter(|n| !crates.contains_key(n))
.collect();
if !not_found.is_empty() {
panic!("dependencies not found in depinfo: {:?}", not_found);
}
flags
crates
.into_iter()
.map(|(name, path)| format!("--extern {}={} ", name, path))
.collect()
}

fn default_config() -> compiletest::Config {
Expand All @@ -105,11 +107,14 @@ fn default_config() -> compiletest::Config {
config.compile_lib_path = path;
}

// Using `-L dependency={}` enforces that external dependencies are added with `--extern`.
// This is valuable because a) it allows us to monitor what external dependencies are used
// and b) it ensures that conflicting rlibs are resolved properly.
config.target_rustcflags = Some(format!(
"--emit=metadata -L {0} -L {1} -Dwarnings -Zui-testing {2}",
"--emit=metadata -L dependency={} -L dependency={} -Dwarnings -Zui-testing {}",
host_lib().join("deps").display(),
cargo::TARGET_LIB.join("deps").display(),
third_party_crates(),
extern_flags(),
));

config.build_base = host_lib().join("test_build_base");
Expand Down Expand Up @@ -316,12 +321,3 @@ impl Drop for VarGuard {
}
}
}

fn strip_current_dir(path: &Path) -> &Path {
if let Ok(curr) = env::current_dir() {
if let Ok(stripped) = path.strip_prefix(curr) {
return stripped;
}
}
path
}

0 comments on commit fec8628

Please sign in to comment.