From b03eeda998d1aafa6f5f02b556d582bfb510218a Mon Sep 17 00:00:00 2001 From: Nikhil Benesch Date: Mon, 19 Aug 2019 16:45:42 -0400 Subject: [PATCH] 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. --- src/cargo/core/compiler/mod.rs | 7 ++++++- tests/testsuite/build.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 14e2f60adad..5567537f095 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/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)?; } diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index b351c2cf102..9a280ff3461 100644 --- a/tests/testsuite/build.rs +++ b/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() {