Revert "feat: Do not lock the artifact-dir for check builds" (#16299)

### What does this PR try to resolve?

Fixes #16297

### How to test and review this PR?

This reverts commit 9f5393a.

Author: weihanglo

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

@@ -6,7 +6,7 @@ use std::sync::{Arc, Mutex};
 
 use crate::core::PackageId;
 use crate::core::compiler::compilation::{self, UnitOutput};
-use crate::core::compiler::{self, Unit, UserIntent, artifact};
+use crate::core::compiler::{self, Unit, artifact};
 use crate::util::cache_lock::CacheLockMode;
 use crate::util::errors::CargoResult;
 use annotate_snippets::{Level, Message};
@@ -352,32 +352,11 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
     #[tracing::instrument(skip_all)]
     pub fn prepare_units(&mut self) -> CargoResult<()> {
         let dest = self.bcx.profiles.get_dir_name();
-        // We try to only lock the artifact-dir if we need to.
-        // For example, `cargo check` does not write any files to the artifact-dir so we don't need
-        // to lock it.
-        let must_take_artifact_dir_lock = match self.bcx.build_config.intent {
-            UserIntent::Check { .. } => {
-                // Generally cargo check does not need to take the artifact-dir lock but there is
-                // one exception: If check has `--timings` we still need to lock artifact-dir since
-                // we will output the report files.
-                !self.bcx.build_config.timing_outputs.is_empty()
-            }
-            UserIntent::Build
-            | UserIntent::Test
-            | UserIntent::Doc { .. }
-            | UserIntent::Doctest
-            | UserIntent::Bench => true,
-        };
-        let host_layout = Layout::new(self.bcx.ws, None, &dest, must_take_artifact_dir_lock)?;
+        let host_layout = Layout::new(self.bcx.ws, None, &dest)?;
         let mut targets = HashMap::new();
         for kind in self.bcx.all_kinds.iter() {
             if let CompileKind::Target(target) = *kind {
-                let layout = Layout::new(
-                    self.bcx.ws,
-                    Some(target),
-                    &dest,
-                    must_take_artifact_dir_lock,
-                )?;
+                let layout = Layout::new(self.bcx.ws, Some(target), &dest)?;
                 targets.insert(target, layout);
             }
         }

src/cargo/core/compiler/layout.rs

@@ -127,7 +127,6 @@ impl Layout {
         ws: &Workspace<'_>,
         target: Option<CompileTarget>,
         dest: &str,
-        must_take_artifact_dir_lock: bool,
     ) -> CargoResult<Layout> {
         let is_new_layout = ws.gctx().cli_unstable().build_dir_new_layout;
         let mut root = ws.target_dir();
@@ -151,6 +150,15 @@ impl Layout {
         // actual destination (sub)subdirectory.
         paths::create_dir_all(dest.as_path_unlocked())?;
 
+        // For now we don't do any more finer-grained locking on the artifact
+        // directory, so just lock the entire thing for the duration of this
+        // compile.
+        let artifact_dir_lock = if is_on_nfs_mount(root.as_path_unlocked()) {
+            None
+        } else {
+            Some(dest.open_rw_exclusive_create(".cargo-lock", ws.gctx(), "artifact directory")?)
+        };
+
         let build_dir_lock = if root == build_root || is_on_nfs_mount(build_root.as_path_unlocked())
         {
             None
@@ -161,38 +169,21 @@ impl Layout {
                 "build directory",
             )?)
         };
+        let root = root.into_path_unlocked();
         let build_root = build_root.into_path_unlocked();
+        let dest = dest.into_path_unlocked();
         let build_dest = build_dest.as_path_unlocked();
         let deps = build_dest.join("deps");
         let artifact = deps.join("artifact");
 
-        let artifact_dir = if must_take_artifact_dir_lock {
-            // For now we don't do any more finer-grained locking on the artifact
-            // directory, so just lock the entire thing for the duration of this
-            // compile.
-            let artifact_dir_lock = if is_on_nfs_mount(root.as_path_unlocked()) {
-                None
-            } else {
-                Some(dest.open_rw_exclusive_create(
-                    ".cargo-lock",
-                    ws.gctx(),
-                    "artifact directory",
-                )?)
-            };
-            let root = root.into_path_unlocked();
-            let dest = dest.into_path_unlocked();
-            Some(ArtifactDirLayout {
+        Ok(Layout {
+            artifact_dir: Some(ArtifactDirLayout {
                 dest: dest.clone(),
                 examples: dest.join("examples"),
                 doc: root.join("doc"),
                 timings: root.join("cargo-timings"),
                 _lock: artifact_dir_lock,
-            })
-        } else {
-            None
-        };
-        Ok(Layout {
-            artifact_dir,
+            }),
             build_dir: BuildDirLayout {
                 root: build_root.clone(),
                 deps,

src/cargo/ops/cargo_clean.rs

@@ -117,17 +117,15 @@ fn clean_specs(
     let target_data = RustcTargetData::new(ws, &requested_kinds)?;
     let (pkg_set, resolve) = ops::resolve_ws(ws, dry_run)?;
     let prof_dir_name = profiles.get_dir_name();
-    let host_layout = Layout::new(ws, None, &prof_dir_name, true)?;
+    let host_layout = Layout::new(ws, None, &prof_dir_name)?;
     // Convert requested kinds to a Vec of layouts.
     let target_layouts: Vec<(CompileKind, Layout)> = requested_kinds
         .into_iter()
         .filter_map(|kind| match kind {
-            CompileKind::Target(target) => {
-                match Layout::new(ws, Some(target), &prof_dir_name, true) {
-                    Ok(layout) => Some(Ok((kind, layout))),
-                    Err(e) => Some(Err(e)),
-                }
-            }
+            CompileKind::Target(target) => match Layout::new(ws, Some(target), &prof_dir_name) {
+                Ok(layout) => Some(Ok((kind, layout))),
+                Err(e) => Some(Err(e)),
+            },
             CompileKind::Host => None,
         })
         .collect::<CargoResult<_>>()?;

tests/testsuite/build_dir.rs

@@ -1007,6 +1007,7 @@ fn template_workspace_path_hash_should_handle_symlink() {
 
     p.root().join("target").assert_build_dir_layout(str![[r#"
 [ROOT]/foo/target/CACHEDIR.TAG
+[ROOT]/foo/target/debug/.cargo-lock
 
 "#]]);
 
@@ -1045,6 +1046,7 @@ fn template_workspace_path_hash_should_handle_symlink() {
 
     p.root().join("target").assert_build_dir_layout(str![[r#"
 [ROOT]/foo/target/CACHEDIR.TAG
+[ROOT]/foo/target/debug/.cargo-lock
 
 "#]]);
 

tests/testsuite/build_dir_legacy.rs

@@ -945,6 +945,7 @@ fn template_workspace_path_hash_should_handle_symlink() {
 
     p.root().join("target").assert_build_dir_layout(str![[r#"
 [ROOT]/foo/target/CACHEDIR.TAG
+[ROOT]/foo/target/debug/.cargo-lock
 
 "#]]);
 
@@ -979,6 +980,7 @@ fn template_workspace_path_hash_should_handle_symlink() {
 
     p.root().join("target").assert_build_dir_layout(str![[r#"
 [ROOT]/foo/target/CACHEDIR.TAG
+[ROOT]/foo/target/debug/.cargo-lock
 
 "#]]);
 

tests/testsuite/check.rs

@@ -1705,16 +1705,17 @@ fn check_build_should_not_output_files_to_artifact_dir() {
         .join("target-dir")
         .assert_build_dir_layout(str![[r#"
 [ROOT]/foo/target-dir/CACHEDIR.TAG
+[ROOT]/foo/target-dir/debug/.cargo-lock
 
 "#]]);
 }
 
 #[cargo_test]
-fn check_build_should_not_lock_artifact_dir() {
+fn check_build_should_lock_artifact_dir() {
     let p = project()
         .file("src/main.rs", r#"fn main() { println!("Hello, World!") }"#)
         .build();
 
     p.cargo("check").enable_mac_dsym().run();
-    assert!(!p.root().join("target/debug/.cargo-lock").exists());
+    assert!(p.root().join("target/debug/.cargo-lock").exists());
 }