Auto merge of #8350 - Mark-Simulacrum:rust-1.44.1, r=ehuss

1.44.1 stable backports

Tried to duplicate #8335, though had to manually adjust 3b9e8dc4c1dc6a6ec714acb6a97b4f7cffda1176 (cherry-pick of #8329) as it had a merge conflict.

Stable backports for:

* #8329 — Don't hash executable filenames on apple platforms. (fix macos backtraces)

Author: bors

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cargo"
-version = "0.45.0"
+version = "0.45.1"
 edition = "2018"
 authors = ["Yehuda Katz <[email protected]>",
            "Carl Lerche <[email protected]>",

src/cargo/core/compiler/context/compilation_files.rs

@@ -542,43 +542,8 @@ fn compute_metadata<'a, 'cfg>(
     cx: &Context<'a, 'cfg>,
     metas: &mut HashMap<Unit<'a>, Option<Metadata>>,
 ) -> Option<Metadata> {
-    if unit.mode.is_doc_test() {
-        // Doc tests do not have metadata.
-        return None;
-    }
-    // No metadata for dylibs because of a couple issues:
-    // - macOS encodes the dylib name in the executable,
-    // - Windows rustc multiple files of which we can't easily link all of them.
-    //
-    // No metadata for bin because of an issue:
-    // - wasm32 rustc/emcc encodes the `.wasm` name in the `.js` (rust-lang/cargo#4535).
-    // - msvc: The path to the PDB is embedded in the executable, and we don't
-    //   want the PDB path to include the hash in it.
-    //
-    // Two exceptions:
-    // 1) Upstream dependencies (we aren't exporting + need to resolve name conflict),
-    // 2) `__CARGO_DEFAULT_LIB_METADATA` env var.
-    //
-    // Note, however, that the compiler's build system at least wants
-    // path dependencies (eg libstd) to have hashes in filenames. To account for
-    // that we have an extra hack here which reads the
-    // `__CARGO_DEFAULT_LIB_METADATA` environment variable and creates a
-    // hash in the filename if that's present.
-    //
-    // This environment variable should not be relied on! It's
-    // just here for rustbuild. We need a more principled method
-    // doing this eventually.
     let bcx = &cx.bcx;
-    let __cargo_default_lib_metadata = env::var("__CARGO_DEFAULT_LIB_METADATA");
-    let short_name = bcx.target_data.short_name(&unit.kind);
-    if !(unit.mode.is_any_test() || unit.mode.is_check())
-        && (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.pkg.package_id().source_id().is_path()
-        && __cargo_default_lib_metadata.is_err()
-    {
+    if !should_use_metadata(bcx, unit) {
         return None;
     }
 
@@ -641,7 +606,7 @@ fn compute_metadata<'a, 'cfg>(
 
     // Seed the contents of `__CARGO_DEFAULT_LIB_METADATA` to the hasher if present.
     // This should be the release channel, to get a different hash for each channel.
-    if let Ok(ref channel) = __cargo_default_lib_metadata {
+    if let Ok(ref channel) = env::var("__CARGO_DEFAULT_LIB_METADATA") {
         channel.hash(&mut hasher);
     }
 
@@ -688,3 +653,53 @@ fn hash_rustc_version(bcx: &BuildContext<'_, '_>, hasher: &mut SipHasher) {
     // the future when cranelift sees more use, and people want to switch
     // between different backends without recompiling.
 }
+
+/// Returns whether or not this unit should use a metadata hash.
+fn should_use_metadata(bcx: &BuildContext<'_, '_>, unit: &Unit<'_>) -> bool {
+    if unit.mode.is_doc_test() {
+        // Doc tests do not have metadata.
+        return false;
+    }
+    if unit.mode.is_any_test() || unit.mode.is_check() {
+        // These always use metadata.
+        return true;
+    }
+    // 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?
+    // - 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.
+    //
+    // This is only done for local packages, as we don't expect to export
+    // dependencies.
+    //
+    // The __CARGO_DEFAULT_LIB_METADATA env var is used to override this to
+    // 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?
+    // 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.pkg.package_id().source_id().is_path()
+        && env::var("__CARGO_DEFAULT_LIB_METADATA").is_err()
+    {
+        return false;
+    }
+    true
+}

tests/testsuite/build.rs

@@ -4162,7 +4162,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());
 }

tests/testsuite/collisions.rs

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

tests/testsuite/cross_compile.rs

@@ -2,8 +2,8 @@
 //!
 //! See `cargo_test_support::cross_compile` for more detail.
 
+use cargo_test_support::rustc_host;
 use cargo_test_support::{basic_bin_manifest, basic_manifest, cross_compile, project};
-use cargo_test_support::{is_nightly, rustc_host};
 
 #[cargo_test]
 fn simple_cross() {
@@ -206,96 +206,6 @@ fn linker() {
         .run();
 }
 
-#[cargo_test]
-fn plugin_with_extra_dylib_dep() {
-    if cross_compile::disabled() {
-        return;
-    }
-    if !is_nightly() {
-        // plugins are unstable
-        return;
-    }
-
-    let foo = project()
-        .file(
-            "Cargo.toml",
-            r#"
-            [package]
-            name = "foo"
-            version = "0.0.1"
-            authors = []
-
-            [dependencies.bar]
-            path = "../bar"
-        "#,
-        )
-        .file(
-            "src/main.rs",
-            r#"
-            #![feature(plugin)]
-            #![plugin(bar)]
-
-            fn main() {}
-        "#,
-        )
-        .build();
-    let _bar = project()
-        .at("bar")
-        .file(
-            "Cargo.toml",
-            r#"
-            [package]
-            name = "bar"
-            version = "0.0.1"
-            authors = []
-
-            [lib]
-            name = "bar"
-            plugin = true
-
-            [dependencies.baz]
-            path = "../baz"
-        "#,
-        )
-        .file(
-            "src/lib.rs",
-            r#"
-            #![feature(plugin_registrar, rustc_private)]
-
-            extern crate baz;
-            extern crate rustc_driver;
-
-            use rustc_driver::plugin::Registry;
-
-            #[plugin_registrar]
-            pub fn foo(reg: &mut Registry) {
-                println!("{}", baz::baz());
-            }
-        "#,
-        )
-        .build();
-    let _baz = project()
-        .at("baz")
-        .file(
-            "Cargo.toml",
-            r#"
-            [package]
-            name = "baz"
-            version = "0.0.1"
-            authors = []
-
-            [lib]
-            name = "baz"
-            crate_type = ["dylib"]
-        "#,
-        )
-        .file("src/lib.rs", "pub fn baz() -> i32 { 1 }")
-        .build();
-
-    let target = cross_compile::alternate();
-    foo.cargo("build --target").arg(&target).run();
-}
-
 #[cargo_test]
 fn cross_tests() {
     if !cross_compile::can_run_on_host() {

tests/testsuite/freshness.rs

@@ -501,8 +501,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[..]");
@@ -511,7 +511,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[..]");