Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dSYM uplifting when symlink is broken #7268

Merged
merged 1 commit into from
Aug 20, 2019

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Aug 19, 2019

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.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2019
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 rust-lang#4671, the root cause is
unrelated.
@alexcrichton
Copy link
Member

@bors: r+

Nice!

@bors
Copy link
Collaborator

bors commented Aug 20, 2019

📌 Commit b03eeda has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 20, 2019
@bors
Copy link
Collaborator

bors commented Aug 20, 2019

⌛ Testing commit b03eeda with merge 027ab62...

bors added a commit that referenced this pull request Aug 20, 2019
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.
@bors
Copy link
Collaborator

bors commented Aug 20, 2019

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing 027ab62 to master...

@bors bors merged commit b03eeda into rust-lang:master Aug 20, 2019
@benesch
Copy link
Contributor Author

benesch commented Aug 20, 2019

Woo, thanks @alexcrichton!

@benesch benesch deleted the dsym-uplifting branch August 20, 2019 14:39
bors added a commit to rust-lang/rust that referenced this pull request Aug 27, 2019
Update cargo

Update cargo

10 commits in 3f700ec43ce72305eb5315cfc710681f3469d4b4..22f7dd0495cd72ce2082d318d5a9b4dccb9c5b8c
2019-08-19 22:43:12 +0000 to 2019-08-27 16:10:51 +0000
- Update and improve zsh completion (rust-lang/cargo#7296)
- Document that `package` can be used in `[patch]` (rust-lang/cargo#7263)
- Fix `error:`/`warning:` coloring inconsistency with rustc (rust-lang/cargo#7294)
- Tests: Import rustc_plugin from its new location (rust-lang/cargo#7287)
- Update README azure badge. (rust-lang/cargo#7293)
- Update home dependencies to v0.5 (rust-lang/cargo#7277)
- Fix typo (rust-lang/cargo#7279)
- Update libgit2 dependencies (rust-lang/cargo#7275)
- Fix old lockfile encoding wrt newlines (rust-lang/cargo#7262)
- Fix dSYM uplifting when symlink is broken (rust-lang/cargo#7268)
@ehuss ehuss added this to the 1.39.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants