From aeef7b644b135c80c2fc7a28954d97194df7fabc Mon Sep 17 00:00:00 2001 From: Victor Song Date: Fri, 29 Sep 2023 03:49:37 -0500 Subject: [PATCH 1/7] Add config option to use `rust-analyzer` specific target dir Adds a Rust Analyzer configuration option to set a custom target directory for builds. This is a workaround for Rust Analyzer blocking debug builds while running `cargo check`. This change should close #6007 --- crates/rust-analyzer/src/config.rs | 200 ++++++++++++++++++++++++----- docs/user/generated_config.adoc | 9 ++ editors/code/package.json | 15 +++ 3 files changed, 190 insertions(+), 34 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 8e780baa36dd..3cd6fa49b204 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -480,6 +480,13 @@ config_data! { /// tests or binaries. For example, it may be `--release`. runnables_extraArgs: Vec = "[]", + /// Optional path to a rust-analyzer specific target directory. + /// This is useful to prevent rust-analyzer's `cargo check` from blocking builds. + /// + /// Set to `true` to use a subdirectory of the existing target directory or + /// set to a path to use that path. + rust_analyzerTargetDir: Option = "null", + /// Path to the Cargo.toml of the rust compiler workspace, for usage in rustc_private /// projects, or "discover" to try to automatically find it if the `rustc-dev` component /// is installed. @@ -1192,6 +1199,7 @@ impl Config { } pub fn cargo(&self) -> CargoConfig { + let target_directory = self.target_dir_from_config(); let rustc_source = self.data.rustc_source.as_ref().map(|rustc_src| { if rustc_src == "discover" { RustLibSource::Discover @@ -1209,6 +1217,10 @@ impl Config { let sysroot_src = self.data.cargo_sysrootSrc.as_ref().map(|sysroot| self.root_path.join(sysroot)); + let mut extra_args = self.data.cargo_extraArgs.clone(); + + add_target_dir_to_args(&mut extra_args, target_directory); + CargoConfig { features: match &self.data.cargo_features { CargoFeaturesDef::All => CargoFeatures::All, @@ -1261,7 +1273,7 @@ impl Config { InvocationLocation::Workspace => project_model::InvocationLocation::Workspace, }, run_build_script_command: self.data.cargo_buildScripts_overrideCommand.clone(), - extra_args: self.data.cargo_extraArgs.clone(), + extra_args, extra_env: self.data.cargo_extraEnv.clone(), } } @@ -1281,10 +1293,14 @@ impl Config { } pub fn flycheck(&self) -> FlycheckConfig { + let target_directory = self.target_dir_from_config(); + match &self.data.check_overrideCommand { Some(args) if !args.is_empty() => { let mut args = args.clone(); let command = args.remove(0); + add_target_dir_to_args(&mut args, target_directory); + FlycheckConfig::CustomCommand { command, args, @@ -1303,42 +1319,61 @@ impl Config { }, } } - Some(_) | None => FlycheckConfig::CargoCommand { - command: self.data.check_command.clone(), - target_triples: self - .data - .check_targets - .clone() - .and_then(|targets| match &targets.0[..] { - [] => None, - targets => Some(targets.into()), - }) - .unwrap_or_else(|| self.data.cargo_target.clone().into_iter().collect()), - all_targets: self.data.check_allTargets, - no_default_features: self - .data - .check_noDefaultFeatures - .unwrap_or(self.data.cargo_noDefaultFeatures), - all_features: matches!( - self.data.check_features.as_ref().unwrap_or(&self.data.cargo_features), - CargoFeaturesDef::All - ), - features: match self - .data - .check_features - .clone() - .unwrap_or_else(|| self.data.cargo_features.clone()) - { - CargoFeaturesDef::All => vec![], - CargoFeaturesDef::Selected(it) => it, - }, - extra_args: self.check_extra_args(), - extra_env: self.check_extra_env(), - ansi_color_output: self.color_diagnostic_output(), - }, + Some(_) | None => { + let mut extra_args = self.check_extra_args(); + add_target_dir_to_args(&mut extra_args, target_directory); + + FlycheckConfig::CargoCommand { + command: self.data.check_command.clone(), + target_triples: self + .data + .check_targets + .clone() + .and_then(|targets| match &targets.0[..] { + [] => None, + targets => Some(targets.into()), + }) + .unwrap_or_else(|| self.data.cargo_target.clone().into_iter().collect()), + all_targets: self.data.check_allTargets, + no_default_features: self + .data + .check_noDefaultFeatures + .unwrap_or(self.data.cargo_noDefaultFeatures), + all_features: matches!( + self.data.check_features.as_ref().unwrap_or(&self.data.cargo_features), + CargoFeaturesDef::All + ), + features: match self + .data + .check_features + .clone() + .unwrap_or_else(|| self.data.cargo_features.clone()) + { + CargoFeaturesDef::All => vec![], + CargoFeaturesDef::Selected(it) => it, + }, + extra_args, + extra_env: self.check_extra_env(), + ansi_color_output: self.color_diagnostic_output(), + } + } } } + fn target_dir_from_config(&self) -> Option { + self.data + .rust_analyzerTargetDir + .as_ref() + .map(|target_dir| match target_dir { + TargetDirectory::UseSubdirectory(yes) if *yes => { + Some(String::from("target/rust-analyzer")) + } + TargetDirectory::UseSubdirectory(_) => None, + TargetDirectory::Directory(dir) => Some(dir.clone()), + }) + .flatten() + } + pub fn check_on_save(&self) -> bool { self.data.checkOnSave } @@ -1690,6 +1725,13 @@ impl Config { self.is_visual_studio_code } } + +fn add_target_dir_to_args(args: &mut Vec, target_dir: Option) { + if let Some(target_dir) = target_dir { + args.push(format!("--target-dir={}", target_dir)); + } +} + // Deserialization definitions macro_rules! create_bool_or_string_de { @@ -2037,6 +2079,14 @@ pub enum MemoryLayoutHoverRenderKindDef { Both, } +#[derive(Deserialize, Debug, Clone, PartialEq)] +#[serde(rename_all = "snake_case")] +#[serde(untagged)] +pub enum TargetDirectory { + UseSubdirectory(bool), + Directory(String), +} + macro_rules! _config_data { (struct $name:ident { $( @@ -2465,6 +2515,19 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json }, ], }, + "Option" => set! { + "anyOf": [ + { + "type": "null" + }, + { + "type": "boolean" + }, + { + "type": "string" + }, + ], + }, _ => panic!("missing entry for {ty}: {default}"), } @@ -2625,4 +2688,73 @@ mod tests { Some(AbsPathBuf::try_from(project_root().join("./server")).unwrap()) ); } + + #[test] + fn cargo_target_dir_unset() { + let mut config = Config::new( + AbsPathBuf::try_from(project_root()).unwrap(), + Default::default(), + vec![], + false, + ); + config + .update(serde_json::json!({ + "rust": { "analyzerTargetDir": null } + })) + .unwrap(); + assert_eq!(config.data.rust_analyzerTargetDir, None); + assert_eq!(config.cargo().extra_args.len(), 0); + assert!( + matches!(config.flycheck(), FlycheckConfig::CargoCommand { extra_args, .. } if extra_args.is_empty()) + ); + } + + #[test] + fn cargo_target_dir_subdir() { + let mut config = Config::new( + AbsPathBuf::try_from(project_root()).unwrap(), + Default::default(), + vec![], + false, + ); + config + .update(serde_json::json!({ + "rust": { "analyzerTargetDir": true } + })) + .unwrap(); + assert_eq!( + config.data.rust_analyzerTargetDir, + Some(TargetDirectory::UseSubdirectory(true)) + ); + assert_eq!( + config.cargo().extra_args, + vec!["--target-dir=target/rust-analyzer".to_string()] + ); + assert!( + matches!(config.flycheck(), FlycheckConfig::CargoCommand { extra_args, .. } if extra_args == vec!["--target-dir=target/rust-analyzer".to_string()]) + ); + } + + #[test] + fn cargo_target_dir_relative_dir() { + let mut config = Config::new( + AbsPathBuf::try_from(project_root()).unwrap(), + Default::default(), + vec![], + false, + ); + config + .update(serde_json::json!({ + "rust": { "analyzerTargetDir": "other_folder" } + })) + .unwrap(); + assert_eq!( + config.data.rust_analyzerTargetDir, + Some(TargetDirectory::Directory("other_folder".to_string())) + ); + assert_eq!(config.cargo().extra_args, vec!["--target-dir=other_folder".to_string()]); + assert!( + matches!(config.flycheck(), FlycheckConfig::CargoCommand { extra_args, .. } if extra_args == vec!["--target-dir=other_folder".to_string()]) + ); + } } diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index dec7a5075748..f7ae6afe3860 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -757,6 +757,15 @@ Command to be executed instead of 'cargo' for runnables. Additional arguments to be passed to cargo for runnables such as tests or binaries. For example, it may be `--release`. -- +[[rust-analyzer.rust.analyzerTargetDir]]rust-analyzer.rust.analyzerTargetDir (default: `null`):: ++ +-- +Optional path to a rust-analyzer specific target directory. +This is useful to prevent rust-analyzer's `cargo check` from blocking builds. + +Set to `true` to use a subdirectory of the existing target directory or +set to a path to use that path. +-- [[rust-analyzer.rustc.source]]rust-analyzer.rustc.source (default: `null`):: + -- diff --git a/editors/code/package.json b/editors/code/package.json index 554b05c46c70..e1402cb0f5c4 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -1488,6 +1488,21 @@ "type": "string" } }, + "rust-analyzer.rust.analyzerTargetDir": { + "markdownDescription": "Optional path to a rust-analyzer specific target directory.\nThis is useful to prevent rust-analyzer's `cargo check` from blocking builds.\n\nSet to `true` to use a subdirectory of the existing target directory or\nset to a path to use that path.", + "default": null, + "anyOf": [ + { + "type": "null" + }, + { + "type": "boolean" + }, + { + "type": "string" + } + ] + }, "rust-analyzer.rustc.source": { "markdownDescription": "Path to the Cargo.toml of the rust compiler workspace, for usage in rustc_private\nprojects, or \"discover\" to try to automatically find it if the `rustc-dev` component\nis installed.\n\nAny project which uses rust-analyzer with the rustcPrivate\ncrates must set `[package.metadata.rust-analyzer] rustc_private=true` to use it.\n\nThis option does not take effect until rust-analyzer is restarted.", "default": null, From 53b670059456ba15e4374e5ab7ba4e2468066d0c Mon Sep 17 00:00:00 2001 From: Victor Song Date: Sat, 30 Sep 2023 21:02:12 -0500 Subject: [PATCH 2/7] Add dedicated `target_dir` field to `CargoConfig` and `FlycheckConfig` Add dedicated field for `target_dir` in the configurations for Cargo and Flycheck. Also change the directory to be a `PathBuf` as opposed to a `String` to be more appropriate to the operating system. --- crates/flycheck/src/lib.rs | 5 + crates/project-model/src/build_scripts.rs | 4 + crates/project-model/src/cargo_workspace.rs | 2 + crates/rust-analyzer/src/config.rs | 111 ++++++++------------ 4 files changed, 54 insertions(+), 68 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 2de719af92ce..0749d91eb32a 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -50,6 +50,7 @@ pub enum FlycheckConfig { extra_args: Vec, extra_env: FxHashMap, ansi_color_output: bool, + target_dir: Option, }, CustomCommand { command: String, @@ -308,6 +309,7 @@ impl FlycheckActor { features, extra_env, ansi_color_output, + target_dir, } => { let mut cmd = Command::new(toolchain::cargo()); cmd.arg(command); @@ -340,6 +342,9 @@ impl FlycheckActor { cmd.arg(features.join(" ")); } } + if let Some(target_dir) = target_dir { + cmd.arg("--target-dir").arg(target_dir); + } cmd.envs(extra_env); (cmd, extra_args) } diff --git a/crates/project-model/src/build_scripts.rs b/crates/project-model/src/build_scripts.rs index fb0f3ab7d174..68cd40c040b3 100644 --- a/crates/project-model/src/build_scripts.rs +++ b/crates/project-model/src/build_scripts.rs @@ -73,6 +73,10 @@ impl WorkspaceBuildScripts { cmd.args(["check", "--quiet", "--workspace", "--message-format=json"]); cmd.args(&config.extra_args); + if let Some(target_dir) = &config.target_dir { + cmd.arg("--target-dir").arg(target_dir); + } + // --all-targets includes tests, benches and examples in addition to the // default lib and bins. This is an independent concept from the --target // flag below. diff --git a/crates/project-model/src/cargo_workspace.rs b/crates/project-model/src/cargo_workspace.rs index e47808a2cc9f..ca3d6e0596ca 100644 --- a/crates/project-model/src/cargo_workspace.rs +++ b/crates/project-model/src/cargo_workspace.rs @@ -96,6 +96,8 @@ pub struct CargoConfig { pub extra_env: FxHashMap, pub invocation_strategy: InvocationStrategy, pub invocation_location: InvocationLocation, + /// Optional path to use instead of `target` when building + pub target_dir: Option, } pub type Package = Idx; diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 3cd6fa49b204..8f1543de0045 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -1199,7 +1199,6 @@ impl Config { } pub fn cargo(&self) -> CargoConfig { - let target_directory = self.target_dir_from_config(); let rustc_source = self.data.rustc_source.as_ref().map(|rustc_src| { if rustc_src == "discover" { RustLibSource::Discover @@ -1217,10 +1216,6 @@ impl Config { let sysroot_src = self.data.cargo_sysrootSrc.as_ref().map(|sysroot| self.root_path.join(sysroot)); - let mut extra_args = self.data.cargo_extraArgs.clone(); - - add_target_dir_to_args(&mut extra_args, target_directory); - CargoConfig { features: match &self.data.cargo_features { CargoFeaturesDef::All => CargoFeatures::All, @@ -1273,8 +1268,9 @@ impl Config { InvocationLocation::Workspace => project_model::InvocationLocation::Workspace, }, run_build_script_command: self.data.cargo_buildScripts_overrideCommand.clone(), - extra_args, + extra_args: self.data.cargo_extraArgs.clone(), extra_env: self.data.cargo_extraEnv.clone(), + target_dir: self.target_dir_from_config(), } } @@ -1293,14 +1289,10 @@ impl Config { } pub fn flycheck(&self) -> FlycheckConfig { - let target_directory = self.target_dir_from_config(); - match &self.data.check_overrideCommand { Some(args) if !args.is_empty() => { let mut args = args.clone(); let command = args.remove(0); - add_target_dir_to_args(&mut args, target_directory); - FlycheckConfig::CustomCommand { command, args, @@ -1319,54 +1311,50 @@ impl Config { }, } } - Some(_) | None => { - let mut extra_args = self.check_extra_args(); - add_target_dir_to_args(&mut extra_args, target_directory); - - FlycheckConfig::CargoCommand { - command: self.data.check_command.clone(), - target_triples: self - .data - .check_targets - .clone() - .and_then(|targets| match &targets.0[..] { - [] => None, - targets => Some(targets.into()), - }) - .unwrap_or_else(|| self.data.cargo_target.clone().into_iter().collect()), - all_targets: self.data.check_allTargets, - no_default_features: self - .data - .check_noDefaultFeatures - .unwrap_or(self.data.cargo_noDefaultFeatures), - all_features: matches!( - self.data.check_features.as_ref().unwrap_or(&self.data.cargo_features), - CargoFeaturesDef::All - ), - features: match self - .data - .check_features - .clone() - .unwrap_or_else(|| self.data.cargo_features.clone()) - { - CargoFeaturesDef::All => vec![], - CargoFeaturesDef::Selected(it) => it, - }, - extra_args, - extra_env: self.check_extra_env(), - ansi_color_output: self.color_diagnostic_output(), - } - } + Some(_) | None => FlycheckConfig::CargoCommand { + command: self.data.check_command.clone(), + target_triples: self + .data + .check_targets + .clone() + .and_then(|targets| match &targets.0[..] { + [] => None, + targets => Some(targets.into()), + }) + .unwrap_or_else(|| self.data.cargo_target.clone().into_iter().collect()), + all_targets: self.data.check_allTargets, + no_default_features: self + .data + .check_noDefaultFeatures + .unwrap_or(self.data.cargo_noDefaultFeatures), + all_features: matches!( + self.data.check_features.as_ref().unwrap_or(&self.data.cargo_features), + CargoFeaturesDef::All + ), + features: match self + .data + .check_features + .clone() + .unwrap_or_else(|| self.data.cargo_features.clone()) + { + CargoFeaturesDef::All => vec![], + CargoFeaturesDef::Selected(it) => it, + }, + extra_args: self.check_extra_args(), + extra_env: self.check_extra_env(), + ansi_color_output: self.color_diagnostic_output(), + target_dir: self.target_dir_from_config(), + }, } } - fn target_dir_from_config(&self) -> Option { + fn target_dir_from_config(&self) -> Option { self.data .rust_analyzerTargetDir .as_ref() .map(|target_dir| match target_dir { TargetDirectory::UseSubdirectory(yes) if *yes => { - Some(String::from("target/rust-analyzer")) + Some(PathBuf::from("target/rust-analyzer")) } TargetDirectory::UseSubdirectory(_) => None, TargetDirectory::Directory(dir) => Some(dir.clone()), @@ -1725,13 +1713,6 @@ impl Config { self.is_visual_studio_code } } - -fn add_target_dir_to_args(args: &mut Vec, target_dir: Option) { - if let Some(target_dir) = target_dir { - args.push(format!("--target-dir={}", target_dir)); - } -} - // Deserialization definitions macro_rules! create_bool_or_string_de { @@ -2084,7 +2065,7 @@ pub enum MemoryLayoutHoverRenderKindDef { #[serde(untagged)] pub enum TargetDirectory { UseSubdirectory(bool), - Directory(String), + Directory(PathBuf), } macro_rules! _config_data { @@ -2703,9 +2684,8 @@ mod tests { })) .unwrap(); assert_eq!(config.data.rust_analyzerTargetDir, None); - assert_eq!(config.cargo().extra_args.len(), 0); assert!( - matches!(config.flycheck(), FlycheckConfig::CargoCommand { extra_args, .. } if extra_args.is_empty()) + matches!(config.flycheck(), FlycheckConfig::CargoCommand { target_dir, .. } if target_dir == None) ); } @@ -2726,12 +2706,8 @@ mod tests { config.data.rust_analyzerTargetDir, Some(TargetDirectory::UseSubdirectory(true)) ); - assert_eq!( - config.cargo().extra_args, - vec!["--target-dir=target/rust-analyzer".to_string()] - ); assert!( - matches!(config.flycheck(), FlycheckConfig::CargoCommand { extra_args, .. } if extra_args == vec!["--target-dir=target/rust-analyzer".to_string()]) + matches!(config.flycheck(), FlycheckConfig::CargoCommand { target_dir, .. } if target_dir == Some(PathBuf::from("target/rust-analyzer"))) ); } @@ -2750,11 +2726,10 @@ mod tests { .unwrap(); assert_eq!( config.data.rust_analyzerTargetDir, - Some(TargetDirectory::Directory("other_folder".to_string())) + Some(TargetDirectory::Directory(PathBuf::from("other_folder"))) ); - assert_eq!(config.cargo().extra_args, vec!["--target-dir=other_folder".to_string()]); assert!( - matches!(config.flycheck(), FlycheckConfig::CargoCommand { extra_args, .. } if extra_args == vec!["--target-dir=other_folder".to_string()]) + matches!(config.flycheck(), FlycheckConfig::CargoCommand { target_dir, .. } if target_dir == Some(PathBuf::from("other_folder"))) ); } } From ef0b3bbef1a1b4cd88619f9a5f43d526f96af184 Mon Sep 17 00:00:00 2001 From: Victor Song Date: Sat, 30 Sep 2023 21:21:00 -0500 Subject: [PATCH 3/7] Clarify documentation on new parameter --- crates/rust-analyzer/src/config.rs | 2 +- docs/user/generated_config.adoc | 2 +- editors/code/package.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 8f1543de0045..39a98bcd29a8 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -484,7 +484,7 @@ config_data! { /// This is useful to prevent rust-analyzer's `cargo check` from blocking builds. /// /// Set to `true` to use a subdirectory of the existing target directory or - /// set to a path to use that path. + /// set to a path relative to the workspace to use that path. rust_analyzerTargetDir: Option = "null", /// Path to the Cargo.toml of the rust compiler workspace, for usage in rustc_private diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index f7ae6afe3860..4440caaed89c 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -764,7 +764,7 @@ Optional path to a rust-analyzer specific target directory. This is useful to prevent rust-analyzer's `cargo check` from blocking builds. Set to `true` to use a subdirectory of the existing target directory or -set to a path to use that path. +set to a path relative to the workspace to use that path. -- [[rust-analyzer.rustc.source]]rust-analyzer.rustc.source (default: `null`):: + diff --git a/editors/code/package.json b/editors/code/package.json index e1402cb0f5c4..a18acb98301d 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -1489,7 +1489,7 @@ } }, "rust-analyzer.rust.analyzerTargetDir": { - "markdownDescription": "Optional path to a rust-analyzer specific target directory.\nThis is useful to prevent rust-analyzer's `cargo check` from blocking builds.\n\nSet to `true` to use a subdirectory of the existing target directory or\nset to a path to use that path.", + "markdownDescription": "Optional path to a rust-analyzer specific target directory.\nThis is useful to prevent rust-analyzer's `cargo check` from blocking builds.\n\nSet to `true` to use a subdirectory of the existing target directory or\nset to a path relative to the workspace to use that path.", "default": null, "anyOf": [ { From 3682c3791bff7a98179d256c1b972ee75e91b5d4 Mon Sep 17 00:00:00 2001 From: Victor Song Date: Sat, 30 Sep 2023 21:23:16 -0500 Subject: [PATCH 4/7] Set `CARGO_TARGET_DIR` when using Flycheck custom command --- crates/flycheck/src/lib.rs | 6 ++++++ crates/rust-analyzer/src/config.rs | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 0749d91eb32a..f0d2f79f6332 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -58,6 +58,7 @@ pub enum FlycheckConfig { extra_env: FxHashMap, invocation_strategy: InvocationStrategy, invocation_location: InvocationLocation, + target_dir: Option, }, } @@ -354,10 +355,15 @@ impl FlycheckActor { extra_env, invocation_strategy, invocation_location, + target_dir, } => { let mut cmd = Command::new(command); cmd.envs(extra_env); + if let Some(target_dir) = target_dir { + cmd.env("CARGO_TARGET_DIR", target_dir); + } + match invocation_location { InvocationLocation::Workspace => { match invocation_strategy { diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 39a98bcd29a8..23cf71cff4fa 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -1289,6 +1289,7 @@ impl Config { } pub fn flycheck(&self) -> FlycheckConfig { + let target_dir = self.target_dir_from_config(); match &self.data.check_overrideCommand { Some(args) if !args.is_empty() => { let mut args = args.clone(); @@ -1309,6 +1310,7 @@ impl Config { } InvocationLocation::Workspace => flycheck::InvocationLocation::Workspace, }, + target_dir, } } Some(_) | None => FlycheckConfig::CargoCommand { @@ -1343,7 +1345,7 @@ impl Config { extra_args: self.check_extra_args(), extra_env: self.check_extra_env(), ansi_color_output: self.color_diagnostic_output(), - target_dir: self.target_dir_from_config(), + target_dir, }, } } From 2290cc3cf393079d30c026249af27838cc2d4595 Mon Sep 17 00:00:00 2001 From: Victor Song Date: Wed, 4 Oct 2023 08:13:01 -0500 Subject: [PATCH 5/7] Pass target directory as flag instead of env variable --- crates/flycheck/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index f0d2f79f6332..c2ad2661eb46 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -360,10 +360,6 @@ impl FlycheckActor { let mut cmd = Command::new(command); cmd.envs(extra_env); - if let Some(target_dir) = target_dir { - cmd.env("CARGO_TARGET_DIR", target_dir); - } - match invocation_location { InvocationLocation::Workspace => { match invocation_strategy { @@ -381,6 +377,10 @@ impl FlycheckActor { } } + if let Some(target_dir) = target_dir { + cmd.arg("--target-dir").arg(target_dir); + } + (cmd, args) } }; From 9771e1e18fafef7571ad46374c5e55d4acd8aa09 Mon Sep 17 00:00:00 2001 From: Victor Song Date: Mon, 9 Oct 2023 02:03:45 -0500 Subject: [PATCH 6/7] Don't pass any target directory options to custom flycheck --- crates/flycheck/src/lib.rs | 6 ------ crates/rust-analyzer/src/config.rs | 4 +--- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index c2ad2661eb46..0749d91eb32a 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -58,7 +58,6 @@ pub enum FlycheckConfig { extra_env: FxHashMap, invocation_strategy: InvocationStrategy, invocation_location: InvocationLocation, - target_dir: Option, }, } @@ -355,7 +354,6 @@ impl FlycheckActor { extra_env, invocation_strategy, invocation_location, - target_dir, } => { let mut cmd = Command::new(command); cmd.envs(extra_env); @@ -377,10 +375,6 @@ impl FlycheckActor { } } - if let Some(target_dir) = target_dir { - cmd.arg("--target-dir").arg(target_dir); - } - (cmd, args) } }; diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 23cf71cff4fa..39a98bcd29a8 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -1289,7 +1289,6 @@ impl Config { } pub fn flycheck(&self) -> FlycheckConfig { - let target_dir = self.target_dir_from_config(); match &self.data.check_overrideCommand { Some(args) if !args.is_empty() => { let mut args = args.clone(); @@ -1310,7 +1309,6 @@ impl Config { } InvocationLocation::Workspace => flycheck::InvocationLocation::Workspace, }, - target_dir, } } Some(_) | None => FlycheckConfig::CargoCommand { @@ -1345,7 +1343,7 @@ impl Config { extra_args: self.check_extra_args(), extra_env: self.check_extra_env(), ansi_color_output: self.color_diagnostic_output(), - target_dir, + target_dir: self.target_dir_from_config(), }, } } From a39d2076db1f39e49a7c7c5365eac8a11abcdc9e Mon Sep 17 00:00:00 2001 From: Victor Song Date: Mon, 9 Oct 2023 02:14:46 -0500 Subject: [PATCH 7/7] Addressed PR style comments --- crates/rust-analyzer/src/config.rs | 21 +++++++++------------ docs/user/generated_config.adoc | 3 ++- editors/code/package.json | 2 +- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 39a98bcd29a8..c8df4255d96b 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -481,7 +481,8 @@ config_data! { runnables_extraArgs: Vec = "[]", /// Optional path to a rust-analyzer specific target directory. - /// This is useful to prevent rust-analyzer's `cargo check` from blocking builds. + /// This prevents rust-analyzer's `cargo check` from locking the `Cargo.lock` + /// at the expense of duplicating build artifacts. /// /// Set to `true` to use a subdirectory of the existing target directory or /// set to a path relative to the workspace to use that path. @@ -1349,17 +1350,13 @@ impl Config { } fn target_dir_from_config(&self) -> Option { - self.data - .rust_analyzerTargetDir - .as_ref() - .map(|target_dir| match target_dir { - TargetDirectory::UseSubdirectory(yes) if *yes => { - Some(PathBuf::from("target/rust-analyzer")) - } - TargetDirectory::UseSubdirectory(_) => None, - TargetDirectory::Directory(dir) => Some(dir.clone()), - }) - .flatten() + self.data.rust_analyzerTargetDir.as_ref().and_then(|target_dir| match target_dir { + TargetDirectory::UseSubdirectory(yes) if *yes => { + Some(PathBuf::from("target/rust-analyzer")) + } + TargetDirectory::UseSubdirectory(_) => None, + TargetDirectory::Directory(dir) => Some(dir.clone()), + }) } pub fn check_on_save(&self) -> bool { diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index 4440caaed89c..7c76ae81bea0 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -761,7 +761,8 @@ tests or binaries. For example, it may be `--release`. + -- Optional path to a rust-analyzer specific target directory. -This is useful to prevent rust-analyzer's `cargo check` from blocking builds. +This prevents rust-analyzer's `cargo check` from locking the `Cargo.lock` +at the expense of duplicating build artifacts. Set to `true` to use a subdirectory of the existing target directory or set to a path relative to the workspace to use that path. diff --git a/editors/code/package.json b/editors/code/package.json index a18acb98301d..fc6597a0d402 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -1489,7 +1489,7 @@ } }, "rust-analyzer.rust.analyzerTargetDir": { - "markdownDescription": "Optional path to a rust-analyzer specific target directory.\nThis is useful to prevent rust-analyzer's `cargo check` from blocking builds.\n\nSet to `true` to use a subdirectory of the existing target directory or\nset to a path relative to the workspace to use that path.", + "markdownDescription": "Optional path to a rust-analyzer specific target directory.\nThis prevents rust-analyzer's `cargo check` from locking the `Cargo.lock`\nat the expense of duplicating build artifacts.\n\nSet to `true` to use a subdirectory of the existing target directory or\nset to a path relative to the workspace to use that path.", "default": null, "anyOf": [ {