From 283bafc3c4dc5d54673975bdc8cd5c643745ac3c Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 5 Jul 2021 16:44:19 +0200 Subject: [PATCH 1/2] Update should_use_metadata function * Correct the reason for not renaming dylibs * Add todo for -install-name/-soname usage * Limit wasm32 executable metadata omission to emscripten. Wasm files don't contain any filename themself. * Don't omit metadata for executables on macOS. backtrace-rs is now able to load debuginfo for renamed .dSYM files. * Mention another reason to include the metadata hash for libstd. --- .../compiler/context/compilation_files.rs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index ad1ae2f8fe4..3dd42f25c32 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -626,19 +626,13 @@ fn should_use_metadata(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool { // No metadata in these cases: // // - dylibs: - // - macOS encodes the dylib name in the executable, so it can't be renamed. - // - TODO: Are there other good reasons? If not, maybe this should be macos specific? + // - if any dylib names are encoded in executables, so they can't be renamed. + // - TODO: Maybe use `-install-name` on macOS or `-soname` on other UNIX systems + // to specify the dylib name to be used by the linker instead of the filename. // - Windows MSVC executables: The path to the PDB is embedded in the // executable, and we don't want the PDB path to include the hash in it. - // - 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. + // - wasm32-unknown-emscripten 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. // // This is only done for local packages, as we don't expect to export // dependencies. @@ -647,14 +641,14 @@ fn should_use_metadata(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool { // force metadata in the hash. This is only used for building libstd. For // example, if libstd is placed in a common location, we don't want a file // named /usr/lib/libstd.so which could conflict with other rustc - // installs. TODO: Is this still a realistic concern? + // installs. In addition it prevents accidentally loading a libstd of a + // different compiler at runtime. // See https://github.com/rust-lang/cargo/issues/3005 let short_name = bcx.target_data.short_name(&unit.kind); 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("-apple-"))) + || (unit.target.is_executable() && short_name == "wasm32-unknown-emscripten") + || (unit.target.is_executable() && short_name.contains("msvc"))) && unit.pkg.package_id().source_id().is_path() && env::var("__CARGO_DEFAULT_LIB_METADATA").is_err() { From 8c24820a3d7f7ee83442a8c8d8a48720c7d21b15 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 5 Jul 2021 18:09:05 +0200 Subject: [PATCH 2/2] Update test --- tests/testsuite/freshness.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/testsuite/freshness.rs b/tests/testsuite/freshness.rs index bdbd44aa95e..c764bcacd9a 100644 --- a/tests/testsuite/freshness.rs +++ b/tests/testsuite/freshness.rs @@ -493,8 +493,8 @@ fn changing_bin_features_caches_targets() { /* Targets should be cached from the first build */ let mut e = p.cargo("build"); - // MSVC/apple does not include hash in binary filename, so it gets recompiled. - if cfg!(any(target_env = "msvc", target_vendor = "apple")) { + // MSVC does not include hash in binary filename, so it gets recompiled. + if cfg!(target_env = "msvc") { e.with_stderr("[COMPILING] foo[..]\n[FINISHED] dev[..]"); } else { e.with_stderr("[FINISHED] dev[..]"); @@ -503,7 +503,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!(any(target_env = "msvc", target_vendor = "apple")) { + if cfg!(target_env = "msvc") { e.with_stderr("[COMPILING] foo[..]\n[FINISHED] dev[..]"); } else { e.with_stderr("[FINISHED] dev[..]");