[hermes] Stop copying shadow crate; run in-place

gherrit-pr-id: G5bgufougyvpozncgq4yokgx3sie6wfy7

Author: joshlf

tools/hermes/src/charon.rs

@@ -46,7 +46,7 @@ pub fn run_charon(args: &Args, roots: &Roots, packages: &[HermesArtifact]) -> Re
         // Ensure cargo emits json msgs which charon-driver natively generates
         cmd.arg("--message-format=json");
 
-        cmd.arg("--manifest-path").arg(&artifact.shadow_manifest_path);
+        cmd.arg("--manifest-path").arg(&artifact.manifest_path);
 
         match artifact.target_kind {
             HermesTargetKind::Lib
@@ -117,7 +117,7 @@ pub fn run_charon(args: &Args, roots: &Roots, packages: &[HermesArtifact]) -> Re
             let reader = BufReader::new(stdout);
 
             let mut mapper = crate::diagnostics::DiagnosticMapper::new(
-                artifact.shadow_manifest_path.parent().unwrap().to_path_buf(),
+                roots.workspace.clone(),
                 roots.workspace.clone(),
             );
 

tools/hermes/src/shadow.rs

@@ -7,7 +7,6 @@ use std::{
 };
 
 use anyhow::{Context, Result};
-use cargo_metadata::PackageName;
 use dashmap::DashSet;
 use walkdir::WalkDir;
 
@@ -19,8 +18,8 @@ use crate::{
 pub struct HermesArtifact {
     pub name: HermesTargetName,
     pub target_kind: HermesTargetKind,
-    /// The path to the shadow crate's `Cargo.toml`.
-    pub shadow_manifest_path: PathBuf,
+    /// The path to the crate's `Cargo.toml`.
+    pub manifest_path: PathBuf,
     pub start_from: Vec<String>,
 }
 
@@ -37,10 +36,9 @@ impl HermesArtifact {
         }
 
         // Double-hash to make sure we can distinguish between e.g.
-        // (shadow_manifest_path, target_name) = ("abc", "def") and ("ab",
-        // "cdef"), which would hash identically if we just hashed their
-        // concatenation.
-        let h0 = hash(&self.shadow_manifest_path);
+        // (manifest_path, target_name) = ("abc", "def") and ("ab", "cdef"),
+        // which would hash identically if we just hashed their concatenation.
+        let h0 = hash(&self.manifest_path);
         let h1 = hash(&self.name.target_name);
         let h = hash(&[h0, h1]);
         format!("{}-{}-{:08x}.llbc", self.name.package_name, self.name.target_name, h)
@@ -54,29 +52,12 @@ impl HermesArtifact {
 /// 2. Creates symlinks for the remaining skeleton.
 pub fn build_shadow_crate(roots: &Roots) -> Result<Vec<HermesArtifact>> {
     log::trace!("build_shadow_crate({:?})", roots);
-    let shadow_root = roots.shadow_root();
-    if shadow_root.exists() {
-        fs::remove_dir_all(&shadow_root).context("Failed to clear shadow root")?;
-    }
-    fs::create_dir_all(&shadow_root).context("Failed to create shadow root")?;
 
     let visited_paths = DashSet::new();
     let (err_tx, err_rx) = mpsc::channel();
     // Items are `((PackageName, TargetName), ItemPath)`.
     let (path_tx, path_rx) = mpsc::channel::<(HermesTargetName, String)>();
 
-    let mut shadow_manifest_paths: HashMap<PackageName, PathBuf> = HashMap::new();
-    for target in &roots.roots {
-        if !shadow_manifest_paths.contains_key(&target.name.package_name) {
-            let relative_manifest_path = find_package_root(&target.src_path)?
-                .strip_prefix(&roots.workspace)
-                .context("Package root outside workspace")?
-                .join("Cargo.toml");
-            let shadow_manifest_path = shadow_root.join(&relative_manifest_path);
-            shadow_manifest_paths.insert(target.name.package_name.clone(), shadow_manifest_path);
-        }
-    }
-
     let monitor_handle = std::thread::spawn(move || {
         let mut error_count = 0;
         for err in err_rx {
@@ -137,23 +118,17 @@ pub fn build_shadow_crate(roots: &Roots) -> Result<Vec<HermesArtifact>> {
         return Err(anyhow::anyhow!("Aborting due to {} previous errors.", count));
     }
 
-    let skip_paths: HashSet<PathBuf> = visited_paths.into_iter().collect();
-    create_symlink_skeleton(&roots.workspace, &shadow_root, &roots.cargo_target_dir, &skip_paths)?;
-
     Ok(roots
         .roots
         .iter()
         .filter_map(|target| {
-            // We know that every root has a shadow manifest path, because we
-            // inserted one for each package that has a root.
-            let shadow_manifest_path =
-                shadow_manifest_paths.get(&target.name.package_name).unwrap();
+            let manifest_path = find_package_root(&target.src_path).ok()?.join("Cargo.toml");
             let start_from = entry_points.remove(&target.name)?;
 
             Some(HermesArtifact {
                 name: target.name.clone(),
                 target_kind: target.kind,
-                shadow_manifest_path: shadow_manifest_path.clone(),
+                manifest_path,
                 start_from,
             })
         })
@@ -324,61 +299,3 @@ fn resolve_module_path(
 
     None
 }
-
-fn create_symlink_skeleton(
-    source_root: &Path,
-    dest_root: &Path,
-    target_dir: &Path,
-    skip_paths: &HashSet<PathBuf>,
-) -> Result<()> {
-    log::trace!("create_symlink_skeleton(source_root: {:?}, dest_root: {:?}, target_dir: {:?}, skip_paths_count: {})", source_root, dest_root, target_dir, skip_paths.len());
-    let walker = WalkDir::new(source_root)
-        .follow_links(false) // Security: don't follow symlinks out of the root.
-        .into_iter();
-
-    let filter = |e: &walkdir::DirEntry| {
-        let p = e.path();
-        // Normally, we expect the `dest_root` to be inside of `target_dir`,
-        // so checking both is redundant. However, if we ever add the option
-        // for the user to manually specify a `dest_root` that is outside of
-        // `target_dir`, this check will prevent us from infinitely recursing
-        // into the source root.
-        p != target_dir && p != dest_root && e.file_name() != ".git"
-    };
-
-    for entry in walker.filter_entry(filter) {
-        let entry = entry.context("Failed to read directory entry")?;
-        let src_path = entry.path();
-
-        let relative_path =
-            src_path.strip_prefix(source_root).context("File is not inside source root")?;
-        let dest_path = dest_root.join(relative_path);
-
-        if entry.file_type().is_dir() {
-            fs::create_dir_all(&dest_path)
-                .with_context(|| format!("Failed to create shadow directory: {:?}", dest_path))?;
-            continue;
-        }
-
-        if entry.file_type().is_file() && !skip_paths.contains(src_path) {
-            make_symlink(src_path, &dest_path)?;
-        }
-    }
-
-    Ok(())
-}
-
-#[cfg(unix)]
-fn make_symlink(original: &Path, link: &Path) -> Result<()> {
-    std::os::unix::fs::symlink(original, link)
-        .with_context(|| format!("Failed to symlink {:?} -> {:?}", original, link))
-}
-
-#[cfg(windows)]
-fn make_symlink(original: &Path, link: &Path) -> Result<()> {
-    // Windows treats file and directory symlinks differently.
-    // Since we only call this on files (checking is_file above),
-    // we use symlink_file.
-    std::os::windows::fs::symlink_file(original, link)
-        .with_context(|| format!("Failed to symlink {:?} -> {:?}", original, link))
-}

tools/hermes/tests/integration.rs

@@ -95,11 +95,9 @@ exec "{1}" "$@"
     let mock_json_file = test_case_root.join("mock_charon_output.json");
     if mock_json_file.exists() {
         // Instead of writing the mock json to the shadow root (which gets cleared by build_shadow_crate), write it to the test workspace root!
-        let shadow_root =
-            sandbox_root.join("target").join("hermes").join("hermes_test_target").join("shadow");
         // We still need the path for mapping `[SHADOW_ROOT]` correctly!
         // But we construct it manually since it might not be created yet:
-        let abs_shadow_root = std::env::current_dir().unwrap().join(&shadow_root);
+        let abs_shadow_root = std::env::current_dir().unwrap().join(&sandbox_root);
         let abs_test_case_root =
             test_case_root.canonicalize().unwrap_or_else(|_| test_case_root.to_path_buf());
 
@@ -187,13 +185,6 @@ exec "{1}" "$@"
         }
     }
 
-    // Tests can specify the expected shadow crate content.
-    let actual_shadow = sandbox_root.join("target/hermes/hermes_test_target/shadow");
-    let expected_shadow = test_case_root.join("expected");
-    if expected_shadow.exists() {
-        assert_directories_match(&expected_shadow, &actual_shadow)?;
-    }
-
     // Load Config
     let mut config = TestConfig { artifact: vec![], command: vec![] };
     let config_file = test_case_root.join("expected_config.toml");
@@ -324,26 +315,6 @@ fn assert_artifacts_match(
     Ok(())
 }
 
-fn assert_directories_match(expected: &Path, actual: &Path) -> std::io::Result<()> {
-    for entry in WalkDir::new(expected) {
-        let entry = entry?;
-        if !entry.file_type().is_file() {
-            continue;
-        }
-        let rel = entry.path().strip_prefix(expected).unwrap();
-        let act = actual.join(rel);
-        if !act.exists() {
-            panic!("Missing file {:?}", rel);
-        }
-        let e_txt = fs::read_to_string(entry.path())?.replace("\r\n", "\n");
-        let a_txt = fs::read_to_string(&act)?.replace("\r\n", "\n");
-        if e_txt != a_txt {
-            panic!("Mismatch in {:?}", rel);
-        }
-    }
-    Ok(())
-}
-
 fn copy_dir_contents(src: &Path, dst: &Path) -> std::io::Result<()> {
     fs::create_dir_all(dst)?;
     for entry in fs::read_dir(src)? {