Skip to content

Commit 101549d

Browse files
authored
Never include use extra-filename in build scripts (#16855)
### What does this PR try to resolve? See: #16854 This is that in #16812 we stopped adding the metadata hash to build script names and just call them `build_script_build[EXE]` since they are already contained within a directory with the hash. However, I didn't take into consideration `__CARGO_DEFAULT_LIB_METADATA`, so if it is passed with a build script in the build graph it will get the wrong path (`build_script_build-[HASH][EXE]`) when it tries to execute and fail. Looking at [the comments](https://github.com/rust-lang/cargo/blob/master/src/cargo/core/compiler/build_runner/compilation_files.rs#L922) about `__CARGO_DEFAULT_LIB_METADATA` I get the impression there is no usecase for using it with a build script. So I modified the logic to never include the meta in the build script file names even when `__CARGO_DEFAULT_LIB_METADATA`. Let me know if this is a bad assumption. ### How to test and review this PR? See the included test. Also double checked on https://github.com/RalfJung/rustc-build-sysroot/ and this change fixes the issue for me cc: @RalfJung r? @epage
2 parents 35f8202 + 3cc34d6 commit 101549d

2 files changed

Lines changed: 62 additions & 5 deletions

File tree

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -914,9 +914,14 @@ fn use_extra_filename(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool {
914914
// These always use metadata.
915915
return true;
916916
}
917+
918+
if unit.target.is_custom_build() {
919+
// Build scripts never use metadata
920+
return false;
921+
}
917922
// No metadata in these cases:
918923
//
919-
// - dylib, cdylib, executable, build-scripts: `pkg_dir` avoids collisions for us and rustc isn't
924+
// - dylib, cdylib, executable: `pkg_dir` avoids collisions for us and rustc isn't
920925
// looking these up by `-Cextra-filename`
921926
//
922927
// The __CARGO_DEFAULT_LIB_METADATA env var is used to override this to
@@ -926,10 +931,7 @@ fn use_extra_filename(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool {
926931
// installs. In addition it prevents accidentally loading a libstd of a
927932
// different compiler at runtime.
928933
// See https://github.com/rust-lang/cargo/issues/3005
929-
if (unit.target.is_dylib()
930-
|| unit.target.is_cdylib()
931-
|| unit.target.is_executable()
932-
|| unit.target.is_custom_build())
934+
if (unit.target.is_dylib() || unit.target.is_cdylib() || unit.target.is_executable())
933935
&& bcx.gctx.get_env("__CARGO_DEFAULT_LIB_METADATA").is_err()
934936
{
935937
return false;

tests/testsuite/build_dir.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,61 @@ CARGO_BIN_FILE_BAR_bar=[ROOT]/foo/build-dir/debug/build/bar/[HASH]/artifact/bin/
12191219
"#]]);
12201220
}
12211221

1222+
/// __CARGO_DEFAULT_LIB_METADATA is internal but used by rustc bootstrap and Miri
1223+
/// Regression test for https://github.com/rust-lang/cargo/issues/16854
1224+
#[cargo_test]
1225+
fn should_work_with_cargo_default_lib_metadata() {
1226+
let p = project()
1227+
.file("src/main.rs", r#"fn main() { println!("Hello, World!") }"#)
1228+
.file("build.rs", r#"fn main() { }"#)
1229+
.file(
1230+
".cargo/config.toml",
1231+
r#"
1232+
[build]
1233+
target-dir = "target-dir"
1234+
build-dir = "build-dir"
1235+
"#,
1236+
)
1237+
.build();
1238+
1239+
p.cargo("-Zbuild-dir-new-layout build")
1240+
.masquerade_as_nightly_cargo(&["new build-dir layout"])
1241+
.env("__CARGO_DEFAULT_LIB_METADATA", "true")
1242+
.enable_mac_dsym()
1243+
.with_stderr_data(str![[r#"
1244+
[COMPILING] foo v0.0.1 ([ROOT]/foo)
1245+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
1246+
1247+
"#]])
1248+
.run();
1249+
1250+
// NOTE: build_script_build[EXE] does not contain a hash in the filename
1251+
p.root().join("build-dir").assert_build_dir_layout(str![[r#"
1252+
[ROOT]/foo/build-dir/.rustc_info.json
1253+
[ROOT]/foo/build-dir/CACHEDIR.TAG
1254+
[ROOT]/foo/build-dir/debug/.cargo-build-lock
1255+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/bin-foo
1256+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/bin-foo.json
1257+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/dep-bin-foo
1258+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/invoked.timestamp
1259+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/out/foo[..][EXE]
1260+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/out/foo[..].d
1261+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/run-build-script-build-script-build
1262+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/run-build-script-build-script-build.json
1263+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/run/invoked.timestamp
1264+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/run/root-output
1265+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/run/stderr
1266+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/run/stdout
1267+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/build-script-build-script-build
1268+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/build-script-build-script-build.json
1269+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/dep-build-script-build-script-build
1270+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/invoked.timestamp
1271+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/out/build_script_build[EXE]
1272+
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/out/build_script_build.d
1273+
1274+
"#]]);
1275+
}
1276+
12221277
fn parse_workspace_manifest_path_hash(hash_dir: &PathBuf) -> PathBuf {
12231278
// Since the hash will change between test runs simply find the first directories and assume
12241279
// that is the hash dir. The format is a 2 char directory followed by the remaining hash in the

0 commit comments

Comments
 (0)