Use canonical paths when parsing dep-info.

Instead of treating Windows differently, this just always uses canonical paths
on all platforms.  This fixes a problem where symlinks were not treated
correctly on all platforms.

Switching rm_rf to the remove_dir_all crate because deleting symbolic links on
Windows is difficult.

Author: ehuss

Cargo.toml

@@ -47,6 +47,7 @@ memchr = "2.1.3"
 num_cpus = "1.0"
 opener = "0.4"
 percent-encoding = "2.0"
+remove_dir_all = "0.5.2"
 rustfix = "0.4.4"
 same-file = "1"
 semver = { version = "0.9.0", features = ["serde"] }

src/cargo/core/compiler/fingerprint.rs

@@ -1572,18 +1572,18 @@ pub fn translate_dep_info(
         .ok_or_else(|| internal("malformed dep-info format, no targets".to_string()))?
         .1;
 
+    let target_root = target_root.canonicalize()?;
+    let pkg_root = pkg_root.canonicalize()?;
     let mut new_contents = Vec::new();
     for file in deps {
-        let file = if cfg!(windows) && file.starts_with("\\\\?\\") {
-            // Remove Windows extended-length prefix, since functions like
-            // strip_prefix won't work if you mix with traditional dos paths.
-            PathBuf::from(&file[4..])
-        } else {
-            rustc_cwd.join(file)
-        };
-        let (ty, path) = if let Ok(stripped) = file.strip_prefix(target_root) {
+        // The path may be absolute or relative, canonical or not. Make sure
+        // it is canonicalized so we are comparing the same kinds of paths.
+        let canon_file = rustc_cwd.join(file).canonicalize()?;
+        let abs_file = rustc_cwd.join(file);
+
+        let (ty, path) = if let Ok(stripped) = canon_file.strip_prefix(&target_root) {
             (DepInfoPathType::TargetRootRelative, stripped)
-        } else if let Ok(stripped) = file.strip_prefix(pkg_root) {
+        } else if let Ok(stripped) = canon_file.strip_prefix(&pkg_root) {
             if !allow_package {
                 continue;
             }
@@ -1592,8 +1592,7 @@ pub fn translate_dep_info(
             // It's definitely not target root relative, but this is an absolute path (since it was
             // joined to rustc_cwd) and as such re-joining it later to the target root will have no
             // effect.
-            assert!(file.is_absolute(), "{:?} is absolute", file);
-            (DepInfoPathType::TargetRootRelative, &*file)
+            (DepInfoPathType::TargetRootRelative, &*abs_file)
         };
         new_contents.push(ty as u8);
         new_contents.extend(util::path2bytes(path)?);

tests/testsuite/dep_info.rs

@@ -1,7 +1,6 @@
+use crate::support::paths::{self, CargoPathExt};
 use crate::support::registry::Package;
-use crate::support::{
-    basic_bin_manifest, basic_manifest, main_file, paths, project, rustc_host, Project,
-};
+use crate::support::{basic_bin_manifest, basic_manifest, main_file, project, rustc_host, Project};
 use filetime::FileTime;
 use std::fs;
 use std::path::Path;
@@ -460,3 +459,43 @@ fn reg_dep_source_not_tracked() {
         },
     );
 }
+
+#[cargo_test]
+// Remove once https://github.com/rust-lang/rust/pull/61727 lands, and switch
+// to a `nightly` check.
+#[ignore]
+fn canonical_path() {
+    if !crate::support::symlink_supported() {
+        return;
+    }
+    Package::new("regdep", "0.1.0")
+        .file("src/lib.rs", "pub fn f() {}")
+        .publish();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+
+            [dependencies]
+            regdep = "0.1"
+            "#,
+        )
+        .file("src/lib.rs", "pub fn f() { regdep::f(); }")
+        .build();
+
+    let real = p.root().join("real_target");
+    real.mkdir_p();
+    p.symlink(real, "target");
+
+    p.cargo("build").run();
+
+    assert_deps_contains(
+        &p,
+        "target/debug/.fingerprint/foo-*/dep-lib-foo-*",
+        &[(1, "src/lib.rs"), (2, "debug/deps/libregdep-*.rlib")],
+    );
+}

tests/testsuite/support/mod.rs

@@ -448,6 +448,29 @@ impl Project {
             .write_all(contents.replace("#", "").as_bytes())
             .unwrap();
     }
+
+    pub fn symlink(&self, src: impl AsRef<Path>, dst: impl AsRef<Path>) {
+        let src = self.root().join(src.as_ref());
+        let dst = self.root().join(dst.as_ref());
+        #[cfg(unix)]
+        {
+            if let Err(e) = os::unix::fs::symlink(&src, &dst) {
+                panic!("failed to symlink {:?} to {:?}: {:?}", src, dst, e);
+            }
+        }
+        #[cfg(windows)]
+        {
+            if src.is_dir() {
+                if let Err(e) = os::windows::fs::symlink_dir(&src, &dst) {
+                    panic!("failed to symlink {:?} to {:?}: {:?}", src, dst, e);
+                }
+            } else {
+                if let Err(e) = os::windows::fs::symlink_file(&src, &dst) {
+                    panic!("failed to symlink {:?} to {:?}: {:?}", src, dst, e);
+                }
+            }
+        }
+    }
 }
 
 // Generates a project layout
@@ -1746,3 +1769,31 @@ pub fn clippy_is_available() -> bool {
         true
     }
 }
+
+#[cfg(windows)]
+pub fn symlink_supported() -> bool {
+    let src = paths::root().join("symlink_src");
+    fs::write(&src, "").unwrap();
+    let dst = paths::root().join("symlink_dst");
+    let result = match os::windows::fs::symlink_file(&src, &dst) {
+        Ok(_) => {
+            fs::remove_file(&dst).unwrap();
+            true
+        }
+        Err(e) => {
+            eprintln!(
+                "symlinks not supported: {:?}\n\
+                 Windows 10 users should enable developer mode.",
+                e
+            );
+            false
+        }
+    };
+    fs::remove_file(&src).unwrap();
+    return result;
+}
+
+#[cfg(not(windows))]
+pub fn symlink_supported() -> bool {
+    true
+}

tests/testsuite/support/paths.rs

@@ -113,22 +113,11 @@ impl CargoPathExt for Path {
      * care all that much for our tests
      */
     fn rm_rf(&self) {
-        if !self.exists() {
-            return;
-        }
-
-        for file in t!(fs::read_dir(self)) {
-            let file = t!(file);
-            if file.file_type().map(|m| m.is_dir()).unwrap_or(false) {
-                file.path().rm_rf();
-            } else {
-                // On windows we can't remove a readonly file, and git will
-                // often clone files as readonly. As a result, we have some
-                // special logic to remove readonly files on windows.
-                do_op(&file.path(), "remove file", |p| fs::remove_file(p));
+        if self.exists() {
+            if let Err(e) = remove_dir_all::remove_dir_all(self) {
+                panic!("failed to remove {:?}: {:?}", self, e)
             }
         }
-        do_op(self, "remove dir", |p| fs::remove_dir(p));
     }
 
     fn mkdir_p(&self) {