Skip to content

Commit

Permalink
Auto merge of #8329 - ehuss:apple-no-hash, r=alexcrichton
Browse files Browse the repository at this point in the history
Don't hash executable filenames on apple platforms.

Due to some recent changes to the backtrace crate, backtraces on apple platforms haven't been working (they are missing line/filename information). The reason is that previously libbacktrace would hunt through the directory for any matching file in the `.dSYM` directory. The new implementation expects a file matching the executable name exactly (which no longer includes the hash because Cargo renames it).

The solution here is to not include a hash in the executable filename. This matches the behavior on Windows which does it for a similar reason (paths are embedded in pdb files).

The downside is that switching between different settings (like different features) causes Cargo to rebuild the binary each time.  I don't think this is a particularly common use case, at least I've not heard any complaints about this behavior on Windows.

Fixes rust-lang/rust#72550
  • Loading branch information
bors committed Jun 5, 2020
2 parents 02205f5 + f975c2e commit e9d7af4
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 7 deletions.
9 changes: 8 additions & 1 deletion src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,12 @@ fn should_use_metadata(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool {
// - wasm32 executables: When using emscripten, the path to the .wasm file
// is embedded in the .js file, so we don't want the hash in there.
// TODO: Is this necessary for wasm32-unknown-unknown?
// - apple executables: The executable name is used in the dSYM directory
// (such as `target/debug/foo.dSYM/Contents/Resources/DWARF/foo-64db4e4bf99c12dd`).
// Unfortunately this causes problems with our current backtrace
// implementation which looks for a file matching the exe name exactly.
// See https://github.com/rust-lang/rust/issues/72550#issuecomment-638501691
// for more details.
//
// This is only done for local packages, as we don't expect to export
// dependencies.
Expand All @@ -622,7 +628,8 @@ fn should_use_metadata(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool {
if (unit.target.is_dylib()
|| unit.target.is_cdylib()
|| (unit.target.is_executable() && short_name.starts_with("wasm32-"))
|| (unit.target.is_executable() && short_name.contains("msvc")))
|| (unit.target.is_executable() && short_name.contains("msvc"))
|| (unit.target.is_executable() && short_name.contains("-apple-")))
&& unit.pkg.package_id().source_id().is_path()
&& env::var("__CARGO_DEFAULT_LIB_METADATA").is_err()
{
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4149,7 +4149,7 @@ fn uplift_dsym_of_bin_on_mac() {
assert!(p.target_debug_dir().join("foo.dSYM").is_dir());
assert!(p.target_debug_dir().join("b.dSYM").is_dir());
assert!(p.target_debug_dir().join("b.dSYM").is_symlink());
assert!(p.target_debug_dir().join("examples/c.dSYM").is_symlink());
assert!(p.target_debug_dir().join("examples/c.dSYM").is_dir());
assert!(!p.target_debug_dir().join("c.dSYM").exists());
assert!(!p.target_debug_dir().join("d.dSYM").exists());
}
Expand Down
4 changes: 2 additions & 2 deletions tests/testsuite/collisions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ This may become a hard error in the future; see <https://github.com/rust-lang/ca
}

#[cargo_test]
// --out-dir and examples are currently broken on MSVC.
// --out-dir and examples are currently broken on MSVC and apple.
// See https://github.com/rust-lang/cargo/issues/7493
#[cfg(not(target_env = "msvc"))]
#[cfg_attr(any(target_env = "msvc", target_vendor = "apple"), ignore)]
fn collision_export() {
// `--out-dir` combines some things which can cause conflicts.
let p = project()
Expand Down
6 changes: 3 additions & 3 deletions tests/testsuite/freshness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,8 @@ fn changing_bin_features_caches_targets() {
/* Targets should be cached from the first build */

let mut e = p.cargo("build");
// MSVC does not include hash in binary filename, so it gets recompiled.
if cfg!(target_env = "msvc") {
// MSVC/apple does not include hash in binary filename, so it gets recompiled.
if cfg!(any(target_env = "msvc", target_vendor = "apple")) {
e.with_stderr("[COMPILING] foo[..]\n[FINISHED] dev[..]");
} else {
e.with_stderr("[FINISHED] dev[..]");
Expand All @@ -501,7 +501,7 @@ fn changing_bin_features_caches_targets() {
p.rename_run("foo", "off2").with_stdout("feature off").run();

let mut e = p.cargo("build --features foo");
if cfg!(target_env = "msvc") {
if cfg!(any(target_env = "msvc", target_vendor = "apple")) {
e.with_stderr("[COMPILING] foo[..]\n[FINISHED] dev[..]");
} else {
e.with_stderr("[FINISHED] dev[..]");
Expand Down

0 comments on commit e9d7af4

Please sign in to comment.