Skip to content

Commit 17b41ae

Browse files
epageepage
authored
Always take a shared lock on .cargo-lock (#16886)
### What does this PR try to resolve? This PR changes the behavior to always take a shared lock on `.cargo-lock` to ensure backwards compatibility with older versions of cargo that do not lock `.cargo-build-lock`. To allow `cargo check` and `cargo build` to continue running concurrently `.cargo-artifact-lock` was added with the original behavior of `.cargo-lock`. We may be able to revert this change after the new build-dir layout is stablized. (#16807) closes: #16853 ### How to test and review this PR? See the test changes
2 parents b0942d3 + 9a2150b commit 17b41ae

File tree

6 files changed

+58
-7
lines changed

6 files changed

+58
-7
lines changed

‎src/cargo/core/compiler/layout.rs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ use std::path::{Path, PathBuf};
220220
pub struct Layout {
221221
artifact_dir: Option<ArtifactDirLayout>,
222222
build_dir: BuildDirLayout,
223+
_lock: Option<FileLock>,
223224
}
224225

225226
impl Layout {
@@ -281,6 +282,15 @@ impl Layout {
281282
let deps = build_dest.join("deps");
282283
let artifact = deps.join("artifact");
283284

285+
// We take a shared lock on `.cargo-lock` to make sure we don't run currently with
286+
// older versions of Cargo (including tools that use Cargo as a library) that don't support
287+
// `.cargo-build-lock`.
288+
let lock = if is_on_nfs_mount(root.as_path_unlocked()) {
289+
None
290+
} else {
291+
Some(dest.open_ro_shared_create(".cargo-lock", ws.gctx(), "artifact directory")?)
292+
};
293+
284294
let artifact_dir = if must_take_artifact_dir_lock {
285295
// For now we don't do any more finer-grained locking on the artifact
286296
// directory, so just lock the entire thing for the duration of this
@@ -289,7 +299,7 @@ impl Layout {
289299
None
290300
} else {
291301
Some(dest.open_rw_exclusive_create(
292-
".cargo-lock",
302+
".cargo-artifact-lock",
293303
ws.gctx(),
294304
"artifact directory",
295305
)?)
@@ -320,6 +330,7 @@ impl Layout {
320330
_lock: build_dir_lock,
321331
is_new_layout,
322332
},
333+
_lock: lock,
323334
})
324335
}
325336

‎tests/testsuite/build_dir.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ fn binary_with_debug() {
5555
.assert_build_dir_layout(str![[r#"
5656
[ROOT]/foo/target-dir/CACHEDIR.TAG
5757
[ROOT]/foo/target-dir/debug/.cargo-lock
58+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
5859
[ROOT]/foo/target-dir/debug/foo[EXE]
5960
[ROOT]/foo/target-dir/debug/foo.d
6061
@@ -110,6 +111,7 @@ fn binary_with_release() {
110111
.assert_build_dir_layout(str![[r#"
111112
[ROOT]/foo/target-dir/CACHEDIR.TAG
112113
[ROOT]/foo/target-dir/release/.cargo-lock
114+
[ROOT]/foo/target-dir/release/.cargo-artifact-lock
113115
[ROOT]/foo/target-dir/release/foo[EXE]
114116
[ROOT]/foo/target-dir/release/foo.d
115117
@@ -205,6 +207,7 @@ fn should_default_to_target() {
205207
[ROOT]/foo/target/.rustc_info.json
206208
[ROOT]/foo/target/CACHEDIR.TAG
207209
[ROOT]/foo/target/debug/.cargo-lock
210+
[ROOT]/foo/target/debug/.cargo-artifact-lock
208211
[ROOT]/foo/target/debug/.cargo-build-lock
209212
[ROOT]/foo/target/debug/build/foo/[HASH]/fingerprint/bin-foo
210213
[ROOT]/foo/target/debug/build/foo/[HASH]/fingerprint/bin-foo.json
@@ -373,6 +376,7 @@ fn cargo_tmpdir_should_output_to_build_dir() {
373376
.assert_build_dir_layout(str![[r#"
374377
[ROOT]/foo/target-dir/CACHEDIR.TAG
375378
[ROOT]/foo/target-dir/debug/.cargo-lock
379+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
376380
[ROOT]/foo/target-dir/debug/foo[EXE]
377381
378382
"#]]);
@@ -419,6 +423,7 @@ fn examples_should_output_to_build_dir_and_uplift_to_target_dir() {
419423
.assert_build_dir_layout(str![[r#"
420424
[ROOT]/foo/target-dir/CACHEDIR.TAG
421425
[ROOT]/foo/target-dir/debug/.cargo-lock
426+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
422427
[ROOT]/foo/target-dir/debug/examples/foo[EXE]
423428
[ROOT]/foo/target-dir/debug/examples/foo.d
424429
@@ -471,6 +476,7 @@ fn benches_should_output_to_build_dir() {
471476
.assert_build_dir_layout(str![[r#"
472477
[ROOT]/foo/target-dir/CACHEDIR.TAG
473478
[ROOT]/foo/target-dir/debug/.cargo-lock
479+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
474480
[ROOT]/foo/target-dir/debug/foo[EXE]
475481
476482
"#]]);
@@ -528,6 +534,7 @@ fn cargo_package_should_build_in_build_dir_and_output_to_target_dir() {
528534
p.root().join("build-dir").assert_build_dir_layout(str![[r#"
529535
[ROOT]/foo/build-dir/.rustc_info.json
530536
[ROOT]/foo/build-dir/debug/.cargo-lock
537+
[ROOT]/foo/build-dir/debug/.cargo-artifact-lock
531538
[ROOT]/foo/build-dir/debug/.cargo-build-lock
532539
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/bin-foo
533540
[ROOT]/foo/build-dir/debug/build/foo/[HASH]/fingerprint/bin-foo.json
@@ -625,6 +632,7 @@ fn cargo_clean_should_clean_the_target_dir_and_build_dir() {
625632
.assert_build_dir_layout(str![[r#"
626633
[ROOT]/foo/target-dir/CACHEDIR.TAG
627634
[ROOT]/foo/target-dir/debug/.cargo-lock
635+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
628636
[ROOT]/foo/target-dir/debug/foo[EXE]
629637
[ROOT]/foo/target-dir/debug/foo.d
630638
@@ -858,6 +866,7 @@ fn template_workspace_root() {
858866
.assert_build_dir_layout(str![[r#"
859867
[ROOT]/foo/target-dir/CACHEDIR.TAG
860868
[ROOT]/foo/target-dir/debug/.cargo-lock
869+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
861870
[ROOT]/foo/target-dir/debug/foo[EXE]
862871
[ROOT]/foo/target-dir/debug/foo.d
863872
@@ -906,6 +915,7 @@ fn template_cargo_cache_home() {
906915
.assert_build_dir_layout(str![[r#"
907916
[ROOT]/foo/target-dir/CACHEDIR.TAG
908917
[ROOT]/foo/target-dir/debug/.cargo-lock
918+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
909919
[ROOT]/foo/target-dir/debug/foo[EXE]
910920
[ROOT]/foo/target-dir/debug/foo.d
911921
@@ -968,6 +978,7 @@ fn template_workspace_path_hash() {
968978
.assert_build_dir_layout(str![[r#"
969979
[ROOT]/foo/target-dir/CACHEDIR.TAG
970980
[ROOT]/foo/target-dir/debug/.cargo-lock
981+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
971982
[ROOT]/foo/target-dir/debug/foo[EXE]
972983
[ROOT]/foo/target-dir/debug/foo.d
973984
@@ -1033,6 +1044,7 @@ fn template_workspace_path_hash_should_handle_symlink() {
10331044

10341045
p.root().join("target").assert_build_dir_layout(str![[r#"
10351046
[ROOT]/foo/target/CACHEDIR.TAG
1047+
[ROOT]/foo/target/debug/.cargo-lock
10361048
10371049
"#]]);
10381050

@@ -1072,6 +1084,7 @@ fn template_workspace_path_hash_should_handle_symlink() {
10721084

10731085
p.root().join("target").assert_build_dir_layout(str![[r#"
10741086
[ROOT]/foo/target/CACHEDIR.TAG
1087+
[ROOT]/foo/target/debug/.cargo-lock
10751088
10761089
"#]]);
10771090

@@ -1213,6 +1226,7 @@ CARGO_BIN_FILE_BAR_bar=[ROOT]/foo/build-dir/debug/build/bar/[HASH]/artifact/bin/
12131226
.assert_build_dir_layout(str![[r#"
12141227
[ROOT]/foo/target-dir/CACHEDIR.TAG
12151228
[ROOT]/foo/target-dir/debug/.cargo-lock
1229+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
12161230
[ROOT]/foo/target-dir/debug/foo[EXE]
12171231
[ROOT]/foo/target-dir/debug/foo.d
12181232

‎tests/testsuite/build_dir_legacy.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ fn binary_with_debug() {
5151
.assert_build_dir_layout(str![[r#"
5252
[ROOT]/foo/target-dir/CACHEDIR.TAG
5353
[ROOT]/foo/target-dir/debug/.cargo-lock
54+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
5455
[ROOT]/foo/target-dir/debug/foo[EXE]
5556
[ROOT]/foo/target-dir/debug/foo.d
5657
@@ -102,6 +103,7 @@ fn binary_with_release() {
102103
.assert_build_dir_layout(str![[r#"
103104
[ROOT]/foo/target-dir/CACHEDIR.TAG
104105
[ROOT]/foo/target-dir/release/.cargo-lock
106+
[ROOT]/foo/target-dir/release/.cargo-artifact-lock
105107
[ROOT]/foo/target-dir/release/foo[EXE]
106108
[ROOT]/foo/target-dir/release/foo.d
107109
@@ -191,6 +193,7 @@ fn should_default_to_target() {
191193
[ROOT]/foo/target/.rustc_info.json
192194
[ROOT]/foo/target/CACHEDIR.TAG
193195
[ROOT]/foo/target/debug/.cargo-lock
196+
[ROOT]/foo/target/debug/.cargo-artifact-lock
194197
[ROOT]/foo/target/debug/.cargo-build-lock
195198
[ROOT]/foo/target/debug/.fingerprint/foo-[HASH]/bin-foo
196199
[ROOT]/foo/target/debug/.fingerprint/foo-[HASH]/bin-foo.json
@@ -345,6 +348,7 @@ fn cargo_tmpdir_should_output_to_build_dir() {
345348
.assert_build_dir_layout(str![[r#"
346349
[ROOT]/foo/target-dir/CACHEDIR.TAG
347350
[ROOT]/foo/target-dir/debug/.cargo-lock
351+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
348352
[ROOT]/foo/target-dir/debug/foo[EXE]
349353
350354
"#]]);
@@ -385,6 +389,7 @@ fn examples_should_output_to_build_dir_and_uplift_to_target_dir() {
385389
.assert_build_dir_layout(str![[r#"
386390
[ROOT]/foo/target-dir/CACHEDIR.TAG
387391
[ROOT]/foo/target-dir/debug/.cargo-lock
392+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
388393
[ROOT]/foo/target-dir/debug/examples/foo[EXE]
389394
[ROOT]/foo/target-dir/debug/examples/foo.d
390395
@@ -432,6 +437,7 @@ fn benches_should_output_to_build_dir() {
432437
.assert_build_dir_layout(str![[r#"
433438
[ROOT]/foo/target-dir/CACHEDIR.TAG
434439
[ROOT]/foo/target-dir/debug/.cargo-lock
440+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
435441
[ROOT]/foo/target-dir/debug/foo[EXE]
436442
437443
"#]]);
@@ -483,6 +489,7 @@ fn cargo_package_should_build_in_build_dir_and_output_to_target_dir() {
483489
p.root().join("build-dir").assert_build_dir_layout(str![[r#"
484490
[ROOT]/foo/build-dir/.rustc_info.json
485491
[ROOT]/foo/build-dir/debug/.cargo-lock
492+
[ROOT]/foo/build-dir/debug/.cargo-artifact-lock
486493
[ROOT]/foo/build-dir/debug/.cargo-build-lock
487494
[ROOT]/foo/build-dir/debug/.fingerprint/foo-[HASH]/bin-foo
488495
[ROOT]/foo/build-dir/debug/.fingerprint/foo-[HASH]/bin-foo.json
@@ -574,6 +581,7 @@ fn cargo_clean_should_clean_the_target_dir_and_build_dir() {
574581
.assert_build_dir_layout(str![[r#"
575582
[ROOT]/foo/target-dir/CACHEDIR.TAG
576583
[ROOT]/foo/target-dir/debug/.cargo-lock
584+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
577585
[ROOT]/foo/target-dir/debug/foo[EXE]
578586
[ROOT]/foo/target-dir/debug/foo.d
579587
@@ -785,6 +793,7 @@ fn template_workspace_root() {
785793
.assert_build_dir_layout(str![[r#"
786794
[ROOT]/foo/target-dir/CACHEDIR.TAG
787795
[ROOT]/foo/target-dir/debug/.cargo-lock
796+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
788797
[ROOT]/foo/target-dir/debug/foo[EXE]
789798
[ROOT]/foo/target-dir/debug/foo.d
790799
@@ -829,6 +838,7 @@ fn template_cargo_cache_home() {
829838
.assert_build_dir_layout(str![[r#"
830839
[ROOT]/foo/target-dir/CACHEDIR.TAG
831840
[ROOT]/foo/target-dir/debug/.cargo-lock
841+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
832842
[ROOT]/foo/target-dir/debug/foo[EXE]
833843
[ROOT]/foo/target-dir/debug/foo.d
834844
@@ -887,6 +897,7 @@ fn template_workspace_path_hash() {
887897
.assert_build_dir_layout(str![[r#"
888898
[ROOT]/foo/target-dir/CACHEDIR.TAG
889899
[ROOT]/foo/target-dir/debug/.cargo-lock
900+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
890901
[ROOT]/foo/target-dir/debug/foo[EXE]
891902
[ROOT]/foo/target-dir/debug/foo.d
892903
@@ -948,6 +959,7 @@ fn template_workspace_path_hash_should_handle_symlink() {
948959

949960
p.root().join("target").assert_build_dir_layout(str![[r#"
950961
[ROOT]/foo/target/CACHEDIR.TAG
962+
[ROOT]/foo/target/debug/.cargo-lock
951963
952964
"#]]);
953965

@@ -982,6 +994,7 @@ fn template_workspace_path_hash_should_handle_symlink() {
982994

983995
p.root().join("target").assert_build_dir_layout(str![[r#"
984996
[ROOT]/foo/target/CACHEDIR.TAG
997+
[ROOT]/foo/target/debug/.cargo-lock
985998
986999
"#]]);
9871000

@@ -1122,6 +1135,7 @@ CARGO_BIN_FILE_BAR_bar=[ROOT]/foo/build-dir/debug/deps/artifact/bar-[HASH]/bin/b
11221135
.assert_build_dir_layout(str![[r#"
11231136
[ROOT]/foo/target-dir/CACHEDIR.TAG
11241137
[ROOT]/foo/target-dir/debug/.cargo-lock
1138+
[ROOT]/foo/target-dir/debug/.cargo-artifact-lock
11251139
[ROOT]/foo/target-dir/debug/foo[EXE]
11261140
[ROOT]/foo/target-dir/debug/foo.d
11271141

‎tests/testsuite/check.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,6 +1715,7 @@ fn check_build_should_not_output_files_to_artifact_dir() {
17151715
.join("target-dir")
17161716
.assert_build_dir_layout(str![[r#"
17171717
[ROOT]/foo/target-dir/CACHEDIR.TAG
1718+
[ROOT]/foo/target-dir/debug/.cargo-lock
17181719
17191720
"#]]);
17201721
}
@@ -1745,8 +1746,12 @@ fn check_build_should_not_lock_artifact_dir_when_build_dir_is_not_same_dir() {
17451746

17461747
p.cargo("check").enable_mac_dsym().run();
17471748

1748-
// Verify we did NOT take the build-dir lock
1749-
assert!(!p.root().join("target-dir/debug/.cargo-lock").exists());
1749+
// Verify we did NOT take the artifact-dir lock
1750+
assert!(
1751+
!p.root()
1752+
.join("target-dir/debug/.cargo-artifact-lock")
1753+
.exists()
1754+
);
17501755
// Verify we did take the build-dir lock
17511756
assert!(p.root().join("build-dir/debug/.cargo-build-lock").exists());
17521757
}
@@ -1838,6 +1843,7 @@ fn check_build_should_not_uplift_proc_macro_dylib_deps() {
18381843
.join("target-dir")
18391844
.assert_build_dir_layout(str![[r#"
18401845
[ROOT]/foo/target-dir/CACHEDIR.TAG
1846+
[ROOT]/foo/target-dir/debug/.cargo-lock
18411847
18421848
"#]]);
18431849
}

‎tests/testsuite/clean.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -736,8 +736,11 @@ fn assert_all_clean(build_dir: &Path) {
736736
}) {
737737
let entry = entry.unwrap();
738738
let path = entry.path();
739-
if let ".rustc_info.json" | ".cargo-lock" | ".cargo-build-lock" | "CACHEDIR.TAG" =
740-
path.file_name().unwrap().to_str().unwrap()
739+
if let ".rustc_info.json"
740+
| ".cargo-lock"
741+
| ".cargo-build-lock"
742+
| ".cargo-artifact-lock"
743+
| "CACHEDIR.TAG" = path.file_name().unwrap().to_str().unwrap()
741744
{
742745
continue;
743746
}

‎tests/testsuite/clean_new_layout.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -702,8 +702,11 @@ fn assert_all_clean(build_dir: &Path) {
702702
}) {
703703
let entry = entry.unwrap();
704704
let path = entry.path();
705-
if let ".rustc_info.json" | ".cargo-lock" | ".cargo-build-lock" | "CACHEDIR.TAG" =
706-
path.file_name().unwrap().to_str().unwrap()
705+
if let ".rustc_info.json"
706+
| ".cargo-lock"
707+
| ".cargo-build-lock"
708+
| ".cargo-artifact-lock"
709+
| "CACHEDIR.TAG" = path.file_name().unwrap().to_str().unwrap()
707710
{
708711
continue;
709712
}

0 commit comments

Comments
 (0)