Skip to content

Commit 4d0eff0

Browse files
authored
Fixed incorrect locking logic when artifact-dir == build-dir (#16385)
### What does this PR try to resolve? This PR fixes incorrect locking behavior in `layout.rs` that forgets to lock the `build-dir` when `build-dir == artifact-dir`. This is problematic is the default for both are `$WORKSPACE/target` and I overlooked this :( Issue reported in #16384 ### How to test and review this PR? See the tests. You can also run `cargo check` and check for the presence of `.cargo-lock` r? @weihanglo
2 parents e6b0a65 + ea9b240 commit 4d0eff0

File tree

2 files changed

+30
-3
lines changed

2 files changed

+30
-3
lines changed

src/cargo/core/compiler/layout.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,12 @@ impl Layout {
151151
// actual destination (sub)subdirectory.
152152
paths::create_dir_all(dest.as_path_unlocked())?;
153153

154-
let build_dir_lock = if root == build_root || is_on_nfs_mount(build_root.as_path_unlocked())
154+
// We always need to take the build-dir lock but if the build-dir == artifact-dir then we
155+
// only take the artifact-dir. (locking both as they are the same dir)
156+
// However we need to take into account that for some builds like `cargo check` we avoid
157+
// locking the artifact-dir. We still need to lock the build-dir to avoid file corruption.
158+
let build_dir_lock = if (must_take_artifact_dir_lock && root == build_root)
159+
|| is_on_nfs_mount(build_root.as_path_unlocked())
155160
{
156161
None
157162
} else {

tests/testsuite/check.rs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1710,13 +1710,35 @@ fn check_build_should_not_output_files_to_artifact_dir() {
17101710
}
17111711

17121712
#[cargo_test]
1713-
fn check_build_should_not_lock_artifact_dir() {
1713+
fn check_build_should_lock_target_dir_when_artifact_dir_is_same_as_build_dir() {
17141714
let p = project()
17151715
.file("src/main.rs", r#"fn main() { println!("Hello, World!") }"#)
17161716
.build();
17171717

17181718
p.cargo("check").enable_mac_dsym().run();
1719-
assert!(!p.root().join("target/debug/.cargo-lock").exists());
1719+
assert!(p.root().join("target/debug/.cargo-lock").exists());
1720+
}
1721+
1722+
#[cargo_test]
1723+
fn check_build_should_not_lock_artifact_dir_when_build_dir_is_not_same_dir() {
1724+
let p = project()
1725+
.file("src/main.rs", r#"fn main() { println!("Hello, World!") }"#)
1726+
.file(
1727+
".cargo/config.toml",
1728+
r#"
1729+
[build]
1730+
target-dir = "target-dir"
1731+
build-dir = "build-dir"
1732+
"#,
1733+
)
1734+
.build();
1735+
1736+
p.cargo("check").enable_mac_dsym().run();
1737+
1738+
// Verify we did NOT take the build-dir lock
1739+
assert!(!p.root().join("target-dir/debug/.cargo-lock").exists());
1740+
// Verify we did take the build-dir lock
1741+
assert!(p.root().join("build-dir/debug/.cargo-lock").exists());
17201742
}
17211743

17221744
// Regression test for #16305

0 commit comments

Comments
 (0)