Auto merge of #17888 - Tyrubias:remove-invocation-location, r=Veykril

chore(config): remove `invocationLocation` in favor of `invocationStrategy`

These flags were added to help rust-analyzer integrate with repos requiring non-Cargo invocations. The consensus is that having two independent settings are no longer needed. This change removes `invocationLocation` in favor of `invocationStrategy` and changes the internal representation of `InvocationStrategy::Once` to hold the workspace root.

Closes #17848.
This commit is contained in:
bors 2024-08-19 08:19:38 +00:00
commit c1879398af
8 changed files with 33 additions and 173 deletions

View file

@ -19,8 +19,8 @@ use serde::Deserialize;
use toolchain::Tool; use toolchain::Tool;
use crate::{ use crate::{
utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation, utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationStrategy, ManifestPath,
InvocationStrategy, ManifestPath, Package, Sysroot, TargetKind, Package, Sysroot, TargetKind,
}; };
/// Output of the build script and proc-macro building steps for a workspace. /// Output of the build script and proc-macro building steps for a workspace.
@ -63,10 +63,7 @@ impl WorkspaceBuildScripts {
progress: &dyn Fn(String), progress: &dyn Fn(String),
sysroot: &Sysroot, sysroot: &Sysroot,
) -> io::Result<WorkspaceBuildScripts> { ) -> io::Result<WorkspaceBuildScripts> {
let current_dir = match &config.invocation_location { let current_dir = workspace.workspace_root();
InvocationLocation::Root(root) if config.run_build_script_command.is_some() => root,
_ => workspace.workspace_root(),
};
let allowed_features = workspace.workspace_features(); let allowed_features = workspace.workspace_features();
let cmd = Self::build_command( let cmd = Self::build_command(
@ -89,15 +86,7 @@ impl WorkspaceBuildScripts {
) -> io::Result<Vec<WorkspaceBuildScripts>> { ) -> io::Result<Vec<WorkspaceBuildScripts>> {
assert_eq!(config.invocation_strategy, InvocationStrategy::Once); assert_eq!(config.invocation_strategy, InvocationStrategy::Once);
let current_dir = match &config.invocation_location { let current_dir = workspace_root;
InvocationLocation::Root(root) => root,
InvocationLocation::Workspace => {
return Err(io::Error::new(
io::ErrorKind::Other,
"Cannot run build scripts from workspace with invocation strategy `once`",
))
}
};
let cmd = Self::build_command( let cmd = Self::build_command(
config, config,
&Default::default(), &Default::default(),

View file

@ -13,7 +13,7 @@ use serde_json::from_value;
use span::Edition; use span::Edition;
use toolchain::Tool; use toolchain::Tool;
use crate::{utf8_stdout, InvocationLocation, ManifestPath, Sysroot}; use crate::{utf8_stdout, ManifestPath, Sysroot};
use crate::{CfgOverrides, InvocationStrategy}; use crate::{CfgOverrides, InvocationStrategy};
/// [`CargoWorkspace`] represents the logical structure of, well, a Cargo /// [`CargoWorkspace`] represents the logical structure of, well, a Cargo
@ -100,7 +100,6 @@ pub struct CargoConfig {
/// Extra env vars to set when invoking the cargo command /// Extra env vars to set when invoking the cargo command
pub extra_env: FxHashMap<String, String>, pub extra_env: FxHashMap<String, String>,
pub invocation_strategy: InvocationStrategy, pub invocation_strategy: InvocationStrategy,
pub invocation_location: InvocationLocation,
/// Optional path to use instead of `target` when building /// Optional path to use instead of `target` when building
pub target_dir: Option<Utf8PathBuf>, pub target_dir: Option<Utf8PathBuf>,
} }

View file

@ -186,20 +186,13 @@ fn utf8_stdout(mut cmd: Command) -> anyhow::Result<String> {
Ok(stdout.trim().to_owned()) Ok(stdout.trim().to_owned())
} }
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] #[derive(Clone, Debug, Default, PartialEq, Eq)]
pub enum InvocationStrategy { pub enum InvocationStrategy {
Once, Once,
#[default] #[default]
PerWorkspace, PerWorkspace,
} }
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub enum InvocationLocation {
Root(AbsPathBuf),
#[default]
Workspace,
}
/// A set of cfg-overrides per crate. /// A set of cfg-overrides per crate.
#[derive(Default, Debug, Clone, Eq, PartialEq)] #[derive(Default, Debug, Clone, Eq, PartialEq)]
pub struct CfgOverrides { pub struct CfgOverrides {

View file

@ -80,13 +80,6 @@ config_data! {
pub(crate) cargo_autoreload: bool = true, pub(crate) cargo_autoreload: bool = true,
/// Run build scripts (`build.rs`) for more precise code analysis. /// Run build scripts (`build.rs`) for more precise code analysis.
cargo_buildScripts_enable: bool = true, cargo_buildScripts_enable: bool = true,
/// Specifies the working directory for running build scripts.
/// - "workspace": run build scripts for a workspace in the workspace's root directory.
/// This is incompatible with `#rust-analyzer.cargo.buildScripts.invocationStrategy#` set to `once`.
/// - "root": run build scripts in the project's root directory.
/// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
/// is set.
cargo_buildScripts_invocationLocation: InvocationLocation = InvocationLocation::Workspace,
/// Specifies the invocation strategy to use when running the build scripts command. /// Specifies the invocation strategy to use when running the build scripts command.
/// If `per_workspace` is set, the command will be executed for each workspace. /// If `per_workspace` is set, the command will be executed for each workspace.
/// If `once` is set, the command will be executed once. /// If `once` is set, the command will be executed once.
@ -101,8 +94,7 @@ config_data! {
/// If there are multiple linked projects/workspaces, this command is invoked for /// If there are multiple linked projects/workspaces, this command is invoked for
/// each of them, with the working directory being the workspace root /// each of them, with the working directory being the workspace root
/// (i.e., the folder containing the `Cargo.toml`). This can be overwritten /// (i.e., the folder containing the `Cargo.toml`). This can be overwritten
/// by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and /// by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#`.
/// `#rust-analyzer.cargo.buildScripts.invocationLocation#`.
/// ///
/// By default, a cargo invocation will be constructed for the configured /// By default, a cargo invocation will be constructed for the configured
/// targets and features, with the following base command line: /// targets and features, with the following base command line:
@ -182,14 +174,6 @@ config_data! {
/// ///
/// For example for `cargo check`: `dead_code`, `unused_imports`, `unused_variables`,... /// For example for `cargo check`: `dead_code`, `unused_imports`, `unused_variables`,...
check_ignore: FxHashSet<String> = FxHashSet::default(), check_ignore: FxHashSet<String> = FxHashSet::default(),
/// Specifies the working directory for running checks.
/// - "workspace": run checks for workspaces in the corresponding workspaces' root directories.
// FIXME: Ideally we would support this in some way
/// This falls back to "root" if `#rust-analyzer.check.invocationStrategy#` is set to `once`.
/// - "root": run checks in the project's root directory.
/// This config only has an effect when `#rust-analyzer.check.overrideCommand#`
/// is set.
check_invocationLocation | checkOnSave_invocationLocation: InvocationLocation = InvocationLocation::Workspace,
/// Specifies the invocation strategy to use when running the check command. /// Specifies the invocation strategy to use when running the check command.
/// If `per_workspace` is set, the command will be executed for each workspace. /// If `per_workspace` is set, the command will be executed for each workspace.
/// If `once` is set, the command will be executed once. /// If `once` is set, the command will be executed once.
@ -212,8 +196,7 @@ config_data! {
/// If there are multiple linked projects/workspaces, this command is invoked for /// If there are multiple linked projects/workspaces, this command is invoked for
/// each of them, with the working directory being the workspace root /// each of them, with the working directory being the workspace root
/// (i.e., the folder containing the `Cargo.toml`). This can be overwritten /// (i.e., the folder containing the `Cargo.toml`). This can be overwritten
/// by changing `#rust-analyzer.check.invocationStrategy#` and /// by changing `#rust-analyzer.check.invocationStrategy#`.
/// `#rust-analyzer.check.invocationLocation#`.
/// ///
/// If `$saved_file` is part of the command, rust-analyzer will pass /// If `$saved_file` is part of the command, rust-analyzer will pass
/// the absolute path of the saved file to the provided command. This is /// the absolute path of the saved file to the provided command. This is
@ -1868,12 +1851,6 @@ impl Config {
InvocationStrategy::Once => project_model::InvocationStrategy::Once, InvocationStrategy::Once => project_model::InvocationStrategy::Once,
InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace, InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace,
}, },
invocation_location: match self.cargo_buildScripts_invocationLocation(None) {
InvocationLocation::Root => {
project_model::InvocationLocation::Root(self.root_path.clone())
}
InvocationLocation::Workspace => project_model::InvocationLocation::Workspace,
},
run_build_script_command: self.cargo_buildScripts_overrideCommand(None).clone(), run_build_script_command: self.cargo_buildScripts_overrideCommand(None).clone(),
extra_args: self.cargo_extraArgs(None).clone(), extra_args: self.cargo_extraArgs(None).clone(),
extra_env: self.cargo_extraEnv(None).clone(), extra_env: self.cargo_extraEnv(None).clone(),
@ -1930,14 +1907,6 @@ impl Config {
crate::flycheck::InvocationStrategy::PerWorkspace crate::flycheck::InvocationStrategy::PerWorkspace
} }
}, },
invocation_location: match self.check_invocationLocation(None) {
InvocationLocation::Root => {
crate::flycheck::InvocationLocation::Root(self.root_path.clone())
}
InvocationLocation::Workspace => {
crate::flycheck::InvocationLocation::Workspace
}
},
} }
} }
Some(_) | None => FlycheckConfig::CargoCommand { Some(_) | None => FlycheckConfig::CargoCommand {
@ -2348,13 +2317,6 @@ pub(crate) enum InvocationStrategy {
#[derive(Serialize, Deserialize, Debug, Clone)] #[derive(Serialize, Deserialize, Debug, Clone)]
struct CheckOnSaveTargets(#[serde(with = "single_or_array")] Vec<String>); struct CheckOnSaveTargets(#[serde(with = "single_or_array")] Vec<String>);
#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "snake_case")]
enum InvocationLocation {
Root,
Workspace,
}
#[derive(Serialize, Deserialize, Debug, Clone)] #[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "snake_case")] #[serde(rename_all = "snake_case")]
enum LifetimeElisionDef { enum LifetimeElisionDef {
@ -3196,14 +3158,6 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
"The command will be executed once." "The command will be executed once."
], ],
}, },
"InvocationLocation" => set! {
"type": "string",
"enum": ["workspace", "root"],
"enumDescriptions": [
"The command will be executed in the corresponding workspace root.",
"The command will be executed in the project root."
],
},
"Option<CheckOnSaveTargets>" => set! { "Option<CheckOnSaveTargets>" => set! {
"anyOf": [ "anyOf": [
{ {

View file

@ -15,20 +15,13 @@ use toolchain::Tool;
use crate::command::{CommandHandle, ParseFromLine}; use crate::command::{CommandHandle, ParseFromLine};
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] #[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) enum InvocationStrategy { pub(crate) enum InvocationStrategy {
Once, Once,
#[default] #[default]
PerWorkspace, PerWorkspace,
} }
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) enum InvocationLocation {
Root(AbsPathBuf),
#[default]
Workspace,
}
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct CargoOptions { pub(crate) struct CargoOptions {
pub(crate) target_triples: Vec<String>, pub(crate) target_triples: Vec<String>,
@ -79,7 +72,6 @@ pub(crate) enum FlycheckConfig {
args: Vec<String>, args: Vec<String>,
extra_env: FxHashMap<String, String>, extra_env: FxHashMap<String, String>,
invocation_strategy: InvocationStrategy, invocation_strategy: InvocationStrategy,
invocation_location: InvocationLocation,
}, },
} }
@ -424,30 +416,17 @@ impl FlycheckActor {
cmd.args(&options.extra_args); cmd.args(&options.extra_args);
Some(cmd) Some(cmd)
} }
FlycheckConfig::CustomCommand { FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy } => {
command,
args,
extra_env,
invocation_strategy,
invocation_location,
} => {
let mut cmd = Command::new(command); let mut cmd = Command::new(command);
cmd.envs(extra_env); cmd.envs(extra_env);
match invocation_location { match invocation_strategy {
InvocationLocation::Workspace => { InvocationStrategy::Once => {
match invocation_strategy { cmd.current_dir(&self.root);
InvocationStrategy::Once => {
cmd.current_dir(&self.root);
}
InvocationStrategy::PerWorkspace => {
// FIXME: cmd.current_dir(&affected_workspace);
cmd.current_dir(&self.root);
}
}
} }
InvocationLocation::Root(root) => { InvocationStrategy::PerWorkspace => {
cmd.current_dir(root); // FIXME: cmd.current_dir(&affected_workspace);
cmd.current_dir(&self.root);
} }
} }

View file

@ -764,18 +764,22 @@ impl GlobalState {
FlycheckConfig::CargoCommand { .. } => { FlycheckConfig::CargoCommand { .. } => {
crate::flycheck::InvocationStrategy::PerWorkspace crate::flycheck::InvocationStrategy::PerWorkspace
} }
FlycheckConfig::CustomCommand { invocation_strategy, .. } => invocation_strategy, FlycheckConfig::CustomCommand { ref invocation_strategy, .. } => {
invocation_strategy.clone()
}
}; };
self.flycheck = match invocation_strategy { self.flycheck = match invocation_strategy {
crate::flycheck::InvocationStrategy::Once => vec![FlycheckHandle::spawn( crate::flycheck::InvocationStrategy::Once => {
0, vec![FlycheckHandle::spawn(
sender, 0,
config, sender,
None, config,
self.config.root_path().clone(), None,
None, self.config.root_path().clone(),
)], None,
)]
}
crate::flycheck::InvocationStrategy::PerWorkspace => { crate::flycheck::InvocationStrategy::PerWorkspace => {
self.workspaces self.workspaces
.iter() .iter()

View file

@ -45,16 +45,6 @@ Automatically refresh project info via `cargo metadata` on
-- --
Run build scripts (`build.rs`) for more precise code analysis. Run build scripts (`build.rs`) for more precise code analysis.
-- --
[[rust-analyzer.cargo.buildScripts.invocationLocation]]rust-analyzer.cargo.buildScripts.invocationLocation (default: `"workspace"`)::
+
--
Specifies the working directory for running build scripts.
- "workspace": run build scripts for a workspace in the workspace's root directory.
This is incompatible with `#rust-analyzer.cargo.buildScripts.invocationStrategy#` set to `once`.
- "root": run build scripts in the project's root directory.
This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
is set.
--
[[rust-analyzer.cargo.buildScripts.invocationStrategy]]rust-analyzer.cargo.buildScripts.invocationStrategy (default: `"per_workspace"`):: [[rust-analyzer.cargo.buildScripts.invocationStrategy]]rust-analyzer.cargo.buildScripts.invocationStrategy (default: `"per_workspace"`)::
+ +
-- --
@ -75,8 +65,7 @@ option.
If there are multiple linked projects/workspaces, this command is invoked for If there are multiple linked projects/workspaces, this command is invoked for
each of them, with the working directory being the workspace root each of them, with the working directory being the workspace root
(i.e., the folder containing the `Cargo.toml`). This can be overwritten (i.e., the folder containing the `Cargo.toml`). This can be overwritten
by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#`.
`#rust-analyzer.cargo.buildScripts.invocationLocation#`.
By default, a cargo invocation will be constructed for the configured By default, a cargo invocation will be constructed for the configured
targets and features, with the following base command line: targets and features, with the following base command line:
@ -209,16 +198,6 @@ List of `cargo check` (or other command specified in `check.command`) diagnostic
For example for `cargo check`: `dead_code`, `unused_imports`, `unused_variables`,... For example for `cargo check`: `dead_code`, `unused_imports`, `unused_variables`,...
-- --
[[rust-analyzer.check.invocationLocation]]rust-analyzer.check.invocationLocation (default: `"workspace"`)::
+
--
Specifies the working directory for running checks.
- "workspace": run checks for workspaces in the corresponding workspaces' root directories.
This falls back to "root" if `#rust-analyzer.check.invocationStrategy#` is set to `once`.
- "root": run checks in the project's root directory.
This config only has an effect when `#rust-analyzer.check.overrideCommand#`
is set.
--
[[rust-analyzer.check.invocationStrategy]]rust-analyzer.check.invocationStrategy (default: `"per_workspace"`):: [[rust-analyzer.check.invocationStrategy]]rust-analyzer.check.invocationStrategy (default: `"per_workspace"`)::
+ +
-- --
@ -250,8 +229,7 @@ Cargo, you might also want to change
If there are multiple linked projects/workspaces, this command is invoked for If there are multiple linked projects/workspaces, this command is invoked for
each of them, with the working directory being the workspace root each of them, with the working directory being the workspace root
(i.e., the folder containing the `Cargo.toml`). This can be overwritten (i.e., the folder containing the `Cargo.toml`). This can be overwritten
by changing `#rust-analyzer.check.invocationStrategy#` and by changing `#rust-analyzer.check.invocationStrategy#`.
`#rust-analyzer.check.invocationLocation#`.
If `$saved_file` is part of the command, rust-analyzer will pass If `$saved_file` is part of the command, rust-analyzer will pass
the absolute path of the saved file to the provided command. This is the absolute path of the saved file to the provided command. This is

View file

@ -663,24 +663,6 @@
} }
} }
}, },
{
"title": "cargo",
"properties": {
"rust-analyzer.cargo.buildScripts.invocationLocation": {
"markdownDescription": "Specifies the working directory for running build scripts.\n- \"workspace\": run build scripts for a workspace in the workspace's root directory.\n This is incompatible with `#rust-analyzer.cargo.buildScripts.invocationStrategy#` set to `once`.\n- \"root\": run build scripts in the project's root directory.\nThis config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`\nis set.",
"default": "workspace",
"type": "string",
"enum": [
"workspace",
"root"
],
"enumDescriptions": [
"The command will be executed in the corresponding workspace root.",
"The command will be executed in the project root."
]
}
}
},
{ {
"title": "cargo", "title": "cargo",
"properties": { "properties": {
@ -703,7 +685,7 @@
"title": "cargo", "title": "cargo",
"properties": { "properties": {
"rust-analyzer.cargo.buildScripts.overrideCommand": { "rust-analyzer.cargo.buildScripts.overrideCommand": {
"markdownDescription": "Override the command rust-analyzer uses to run build scripts and\nbuild procedural macros. The command is required to output json\nand should therefore include `--message-format=json` or a similar\noption.\n\nIf there are multiple linked projects/workspaces, this command is invoked for\neach of them, with the working directory being the workspace root\n(i.e., the folder containing the `Cargo.toml`). This can be overwritten\nby changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and\n`#rust-analyzer.cargo.buildScripts.invocationLocation#`.\n\nBy default, a cargo invocation will be constructed for the configured\ntargets and features, with the following base command line:\n\n```bash\ncargo check --quiet --workspace --message-format=json --all-targets --keep-going\n```\n.", "markdownDescription": "Override the command rust-analyzer uses to run build scripts and\nbuild procedural macros. The command is required to output json\nand should therefore include `--message-format=json` or a similar\noption.\n\nIf there are multiple linked projects/workspaces, this command is invoked for\neach of them, with the working directory being the workspace root\n(i.e., the folder containing the `Cargo.toml`). This can be overwritten\nby changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#`.\n\nBy default, a cargo invocation will be constructed for the configured\ntargets and features, with the following base command line:\n\n```bash\ncargo check --quiet --workspace --message-format=json --all-targets --keep-going\n```\n.",
"default": null, "default": null,
"type": [ "type": [
"null", "null",
@ -965,24 +947,6 @@
} }
} }
}, },
{
"title": "check",
"properties": {
"rust-analyzer.check.invocationLocation": {
"markdownDescription": "Specifies the working directory for running checks.\n- \"workspace\": run checks for workspaces in the corresponding workspaces' root directories.\n This falls back to \"root\" if `#rust-analyzer.check.invocationStrategy#` is set to `once`.\n- \"root\": run checks in the project's root directory.\nThis config only has an effect when `#rust-analyzer.check.overrideCommand#`\nis set.",
"default": "workspace",
"type": "string",
"enum": [
"workspace",
"root"
],
"enumDescriptions": [
"The command will be executed in the corresponding workspace root.",
"The command will be executed in the project root."
]
}
}
},
{ {
"title": "check", "title": "check",
"properties": { "properties": {
@ -1018,7 +982,7 @@
"title": "check", "title": "check",
"properties": { "properties": {
"rust-analyzer.check.overrideCommand": { "rust-analyzer.check.overrideCommand": {
"markdownDescription": "Override the command rust-analyzer uses instead of `cargo check` for\ndiagnostics on save. The command is required to output json and\nshould therefore include `--message-format=json` or a similar option\n(if your client supports the `colorDiagnosticOutput` experimental\ncapability, you can use `--message-format=json-diagnostic-rendered-ansi`).\n\nIf you're changing this because you're using some tool wrapping\nCargo, you might also want to change\n`#rust-analyzer.cargo.buildScripts.overrideCommand#`.\n\nIf there are multiple linked projects/workspaces, this command is invoked for\neach of them, with the working directory being the workspace root\n(i.e., the folder containing the `Cargo.toml`). This can be overwritten\nby changing `#rust-analyzer.check.invocationStrategy#` and\n`#rust-analyzer.check.invocationLocation#`.\n\nIf `$saved_file` is part of the command, rust-analyzer will pass\nthe absolute path of the saved file to the provided command. This is\nintended to be used with non-Cargo build systems.\nNote that `$saved_file` is experimental and may be removed in the future.\n\nAn example command would be:\n\n```bash\ncargo check --workspace --message-format=json --all-targets\n```\n.", "markdownDescription": "Override the command rust-analyzer uses instead of `cargo check` for\ndiagnostics on save. The command is required to output json and\nshould therefore include `--message-format=json` or a similar option\n(if your client supports the `colorDiagnosticOutput` experimental\ncapability, you can use `--message-format=json-diagnostic-rendered-ansi`).\n\nIf you're changing this because you're using some tool wrapping\nCargo, you might also want to change\n`#rust-analyzer.cargo.buildScripts.overrideCommand#`.\n\nIf there are multiple linked projects/workspaces, this command is invoked for\neach of them, with the working directory being the workspace root\n(i.e., the folder containing the `Cargo.toml`). This can be overwritten\nby changing `#rust-analyzer.check.invocationStrategy#`.\n\nIf `$saved_file` is part of the command, rust-analyzer will pass\nthe absolute path of the saved file to the provided command. This is\nintended to be used with non-Cargo build systems.\nNote that `$saved_file` is experimental and may be removed in the future.\n\nAn example command would be:\n\n```bash\ncargo check --workspace --message-format=json --all-targets\n```\n.",
"default": null, "default": null,
"type": [ "type": [
"null", "null",