Do not lock the artifact-dir for check builds (#16230)

### What does this PR try to resolve?

This PR modifies the locking logic to avoid locking `artifact-dir` for
check builds.
Part of #4282

Note: This change is **not** behind `-Zbuild-dir-new-layout` or
`-Zfine-grain-locking` unstable flags.

### How to test and review this PR?

See the tests included in the PR.
You can run `cargo check` to verify.

r? @epage

Author: epage

src/cargo/core/compiler/build_context/target_info.rs

@@ -1181,7 +1181,14 @@ impl RustDocFingerprint {
             .bcx
             .all_kinds
             .iter()
-            .map(|kind| build_runner.files().layout(*kind).artifact_dir().doc())
+            .map(|kind| {
+                build_runner
+                    .files()
+                    .layout(*kind)
+                    .artifact_dir()
+                    .expect("artifact-dir was not locked")
+                    .doc()
+            })
             .filter(|path| path.exists())
             .try_for_each(|path| clean_doc(path))?;
         write_fingerprint()?;

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

@@ -211,7 +211,11 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
         // Docscrape units need to have doc/ set as the out_dir so sources for reverse-dependencies
         // will be put into doc/ and not into deps/ where the *.examples files are stored.
         if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
-            self.layout(unit.kind).artifact_dir().doc().to_path_buf()
+            self.layout(unit.kind)
+                .artifact_dir()
+                .expect("artifact-dir was not locked")
+                .doc()
+                .to_path_buf()
         } else if unit.mode.is_doc_test() {
             panic!("doc tests do not have an out dir");
         } else if unit.target.is_custom_build() {
@@ -249,8 +253,8 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
     }
 
     /// Returns the final artifact path for the host (`/…/target/debug`)
-    pub fn host_dest(&self) -> &Path {
-        self.host.artifact_dir().dest()
+    pub fn host_dest(&self) -> Option<&Path> {
+        self.host.artifact_dir().map(|v| v.dest())
     }
 
     /// Returns the root of the build output tree for the host (`/…/build-dir`)
@@ -283,8 +287,8 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
     }
 
     /// Directory where timing output should go.
-    pub fn timings_dir(&self) -> &Path {
-        self.host.artifact_dir().timings()
+    pub fn timings_dir(&self) -> Option<&Path> {
+        self.host.artifact_dir().map(|v| v.timings())
     }
 
     /// Returns the path for a file in the fingerprint directory.
@@ -378,9 +382,11 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
         target: &Target,
         kind: CompileKind,
         bcx: &BuildContext<'_, '_>,
-    ) -> CargoResult<PathBuf> {
+    ) -> CargoResult<Option<PathBuf>> {
         assert!(target.is_bin());
-        let dest = self.layout(kind).artifact_dir().dest();
+        let Some(dest) = self.layout(kind).artifact_dir().map(|v| v.dest()) else {
+            return Ok(None);
+        };
         let info = bcx.target_data.info(kind);
         let (file_types, _) = info
             .rustc_outputs(
@@ -396,7 +402,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
             .find(|file_type| file_type.flavor == FileFlavor::Normal)
             .expect("target must support `bin`");
 
-        Ok(dest.join(file_type.uplift_filename(target)))
+        Ok(Some(dest.join(file_type.uplift_filename(target))))
     }
 
     /// Returns the filenames that the given unit will generate.
@@ -450,12 +456,17 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
             // Examples live in their own little world.
             self.layout(unit.kind)
                 .artifact_dir()
+                .expect("artifact-dir was not locked")
                 .examples()
                 .join(filename)
         } else if unit.target.is_custom_build() {
             self.build_script_dir(unit).join(filename)
         } else {
-            self.layout(unit.kind).artifact_dir().dest().join(filename)
+            self.layout(unit.kind)
+                .artifact_dir()
+                .expect("artifact-dir was not locked")
+                .dest()
+                .join(filename)
         };
         if from_path == uplift_path {
             // This can happen with things like examples that reside in the

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, artifact};
+use crate::core::compiler::{self, Unit, UserIntent, artifact};
 use crate::util::cache_lock::CacheLockMode;
 use crate::util::errors::CargoResult;
 use annotate_snippets::{Level, Message};
@@ -352,11 +352,32 @@ 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();
-        let host_layout = Layout::new(self.bcx.ws, None, &dest)?;
+        // 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 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)?;
+                let layout = Layout::new(
+                    self.bcx.ws,
+                    Some(target),
+                    &dest,
+                    must_take_artifact_dir_lock,
+                )?;
                 targets.insert(target, layout);
             }
         }
@@ -392,9 +413,11 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
         let files = self.files.as_ref().unwrap();
         for &kind in self.bcx.all_kinds.iter() {
             let layout = files.layout(kind);
-            self.compilation
-                .root_output
-                .insert(kind, layout.artifact_dir().dest().to_path_buf());
+            if let Some(artifact_dir) = layout.artifact_dir() {
+                self.compilation
+                    .root_output
+                    .insert(kind, artifact_dir.dest().to_path_buf());
+            }
             if self.bcx.gctx.cli_unstable().build_dir_new_layout {
                 for (unit, _) in self.bcx.unit_graph.iter() {
                     let dep_dir = self.files().deps_dir(unit);

src/cargo/core/compiler/compilation.rs

@@ -305,12 +305,14 @@ impl<'gctx> Compilation<'gctx> {
             }
             search_path.push(self.deps_output[&CompileKind::Host].clone());
         } else {
-            search_path.extend(super::filter_dynamic_search_path(
-                self.native_dirs.iter(),
-                &self.root_output[&kind],
-            ));
+            if let Some(path) = self.root_output.get(&kind) {
+                search_path.extend(super::filter_dynamic_search_path(
+                    self.native_dirs.iter(),
+                    path,
+                ));
+                search_path.push(path.clone());
+            }
             search_path.push(self.deps_output[&kind].clone());
-            search_path.push(self.root_output[&kind].clone());
             // For build-std, we don't want to accidentally pull in any shared
             // libs from the sysroot that ships with rustc. This may not be
             // required (at least I cannot craft a situation where it

src/cargo/core/compiler/custom_build.rs

@@ -473,7 +473,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
     let output_file = script_run_dir.join("output");
     let err_file = script_run_dir.join("stderr");
     let root_output_file = script_run_dir.join("root-output");
-    let host_target_root = build_runner.files().host_dest().to_path_buf();
+    let host_target_root = build_runner.files().host_dest().map(|v| v.to_path_buf());
     let all = (
         id,
         library_name.clone(),
@@ -541,12 +541,14 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
                     );
                 }
             }
-            if let Some(build_scripts) = build_scripts {
+            if let Some(build_scripts) = build_scripts
+                && let Some(ref host_target_root) = host_target_root
+            {
                 super::add_plugin_deps(
                     &mut cmd,
                     &build_script_outputs,
                     &build_scripts,
-                    &host_target_root,
+                    host_target_root,
                 )?;
             }
         }

src/cargo/core/compiler/layout.rs

@@ -112,7 +112,7 @@ use std::path::{Path, PathBuf};
 ///
 /// See module docs for more information.
 pub struct Layout {
-    artifact_dir: ArtifactDirLayout,
+    artifact_dir: Option<ArtifactDirLayout>,
     build_dir: BuildDirLayout,
 }
 
@@ -127,6 +127,7 @@ 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();
@@ -150,15 +151,6 @@ 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
@@ -169,21 +161,38 @@ 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");
 
-        Ok(Layout {
-            artifact_dir: ArtifactDirLayout {
+        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 {
                 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,
@@ -201,14 +210,16 @@ impl Layout {
 
     /// Makes sure all directories stored in the Layout exist on the filesystem.
     pub fn prepare(&mut self) -> CargoResult<()> {
-        self.artifact_dir.prepare()?;
+        if let Some(ref mut artifact_dir) = self.artifact_dir {
+            artifact_dir.prepare()?;
+        }
         self.build_dir.prepare()?;
 
         Ok(())
     }
 
-    pub fn artifact_dir(&self) -> &ArtifactDirLayout {
-        &self.artifact_dir
+    pub fn artifact_dir(&self) -> Option<&ArtifactDirLayout> {
+        self.artifact_dir.as_ref()
     }
 
     pub fn build_dir(&self) -> &BuildDirLayout {

src/cargo/core/compiler/mod.rs

@@ -298,7 +298,7 @@ fn rustc(
     exec.init(build_runner, unit);
     let exec = exec.clone();
 
-    let root_output = build_runner.files().host_dest().to_path_buf();
+    let root_output = build_runner.files().host_dest().map(|v| v.to_path_buf());
     let build_dir = build_runner.bcx.ws.build_dir().into_path_unlocked();
     let pkg_root = unit.pkg.root().to_path_buf();
     let cwd = rustc
@@ -361,7 +361,9 @@ fn rustc(
                 current_id,
                 mode,
             )?;
-            add_plugin_deps(&mut rustc, &script_outputs, &build_scripts, &root_output)?;
+            if let Some(ref root_output) = root_output {
+                add_plugin_deps(&mut rustc, &script_outputs, &build_scripts, root_output)?;
+            }
             add_custom_flags(&mut rustc, &script_outputs, script_metadatas)?;
         }
 
@@ -1421,11 +1423,15 @@ fn build_base_args(
             .iter()
             .filter(|target| target.is_bin())
         {
-            let exe_path = build_runner.files().bin_link_for_target(
-                bin_target,
-                unit.kind,
-                build_runner.bcx,
-            )?;
+            // For `cargo check` builds we do not uplift the CARGO_BIN_EXE_ artifacts to the
+            // artifact-dir. We do not want to provide a path to a non-existent binary but we still
+            // need to provide *something* so `env!("CARGO_BIN_EXE_...")` macros will compile.
+            let exe_path = build_runner
+                .files()
+                .bin_link_for_target(bin_target, unit.kind, build_runner.bcx)?
+                .map(|path| path.as_os_str().to_os_string())
+                .unwrap_or_else(|| OsString::from(format!("placeholder:{}", bin_target.name())));
+
             let name = bin_target
                 .binary_filename()
                 .unwrap_or(bin_target.name().to_string());

src/cargo/core/compiler/timings.rs

@@ -457,7 +457,10 @@ impl<'gctx> Timings<'gctx> {
     ) -> CargoResult<()> {
         let duration = self.start.elapsed().as_secs_f64();
         let timestamp = self.start_str.replace(&['-', ':'][..], "");
-        let timings_path = build_runner.files().timings_dir();
+        let timings_path = build_runner
+            .files()
+            .timings_dir()
+            .expect("artifact-dir was not locked");
         paths::create_dir_all(&timings_path)?;
         let filename = timings_path.join(format!("cargo-timing-{}.html", timestamp));
         let mut f = BufWriter::new(paths::create(&filename)?);

src/cargo/ops/cargo_clean.rs

@@ -116,15 +116,17 @@ 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)?;
+    let host_layout = Layout::new(ws, None, &prof_dir_name, true)?;
     // 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) {
-                Ok(layout) => Some(Ok((kind, layout))),
-                Err(e) => Some(Err(e)),
-            },
+            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::Host => None,
         })
         .collect::<CargoResult<_>>()?;
@@ -236,19 +238,18 @@ fn clean_specs(
                     let (file_types, _unsupported) = target_data
                         .info(*compile_kind)
                         .rustc_outputs(mode, target.kind(), triple, clean_ctx.gctx)?;
+                    let artifact_dir = layout
+                        .artifact_dir()
+                        .expect("artifact-dir was not locked during clean");
                     let (dir, uplift_dir) = match target.kind() {
-                        TargetKind::ExampleBin | TargetKind::ExampleLib(..) => (
-                            layout.build_dir().examples(),
-                            Some(layout.artifact_dir().examples()),
-                        ),
+                        TargetKind::ExampleBin | TargetKind::ExampleLib(..) => {
+                            (layout.build_dir().examples(), Some(artifact_dir.examples()))
+                        }
                         // Tests/benchmarks are never uplifted.
                         TargetKind::Test | TargetKind::Bench => {
                             (layout.build_dir().legacy_deps(), None)
                         }
-                        _ => (
-                            layout.build_dir().legacy_deps(),
-                            Some(layout.artifact_dir().dest()),
-                        ),
+                        _ => (layout.build_dir().legacy_deps(), Some(artifact_dir.dest())),
                     };
                     let mut dir_glob_str = escape_glob_path(dir)?;
                     let dir_glob = Path::new(&dir_glob_str);