Auto merge of #9653 - bjorn3:update_should_use_metadata, r=alexcrichton

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 file 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: https://github.com/rust-lang/backtrace-rs/blob/ed3689c2f2b9a31546d75fae389a32a572957dbd/src/symbolize/gimli/macho.rs#L51-L65
* Mention another reason to include the metadata hash for libstd.

Author: bors

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()
     {

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[..]");