Correctly import dependencies instead of exposing all transitive dependencies to tests

Author: oli-obk

test-cargo-miri/Cargo.toml

@@ -18,9 +18,6 @@ issue_1760 = { path = "issue-1760" }
 issue_rust_86261 = { path = "issue-rust-86261" }
 
 [dev-dependencies]
-rand = { version = "0.8", features = ["small_rng"] }
-getrandom_1 = { package = "getrandom", version = "0.1" }
-getrandom_2 = { package = "getrandom", version = "0.2" }
 serde_derive = "1.0" # not actually used, but exercises some unique code path (`--extern` .so file)
 page_size = "0.4.1"
 

test-cargo-miri/tests/test.rs

@@ -20,28 +20,6 @@ fn does_not_work_on_miri() {
     assert!(&x as *const _ as usize % 4 < 4);
 }
 
-// We also use this to test some external crates, that we cannot depend on in the compiletest suite.
-
-#[test]
-fn entropy_rng() {
-    // Test `getrandom` directly (in multiple different versions).
-    let mut data = vec![0; 16];
-    getrandom_1::getrandom(&mut data).unwrap();
-    getrandom_2::getrandom(&mut data).unwrap();
-
-    // Try seeding with "real" entropy.
-    let mut rng = SmallRng::from_entropy();
-    let _val = rng.gen::<i32>();
-    let _val = rng.gen::<isize>();
-    let _val = rng.gen::<i128>();
-
-    // Also try per-thread RNG.
-    let mut rng = rand::thread_rng();
-    let _val = rng.gen::<i32>();
-    let _val = rng.gen::<isize>();
-    let _val = rng.gen::<i128>();
-}
-
 #[test]
 fn cargo_env() {
     assert_eq!(env!("CARGO_PKG_NAME"), "cargo-miri-test");

test_dependencies/Cargo.lock

@@ -12,4 +12,8 @@ edition = "2021"
 tokio = { version = "1.0", features = ["full"] }
 libc = "0.2"
 
+getrandom_1 = { package = "getrandom", version = "0.1" }
+getrandom_2 = { package = "getrandom", version = "0.2" }
+rand = { version = "0.8", features = ["small_rng"] }
+
 [workspace]

test_dependencies/Cargo.toml

@@ -0,0 +1,20 @@
+use rand::{rngs::SmallRng, Rng, SeedableRng};
+
+fn main() {
+    // Test `getrandom` directly (in multiple different versions).
+    let mut data = vec![0; 16];
+    getrandom_1::getrandom(&mut data).unwrap();
+    getrandom_2::getrandom(&mut data).unwrap();
+
+    // Try seeding with "real" entropy.
+    let mut rng = SmallRng::from_entropy();
+    let _val = rng.gen::<i32>();
+    let _val = rng.gen::<isize>();
+    let _val = rng.gen::<i128>();
+
+    // Also try per-thread RNG.
+    let mut rng = rand::thread_rng();
+    let _val = rng.gen::<i32>();
+    let _val = rng.gen::<isize>();
+    let _val = rng.gen::<i128>();
+}

tests/pass/random.rs

@@ -1,34 +1,49 @@
 use color_eyre::eyre::{bail, Result};
 use std::{
+    collections::{HashMap, HashSet},
     path::{Path, PathBuf},
     process::Command,
 };
 
 use crate::Config;
 
+#[derive(Default, Debug)]
+pub struct Dependencies {
+    /// All paths that must be imported with `-L dependency=`. This is for
+    /// finding proc macros run on the host and dependencies for the target.
+    pub import_paths: Vec<PathBuf>,
+    /// The name as chosen in the `Cargo.toml` and its corresponding rmeta file.
+    pub dependencies: Vec<(String, PathBuf)>,
+}
+
 /// Compiles dependencies and returns the crate names and corresponding rmeta files.
-pub fn build_dependencies(config: &Config) -> Result<Vec<(String, PathBuf)>> {
+pub fn build_dependencies(config: &Config) -> Result<Dependencies> {
     let manifest_path = match &config.dependencies_crate_manifest_path {
         Some(path) => path,
-        None => return Ok(vec![]),
+        None => return Ok(Default::default()),
     };
     let (program, args, envs): (&Path, &[_], &[_]) = match &config.dependency_builder {
         Some(db) => (&db.program, &db.args, &db.envs),
         None => (Path::new("cargo"), &[], &[]),
     };
     let mut build = Command::new(program);
-
-    // Avoid poisoning the target directory and causing unnecessary rebuilds.
-    build.env_remove("RUSTFLAGS");
-
-    build.envs(envs.iter().map(|(k, v)| (k, v)));
     build.args(args);
     build.arg("run");
-    if let Some(target) = &config.target {
-        build.arg(format!("--target={target}"));
-    }
+
+    // Reusable closure for setting up the environment both for artifact generation and `cargo_metadata`
+    let setup_command = |cmd: &mut Command| {
+        // Avoid poisoning the target directory and causing unnecessary rebuilds.
+        cmd.env_remove("RUSTFLAGS");
+
+        cmd.envs(envs.iter().map(|(k, v)| (k, v)));
+        if let Some(target) = &config.target {
+            cmd.arg(format!("--target={target}"));
+        }
+        cmd.arg(format!("--manifest-path={}", manifest_path.display()));
+    };
+
+    setup_command(&mut build);
     build
-        .arg(format!("--manifest-path={}", manifest_path.display()))
         .arg("--target-dir=target/test_dependencies")
         .arg("--message-format=json")
         .arg("-Zunstable-options");
@@ -41,24 +56,79 @@ pub fn build_dependencies(config: &Config) -> Result<Vec<(String, PathBuf)>> {
         bail!("failed to compile dependencies:\nstderr:\n{stderr}\n\nstdout:{stdout}");
     }
 
+    // Collect all artifacts generated
     let output = output.stdout;
     let output = String::from_utf8(output)?;
-    Ok(output
+    let mut import_paths: HashSet<PathBuf> = HashSet::new();
+    let mut artifacts: HashMap<_, _> = output
         .lines()
         .filter_map(|line| {
             let message = serde_json::from_str::<cargo_metadata::Message>(line).ok()?;
             if let cargo_metadata::Message::CompilerArtifact(artifact) = message {
+                for filename in &artifact.filenames {
+                    import_paths.insert(filename.parent().unwrap().into());
+                }
                 let filename = artifact
                     .filenames
                     .into_iter()
                     .find(|filename| filename.extension() == Some("rmeta"))?;
-                Some((
-                    artifact.package_id.repr.split_once(' ').unwrap().0.to_string(),
-                    filename.into_std_path_buf(),
-                ))
+                Some((artifact.package_id, filename.into_std_path_buf()))
             } else {
                 None
             }
         })
-        .collect())
+        .collect();
+
+    // Check which crates are mentioned in the crate itself
+    let mut metadata = cargo_metadata::MetadataCommand::new().cargo_command();
+    setup_command(&mut metadata);
+    let output = metadata.output()?;
+
+    if !output.status.success() {
+        let stdout = String::from_utf8(output.stdout)?;
+        let stderr = String::from_utf8(output.stderr)?;
+        bail!("failed to run cargo-metadata:\nstderr:\n{stderr}\n\nstdout:{stdout}");
+    }
+
+    let output = output.stdout;
+    let output = String::from_utf8(output)?;
+
+    for line in output.lines() {
+        if !line.starts_with('{') {
+            continue;
+        }
+        let metadata: cargo_metadata::Metadata = serde_json::from_str(line)?;
+        // Only take artifacts that are defined in the Cargo.toml
+
+        // First, find the root artifact
+        let root = metadata
+            .packages
+            .iter()
+            .find(|package| package.manifest_path.as_std_path() == manifest_path)
+            .unwrap();
+
+        // Then go over all of its dependencies
+        let dependencies = root
+            .dependencies
+            .iter()
+            .map(|package| {
+                // Get the id for the package matching the version requirement of the dep
+                let id = &metadata
+                    .packages
+                    .iter()
+                    .find(|&dep| dep.name == package.name && package.req.matches(&dep.version))
+                    .expect("dependency does not exist")
+                    .id;
+                // Return the name chosen in `Cargo.toml` and the path to the corresponding artifact
+                (
+                    package.rename.clone().unwrap_or_else(|| package.name.clone()),
+                    artifacts.remove(id).expect("package without artifact"),
+                )
+            })
+            .collect();
+        let import_paths = import_paths.into_iter().collect();
+        return Ok(Dependencies { dependencies, import_paths });
+    }
+
+    bail!("no json found in cargo-metadata output")
 }

ui_test/src/dependencies.rs

@@ -75,11 +75,13 @@ pub fn run_tests(mut config: Config) -> Result<()> {
     let target = config.target.clone().unwrap_or_else(|| config.get_host());
 
     let dependencies = build_dependencies(&config)?;
-    for (name, dependency) in dependencies {
+    for (name, dependency) in dependencies.dependencies {
         config.args.push("--extern".into());
         config.args.push(format!("{name}={}", dependency.display()).into());
+    }
+    for import_path in dependencies.import_paths {
         config.args.push("-L".into());
-        config.args.push(dependency.parent().unwrap().display().to_string().into());
+        config.args.push(import_path.into());
     }
     let config = config;