Merge pull request #20920 from ShoyuVanilla/target-dirs

fix: Resolve `target-dir` more precisely

Author: ShoyuVanilla

src/tools/rust-analyzer/crates/project-model/src/build_dependencies.rs

@@ -86,6 +86,7 @@ impl WorkspaceBuildScripts {
             config,
             &allowed_features,
             workspace.manifest_path(),
+            workspace.target_directory().as_ref(),
             current_dir,
             sysroot,
             toolchain,
@@ -106,8 +107,9 @@ impl WorkspaceBuildScripts {
         let (_guard, cmd) = Self::build_command(
             config,
             &Default::default(),
-            // This is not gonna be used anyways, so just construct a dummy here
+            // These are not gonna be used anyways, so just construct a dummy here
             &ManifestPath::try_from(working_directory.clone()).unwrap(),
+            working_directory.as_ref(),
             working_directory,
             &Sysroot::empty(),
             None,
@@ -430,6 +432,7 @@ impl WorkspaceBuildScripts {
         config: &CargoConfig,
         allowed_features: &FxHashSet<String>,
         manifest_path: &ManifestPath,
+        target_dir: &Utf8Path,
         current_dir: &AbsPath,
         sysroot: &Sysroot,
         toolchain: Option<&semver::Version>,
@@ -450,8 +453,9 @@ impl WorkspaceBuildScripts {
                 cmd.arg("--manifest-path");
                 cmd.arg(manifest_path);
 
-                if let Some(target_dir) = &config.target_dir {
-                    cmd.arg("--target-dir").arg(target_dir);
+                if let Some(target_dir) = config.target_dir_config.target_dir(Some(target_dir)) {
+                    cmd.arg("--target-dir");
+                    cmd.arg(target_dir.as_ref());
                 }
 
                 if let Some(target) = &config.target {

src/tools/rust-analyzer/crates/project-model/src/cargo_workspace.rs

@@ -1,7 +1,6 @@
 //! See [`CargoWorkspace`].
 
-use std::ops;
-use std::str::from_utf8;
+use std::{borrow::Cow, ops, str::from_utf8};
 
 use anyhow::Context;
 use base_db::Env;
@@ -95,6 +94,29 @@ impl Default for CargoFeatures {
     }
 }
 
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
+pub enum TargetDirectoryConfig {
+    #[default]
+    None,
+    UseSubdirectory,
+    Directory(Utf8PathBuf),
+}
+
+impl TargetDirectoryConfig {
+    pub fn target_dir<'a>(
+        &'a self,
+        ws_target_dir: Option<&'a Utf8Path>,
+    ) -> Option<Cow<'a, Utf8Path>> {
+        match self {
+            TargetDirectoryConfig::None => None,
+            TargetDirectoryConfig::UseSubdirectory => {
+                Some(Cow::Owned(ws_target_dir?.join("rust-analyzer")))
+            }
+            TargetDirectoryConfig::Directory(dir) => Some(Cow::Borrowed(dir)),
+        }
+    }
+}
+
 #[derive(Default, Clone, Debug, PartialEq, Eq)]
 pub struct CargoConfig {
     /// Whether to pass `--all-targets` to cargo invocations.
@@ -121,7 +143,7 @@ pub struct CargoConfig {
     pub extra_env: FxHashMap<String, Option<String>>,
     pub invocation_strategy: InvocationStrategy,
     /// Optional path to use instead of `target` when building
-    pub target_dir: Option<Utf8PathBuf>,
+    pub target_dir_config: TargetDirectoryConfig,
     /// Gate `#[test]` behind `#[cfg(test)]`
     pub set_test: bool,
     /// Load the project without any dependencies
@@ -715,21 +737,15 @@ impl FetchMetadata {
         }
     }
 
-    pub(crate) fn no_deps_metadata(&self) -> Option<&cargo_metadata::Metadata> {
-        self.no_deps_result.as_ref().ok()
-    }
-
     /// Executes the metadata-fetching command.
     ///
     /// A successful result may still contain a metadata error if the full fetch failed,
     /// but the fallback `--no-deps` pre-fetch succeeded during command construction.
     pub(crate) fn exec(
         self,
-        target_dir: &Utf8Path,
         locked: bool,
         progress: &dyn Fn(String),
     ) -> anyhow::Result<(cargo_metadata::Metadata, Option<anyhow::Error>)> {
-        _ = target_dir;
         let Self {
             mut command,
             manifest_path: _,

src/tools/rust-analyzer/crates/project-model/src/lib.rs

@@ -62,7 +62,7 @@ pub use crate::{
     build_dependencies::{ProcMacroDylibPath, WorkspaceBuildScripts},
     cargo_workspace::{
         CargoConfig, CargoFeatures, CargoMetadataConfig, CargoWorkspace, Package, PackageData,
-        PackageDependency, RustLibSource, Target, TargetData, TargetKind,
+        PackageDependency, RustLibSource, Target, TargetData, TargetDirectoryConfig, TargetKind,
     },
     manifest_path::ManifestPath,
     project_json::{ProjectJson, ProjectJsonData},

src/tools/rust-analyzer/crates/project-model/src/sysroot.rs

@@ -9,7 +9,7 @@ use std::{env, fs, ops::Not, path::Path, process::Command};
 
 use anyhow::{Result, format_err};
 use itertools::Itertools;
-use paths::{AbsPath, AbsPathBuf, Utf8Path, Utf8PathBuf};
+use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
 use rustc_hash::FxHashMap;
 use stdx::format_to;
 use toolchain::{Tool, probe_for_binary};
@@ -219,7 +219,6 @@ impl Sysroot {
         &self,
         sysroot_source_config: &RustSourceWorkspaceConfig,
         no_deps: bool,
-        target_dir: &Utf8Path,
         progress: &dyn Fn(String),
     ) -> Option<RustLibSrcWorkspace> {
         assert!(matches!(self.workspace, RustLibSrcWorkspace::Empty), "workspace already loaded");
@@ -233,7 +232,6 @@ impl Sysroot {
                 match self.load_library_via_cargo(
                     &library_manifest,
                     src_root,
-                    target_dir,
                     cargo_config,
                     no_deps,
                     progress,
@@ -328,7 +326,6 @@ impl Sysroot {
         &self,
         library_manifest: &ManifestPath,
         current_dir: &AbsPath,
-        target_dir: &Utf8Path,
         cargo_config: &CargoMetadataConfig,
         no_deps: bool,
         progress: &dyn Fn(String),
@@ -345,7 +342,7 @@ impl Sysroot {
         let locked = true;
         let (mut res, err) =
             FetchMetadata::new(library_manifest, current_dir, &cargo_config, self, no_deps)
-                .exec(target_dir, locked, progress)?;
+                .exec(locked, progress)?;
 
         // Patch out `rustc-std-workspace-*` crates to point to the real crates.
         // This is done prior to `CrateGraph` construction to prevent de-duplication logic from failing.

src/tools/rust-analyzer/crates/project-model/src/tests.rs

@@ -238,12 +238,8 @@ fn smoke_test_real_sysroot_cargo() {
     );
     let cwd = AbsPathBuf::assert_utf8(temp_dir().join("smoke_test_real_sysroot_cargo"));
     std::fs::create_dir_all(&cwd).unwrap();
-    let loaded_sysroot = sysroot.load_workspace(
-        &RustSourceWorkspaceConfig::default_cargo(),
-        false,
-        &Utf8PathBuf::default(),
-        &|_| (),
-    );
+    let loaded_sysroot =
+        sysroot.load_workspace(&RustSourceWorkspaceConfig::default_cargo(), false, &|_| ());
     if let Some(loaded_sysroot) = loaded_sysroot {
         sysroot.set_workspace(loaded_sysroot);
     }

src/tools/rust-analyzer/crates/project-model/src/workspace.rs

@@ -16,7 +16,7 @@ use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
 use rustc_hash::{FxHashMap, FxHashSet};
 use semver::Version;
 use span::{Edition, FileId};
-use toolchain::{NO_RUSTUP_AUTO_INSTALL_ENV, Tool};
+use toolchain::Tool;
 use tracing::instrument;
 use tracing::{debug, error, info};
 use triomphe::Arc;
@@ -295,11 +295,6 @@ impl ProjectWorkspace {
             &sysroot,
             *no_deps,
         );
-        let target_dir = config
-            .target_dir
-            .clone()
-            .or_else(|| fetch_metadata.no_deps_metadata().map(|m| m.target_directory.clone()))
-            .unwrap_or_else(|| workspace_dir.join("target").into());
 
         // We spawn a bunch of processes to query various information about the workspace's
         // toolchain and sysroot
@@ -345,7 +340,7 @@ impl ProjectWorkspace {
                         },
                         &sysroot,
                         *no_deps,
-                    ).exec(&target_dir, true, progress) {
+                    ).exec(true, progress) {
                         Ok((meta, _error)) => {
                             let workspace = CargoWorkspace::new(
                                 meta,
@@ -374,7 +369,7 @@ impl ProjectWorkspace {
                 })
             });
 
-            let cargo_metadata = s.spawn(|| fetch_metadata.exec(&target_dir, false, progress));
+            let cargo_metadata = s.spawn(|| fetch_metadata.exec(false, progress));
             let loaded_sysroot = s.spawn(|| {
                 sysroot.load_workspace(
                     &RustSourceWorkspaceConfig::CargoMetadata(sysroot_metadata_config(
@@ -383,7 +378,6 @@ impl ProjectWorkspace {
                         toolchain.clone(),
                     )),
                     config.no_deps,
-                    &target_dir,
                     progress,
                 )
             });
@@ -463,12 +457,6 @@ impl ProjectWorkspace {
         let targets = target_tuple::get(query_config, config.target.as_deref(), &config.extra_env)
             .unwrap_or_default();
         let toolchain = version::get(query_config, &config.extra_env).ok().flatten();
-        let project_root = project_json.project_root();
-        let target_dir = config
-            .target_dir
-            .clone()
-            .or_else(|| cargo_target_dir(project_json.manifest()?, &config.extra_env, &sysroot))
-            .unwrap_or_else(|| project_root.join("target").into());
 
         // We spawn a bunch of processes to query various information about the workspace's
         // toolchain and sysroot
@@ -486,7 +474,6 @@ impl ProjectWorkspace {
                     sysroot.load_workspace(
                         &RustSourceWorkspaceConfig::Json(*sysroot_project),
                         config.no_deps,
-                        &target_dir,
                         progress,
                     )
                 } else {
@@ -497,7 +484,6 @@ impl ProjectWorkspace {
                             toolchain.clone(),
                         )),
                         config.no_deps,
-                        &target_dir,
                         progress,
                     )
                 }
@@ -545,11 +531,6 @@ impl ProjectWorkspace {
             .unwrap_or_default();
         let rustc_cfg = rustc_cfg::get(query_config, None, &config.extra_env);
         let target_data = target_data::get(query_config, None, &config.extra_env);
-        let target_dir = config
-            .target_dir
-            .clone()
-            .or_else(|| cargo_target_dir(detached_file, &config.extra_env, &sysroot))
-            .unwrap_or_else(|| dir.join("target").into());
 
         let loaded_sysroot = sysroot.load_workspace(
             &RustSourceWorkspaceConfig::CargoMetadata(sysroot_metadata_config(
@@ -558,7 +539,6 @@ impl ProjectWorkspace {
                 toolchain.clone(),
             )),
             config.no_deps,
-            &target_dir,
             &|_| (),
         );
         if let Some(loaded_sysroot) = loaded_sysroot {
@@ -579,21 +559,15 @@ impl ProjectWorkspace {
             &sysroot,
             config.no_deps,
         );
-        let target_dir = config
-            .target_dir
-            .clone()
-            .or_else(|| fetch_metadata.no_deps_metadata().map(|m| m.target_directory.clone()))
-            .unwrap_or_else(|| dir.join("target").into());
-        let cargo_script =
-            fetch_metadata.exec(&target_dir, false, &|_| ()).ok().map(|(ws, error)| {
-                let cargo_config_extra_env =
-                    cargo_config_env(detached_file, &config_file, &config.extra_env);
-                (
-                    CargoWorkspace::new(ws, detached_file.clone(), cargo_config_extra_env, false),
-                    WorkspaceBuildScripts::default(),
-                    error.map(Arc::new),
-                )
-            });
+        let cargo_script = fetch_metadata.exec(false, &|_| ()).ok().map(|(ws, error)| {
+            let cargo_config_extra_env =
+                cargo_config_env(detached_file, &config_file, &config.extra_env);
+            (
+                CargoWorkspace::new(ws, detached_file.clone(), cargo_config_extra_env, false),
+                WorkspaceBuildScripts::default(),
+                error.map(Arc::new),
+            )
+        });
 
         Ok(ProjectWorkspace {
             kind: ProjectWorkspaceKind::DetachedFile {
@@ -1902,25 +1876,3 @@ fn sysroot_metadata_config(
         kind: "sysroot",
     }
 }
-
-fn cargo_target_dir(
-    manifest: &ManifestPath,
-    extra_env: &FxHashMap<String, Option<String>>,
-    sysroot: &Sysroot,
-) -> Option<Utf8PathBuf> {
-    let cargo = sysroot.tool(Tool::Cargo, manifest.parent(), extra_env);
-    let mut meta = cargo_metadata::MetadataCommand::new();
-    meta.env(NO_RUSTUP_AUTO_INSTALL_ENV.0, NO_RUSTUP_AUTO_INSTALL_ENV.1);
-    meta.cargo_path(cargo.get_program());
-    meta.manifest_path(manifest);
-    // `--no-deps` doesn't (over)write lockfiles as it doesn't do any package resolve.
-    // So we can use it to get `target_directory` before copying lockfiles
-    meta.no_deps();
-    let mut other_options = vec![];
-    if manifest.is_rust_manifest() {
-        meta.env("RUSTC_BOOTSTRAP", "1");
-        other_options.push("-Zscript".to_owned());
-    }
-    meta.other_options(other_options);
-    meta.exec().map(|m| m.target_directory).ok()
-}

src/tools/rust-analyzer/crates/rust-analyzer/src/cli/rustc_tests.rs

@@ -9,7 +9,6 @@ use hir::{ChangeWithProcMacros, Crate};
 use ide::{AnalysisHost, DiagnosticCode, DiagnosticsConfig};
 use ide_db::base_db;
 use itertools::Either;
-use paths::Utf8PathBuf;
 use profile::StopWatch;
 use project_model::toolchain_info::{QueryConfig, target_data};
 use project_model::{
@@ -75,12 +74,8 @@ impl Tester {
         };
 
         let mut sysroot = Sysroot::discover(tmp_file.parent().unwrap(), &cargo_config.extra_env);
-        let loaded_sysroot = sysroot.load_workspace(
-            &RustSourceWorkspaceConfig::default_cargo(),
-            false,
-            &Utf8PathBuf::default(),
-            &|_| (),
-        );
+        let loaded_sysroot =
+            sysroot.load_workspace(&RustSourceWorkspaceConfig::default_cargo(), false, &|_| ());
         if let Some(loaded_sysroot) = loaded_sysroot {
             sysroot.set_workspace(loaded_sysroot);
         }