Skip to content

Commit f2729c0

Browse files
weihangloweihanglo
authored
fix(layout): Remove hashes from bins in new layout (#16351)
### What does this PR try to resolve? There is a desire in #8332 to make our use of `-Cextra-file-name` in `deps/` more consistent across platforms. Having it present gets in the way of finding PDBs. With #15010, we no longer need `-Cextra-file-name` to avoid collisions. It might still be used by rustc for faster lookups though. So we can at least adjust all other platforms to be like Windows, making things consistent Addresses #8332 if #15010 is stabilized Part of #15010 ### How to test and review this PR?
2 parents 4d36721 + 98a0684 commit f2729c0

File tree

3 files changed

+61
-37
lines changed

3 files changed

+61
-37
lines changed

‎src/cargo/core/compiler/build_runner/compilation_files.rs‎

Lines changed: 58 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -887,40 +887,64 @@ fn use_extra_filename(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool {
887887
// Doc tests do not have metadata.
888888
return false;
889889
}
890-
if unit.mode.is_any_test() || unit.mode.is_check() {
891-
// These always use metadata.
892-
return true;
893-
}
894-
// No metadata in these cases:
895-
//
896-
// - dylibs:
897-
// - if any dylib names are encoded in executables, so they can't be renamed.
898-
// - TODO: Maybe use `-install-name` on macOS or `-soname` on other UNIX systems
899-
// to specify the dylib name to be used by the linker instead of the filename.
900-
// - Windows MSVC executables: The path to the PDB is embedded in the
901-
// executable, and we don't want the PDB path to include the hash in it.
902-
// - wasm32-unknown-emscripten executables: When using emscripten, the path to the
903-
// .wasm file is embedded in the .js file, so we don't want the hash in there.
904-
//
905-
// This is only done for local packages, as we don't expect to export
906-
// dependencies.
907-
//
908-
// The __CARGO_DEFAULT_LIB_METADATA env var is used to override this to
909-
// force metadata in the hash. This is only used for building libstd. For
910-
// example, if libstd is placed in a common location, we don't want a file
911-
// named /usr/lib/libstd.so which could conflict with other rustc
912-
// installs. In addition it prevents accidentally loading a libstd of a
913-
// different compiler at runtime.
914-
// See https://github.com/rust-lang/cargo/issues/3005
915-
let short_name = bcx.target_data.short_name(&unit.kind);
916-
if (unit.target.is_dylib()
917-
|| unit.target.is_cdylib()
918-
|| (unit.target.is_executable() && short_name == "wasm32-unknown-emscripten")
919-
|| (unit.target.is_executable() && short_name.contains("msvc")))
920-
&& unit.pkg.package_id().source_id().is_path()
921-
&& bcx.gctx.get_env("__CARGO_DEFAULT_LIB_METADATA").is_err()
922-
{
923-
return false;
890+
if bcx.gctx.cli_unstable().build_dir_new_layout {
891+
if unit.mode.is_any_test() || unit.mode.is_check() {
892+
// These always use metadata.
893+
return true;
894+
}
895+
// No metadata in these cases:
896+
//
897+
// - dylib, cdylib, executable: `pkg_dir` avoids collisions for us and rustc isn't looking these
898+
// up by `-Cextra-filename`
899+
//
900+
// The __CARGO_DEFAULT_LIB_METADATA env var is used to override this to
901+
// force metadata in the hash. This is only used for building libstd. For
902+
// example, if libstd is placed in a common location, we don't want a file
903+
// named /usr/lib/libstd.so which could conflict with other rustc
904+
// installs. In addition it prevents accidentally loading a libstd of a
905+
// different compiler at runtime.
906+
// See https://github.com/rust-lang/cargo/issues/3005
907+
if (unit.target.is_dylib() || unit.target.is_cdylib() || unit.target.is_executable())
908+
&& bcx.gctx.get_env("__CARGO_DEFAULT_LIB_METADATA").is_err()
909+
{
910+
return false;
911+
}
912+
} else {
913+
if unit.mode.is_any_test() || unit.mode.is_check() {
914+
// These always use metadata.
915+
return true;
916+
}
917+
// No metadata in these cases:
918+
//
919+
// - dylibs:
920+
// - if any dylib names are encoded in executables, so they can't be renamed.
921+
// - TODO: Maybe use `-install-name` on macOS or `-soname` on other UNIX systems
922+
// to specify the dylib name to be used by the linker instead of the filename.
923+
// - Windows MSVC executables: The path to the PDB is embedded in the
924+
// executable, and we don't want the PDB path to include the hash in it.
925+
// - wasm32-unknown-emscripten executables: When using emscripten, the path to the
926+
// .wasm file is embedded in the .js file, so we don't want the hash in there.
927+
//
928+
// This is only done for local packages, as we don't expect to export
929+
// dependencies.
930+
//
931+
// The __CARGO_DEFAULT_LIB_METADATA env var is used to override this to
932+
// force metadata in the hash. This is only used for building libstd. For
933+
// example, if libstd is placed in a common location, we don't want a file
934+
// named /usr/lib/libstd.so which could conflict with other rustc
935+
// installs. In addition it prevents accidentally loading a libstd of a
936+
// different compiler at runtime.
937+
// See https://github.com/rust-lang/cargo/issues/3005
938+
let short_name = bcx.target_data.short_name(&unit.kind);
939+
if (unit.target.is_dylib()
940+
|| unit.target.is_cdylib()
941+
|| (unit.target.is_executable() && short_name == "wasm32-unknown-emscripten")
942+
|| (unit.target.is_executable() && short_name.contains("msvc")))
943+
&& unit.pkg.package_id().source_id().is_path()
944+
&& bcx.gctx.get_env("__CARGO_DEFAULT_LIB_METADATA").is_err()
945+
{
946+
return false;
947+
}
924948
}
925949
true
926950
}

‎tests/testsuite/build_dir.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,10 @@ fn cargo_tmpdir_should_output_to_build_dir() {
336336
[ROOT]/foo/build-dir/CACHEDIR.TAG
337337
[ROOT]/foo/build-dir/debug/.cargo-lock
338338
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/deps/foo-[HASH].d
339-
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/deps/foo-[HASH].d
339+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/deps/foo.d
340340
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/deps/foo[..].d
341341
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/deps/foo-[HASH][EXE]
342-
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/deps/foo-[HASH][EXE]
342+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/deps/foo[EXE]
343343
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/deps/foo[..][EXE]
344344
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/invoked.timestamp
345345
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/dep-test-bin-foo

‎tests/testsuite/clean_new_layout.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ fn clean_multiple_packages_in_glob_char_path() {
126126
let foo_path = &p.build_dir().join("debug").join("build");
127127

128128
#[cfg(not(target_env = "msvc"))]
129-
let file_glob = "foo/*/deps/foo-*";
129+
let file_glob = "foo/*/deps/foo*";
130130

131131
#[cfg(target_env = "msvc")]
132132
let file_glob = "foo/*/deps/foo.pdb";

0 commit comments

Comments
 (0)