Skip to content

fix: Resolve target-dir more precisely #20920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+140 −136
Open

fix: Resolve target-dir more precisely #20920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f3539a
fix: Resolve `target-dir` more precisely
ShoyuVanilla Oct 27, 2025
b50af95
fix: Canonicalize flycheck output path
ShoyuVanilla Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions crates/project-model/src/build_dependencies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ impl WorkspaceBuildScripts {
config,
&allowed_features,
workspace.manifest_path(),
workspace.target_directory().as_ref(),
current_dir,
sysroot,
toolchain,
Expand All @@ -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,
Expand Down Expand Up @@ -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>,
Expand All @@ -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 {
Expand Down
34 changes: 25 additions & 9 deletions crates/project-model/src/cargo_workspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleting this single line resulted in deleting/simplifying extra dozens of lines 😄

let Self {
mut command,
manifest_path: _,
Expand Down
2 changes: 1 addition & 1 deletion crates/project-model/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
7 changes: 2 additions & 5 deletions crates/project-model/src/sysroot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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");
Expand All @@ -233,7 +232,6 @@ impl Sysroot {
match self.load_library_via_cargo(
&library_manifest,
src_root,
target_dir,
cargo_config,
no_deps,
progress,
Expand Down Expand Up @@ -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),
Expand All @@ -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.
Expand Down
8 changes: 2 additions & 6 deletions crates/project-model/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
72 changes: 12 additions & 60 deletions crates/project-model/src/workspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -383,7 +378,6 @@ impl ProjectWorkspace {
toolchain.clone(),
)),
config.no_deps,
&target_dir,
progress,
)
});
Expand Down Expand Up @@ -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
Expand All @@ -486,7 +474,6 @@ impl ProjectWorkspace {
sysroot.load_workspace(
&RustSourceWorkspaceConfig::Json(*sysroot_project),
config.no_deps,
&target_dir,
progress,
)
} else {
Expand All @@ -497,7 +484,6 @@ impl ProjectWorkspace {
toolchain.clone(),
)),
config.no_deps,
&target_dir,
progress,
)
}
Expand Down Expand Up @@ -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(
Expand All @@ -558,7 +539,6 @@ impl ProjectWorkspace {
toolchain.clone(),
)),
config.no_deps,
&target_dir,
&|_| (),
);
if let Some(loaded_sysroot) = loaded_sysroot {
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
}
9 changes: 2 additions & 7 deletions crates/rust-analyzer/src/cli/rustc_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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);
}
Expand Down
Loading