Auto merge of #7268 - benesch:dsym-uplifting, r=alexcrichton

Fix dSYM uplifting when symlink is broken

We were sporadically but persistently seeing errors like

    failed to link or copy `.../target/debugs/deps/bin-264030cd6c8a02be.dSYM` to `.../target/debug/bin.dSYM`

    Caused by:
      the source path is not an existing regular file

while running `cargo build`. Once the error occurs once, `cargo build`
will fail forever with the same error until `target/debug/bin.dSYM` is
manually unlinked.

After some investigation, I've determined that this situation arises
when the target of `bin.dSYM` goes missing. For example, if bin.dSYM is
pointing at `deps/bin-86908f0fa7f1440e.dSYM`, and
`deps/bin-86908f0fa7f1440e.dSYM` does not exist, then this error will
occur. I'm still not clear on why the underlying dSYM bundle
sporadically goes missing--perhaps it's the result of pressing Ctrl-C at
the wrong moment?--but Cargo should at least be able to handle this
situation better.

It turns out that Cargo was getting confused by the broken symlink. When
it goes to install the new `target/debug/bin.dSYM` link, it will remove
the existing `target/debug/bin.dSYM` file, if it exists. Unfortunately,
Cargo was checking whether the *target* of that symlink existed (e.g.,
`deps/bin-86908f0fa7f1440e.dSYM`, which in the buggy case would not
exist), rather than the symlink itself, deciding that there was no
existing symlink to remove, and crashing with EEXIST when trying to
install the new symlink.

This commit adjusts the existence check to evaluate whether the symlink
itself exists, rather than its target.

Note that while the symptoms are the same as #4671, the root cause is
unrelated.

Author: bors

src/cargo/core/compiler/mod.rs

@@ -486,7 +486,12 @@ fn hardlink_or_copy(src: &Path, dst: &Path) -> CargoResult<()> {
     if is_same_file(src, dst).unwrap_or(false) {
         return Ok(());
     }
-    if dst.exists() {
+
+    // NB: we can't use dst.exists(), as if dst is a broken symlink,
+    // dst.exists() will return false. This is problematic, as we still need to
+    // unlink dst in this case. symlink_metadata(dst).is_ok() will tell us
+    // whether dst exists *without* following symlinks, which is what we want.
+    if fs::symlink_metadata(dst).is_ok() {
         paths::remove_file(&dst)?;
     }
 

tests/testsuite/build.rs

@@ -4167,6 +4167,34 @@ fn uplift_dsym_of_bin_on_mac() {
     assert!(!p.target_debug_dir().join("d.dSYM").exists());
 }
 
+#[cargo_test]
+#[cfg(any(target_os = "macos", target_os = "ios"))]
+fn uplift_dsym_of_bin_on_mac_when_broken_link_exists() {
+    let p = project()
+        .file("src/main.rs", "fn main() { panic!(); }")
+        .build();
+    let dsym = p.target_debug_dir().join("foo.dSYM");
+
+    p.cargo("build").run();
+    assert!(dsym.is_dir());
+
+    // Simulate the situation where the underlying dSYM bundle goes missing
+    // but the uplifted symlink to it remains. This would previously cause
+    // builds to permanently fail until the bad symlink was manually removed.
+    dsym.rm_rf();
+    p.symlink(
+        p.target_debug_dir()
+            .join("deps")
+            .join("foo-baaaaaadbaaaaaad.dSYM"),
+        &dsym,
+    );
+    assert!(dsym.is_symlink());
+    assert!(!dsym.exists());
+
+    p.cargo("build").run();
+    assert!(dsym.is_dir());
+}
+
 #[cargo_test]
 #[cfg(all(target_os = "windows", target_env = "msvc"))]
 fn uplift_pdb_of_bin_on_windows() {