Skip to content

Commit

Permalink
Auto merge of rust-lang#17888 - Tyrubias:remove-invocation-location, …
Browse files Browse the repository at this point in the history
…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 rust-lang#17848.
  • Loading branch information
bors committed Aug 19, 2024
2 parents 2c18533 + 1bfb362 commit 85f6d15
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 173 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use serde::Deserialize;
use toolchain::Tool;

use crate::{
utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
InvocationStrategy, ManifestPath, Package, Sysroot, TargetKind,
utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationStrategy, ManifestPath,
Package, Sysroot, TargetKind,
};

/// Output of the build script and proc-macro building steps for a workspace.
Expand Down Expand Up @@ -63,10 +63,7 @@ impl WorkspaceBuildScripts {
progress: &dyn Fn(String),
sysroot: &Sysroot,
) -> io::Result<WorkspaceBuildScripts> {
let current_dir = match &config.invocation_location {
InvocationLocation::Root(root) if config.run_build_script_command.is_some() => root,
_ => workspace.workspace_root(),
};
let current_dir = workspace.workspace_root();

let allowed_features = workspace.workspace_features();
let cmd = Self::build_command(
Expand All @@ -89,15 +86,7 @@ impl WorkspaceBuildScripts {
) -> io::Result<Vec<WorkspaceBuildScripts>> {
assert_eq!(config.invocation_strategy, InvocationStrategy::Once);

let current_dir = match &config.invocation_location {
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 current_dir = workspace_root;
let cmd = Self::build_command(
config,
&Default::default(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use serde_json::from_value;
use span::Edition;
use toolchain::Tool;

use crate::{utf8_stdout, InvocationLocation, ManifestPath, Sysroot};
use crate::{utf8_stdout, ManifestPath, Sysroot};
use crate::{CfgOverrides, InvocationStrategy};

/// [`CargoWorkspace`] represents the logical structure of, well, a Cargo
Expand Down Expand Up @@ -100,7 +100,6 @@ pub struct CargoConfig {
/// Extra env vars to set when invoking the cargo command
pub extra_env: FxHashMap<String, String>,
pub invocation_strategy: InvocationStrategy,
pub invocation_location: InvocationLocation,
/// Optional path to use instead of `target` when building
pub target_dir: Option<Utf8PathBuf>,
}
Expand Down
9 changes: 1 addition & 8 deletions src/tools/rust-analyzer/crates/project-model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,20 +186,13 @@ fn utf8_stdout(mut cmd: Command) -> anyhow::Result<String> {
Ok(stdout.trim().to_owned())
}

#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub enum InvocationStrategy {
Once,
#[default]
PerWorkspace,
}

#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub enum InvocationLocation {
Root(AbsPathBuf),
#[default]
Workspace,
}

/// A set of cfg-overrides per crate.
#[derive(Default, Debug, Clone, Eq, PartialEq)]
pub struct CfgOverrides {
Expand Down
50 changes: 2 additions & 48 deletions src/tools/rust-analyzer/crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,6 @@ config_data! {
pub(crate) cargo_autoreload: bool = true,
/// Run build scripts (`build.rs`) for more precise code analysis.
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.
/// If `per_workspace` is set, the command will be executed for each workspace.
/// If `once` is set, the command will be executed once.
Expand All @@ -101,8 +94,7 @@ config_data! {
/// If there are multiple linked projects/workspaces, this command is invoked for
/// each of them, with the working directory being the workspace root
/// (i.e., the folder containing the `Cargo.toml`). This can be overwritten
/// by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and
/// `#rust-analyzer.cargo.buildScripts.invocationLocation#`.
/// by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#`.
///
/// By default, a cargo invocation will be constructed for the configured
/// targets and features, with the following base command line:
Expand Down Expand Up @@ -182,14 +174,6 @@ config_data! {
///
/// For example for `cargo check`: `dead_code`, `unused_imports`, `unused_variables`,...
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.
/// If `per_workspace` is set, the command will be executed for each workspace.
/// If `once` is set, the command will be executed once.
Expand All @@ -212,8 +196,7 @@ config_data! {
/// If there are multiple linked projects/workspaces, this command is invoked for
/// each of them, with the working directory being the workspace root
/// (i.e., the folder containing the `Cargo.toml`). This can be overwritten
/// by changing `#rust-analyzer.check.invocationStrategy#` and
/// `#rust-analyzer.check.invocationLocation#`.
/// by changing `#rust-analyzer.check.invocationStrategy#`.
///
/// 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
Expand Down Expand Up @@ -1868,12 +1851,6 @@ impl Config {
InvocationStrategy::Once => project_model::InvocationStrategy::Once,
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(),
extra_args: self.cargo_extraArgs(None).clone(),
extra_env: self.cargo_extraEnv(None).clone(),
Expand Down Expand Up @@ -1930,14 +1907,6 @@ impl Config {
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 {
Expand Down Expand Up @@ -2348,13 +2317,6 @@ pub(crate) enum InvocationStrategy {
#[derive(Serialize, Deserialize, Debug, Clone)]
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)]
#[serde(rename_all = "snake_case")]
enum LifetimeElisionDef {
Expand Down Expand Up @@ -3196,14 +3158,6 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
"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! {
"anyOf": [
{
Expand Down
37 changes: 8 additions & 29 deletions src/tools/rust-analyzer/crates/rust-analyzer/src/flycheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,13 @@ use toolchain::Tool;

use crate::command::{CommandHandle, ParseFromLine};

#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) enum InvocationStrategy {
Once,
#[default]
PerWorkspace,
}

#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub(crate) enum InvocationLocation {
Root(AbsPathBuf),
#[default]
Workspace,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct CargoOptions {
pub(crate) target_triples: Vec<String>,
Expand Down Expand Up @@ -79,7 +72,6 @@ pub(crate) enum FlycheckConfig {
args: Vec<String>,
extra_env: FxHashMap<String, String>,
invocation_strategy: InvocationStrategy,
invocation_location: InvocationLocation,
},
}

Expand Down Expand Up @@ -424,30 +416,17 @@ impl FlycheckActor {
cmd.args(&options.extra_args);
Some(cmd)
}
FlycheckConfig::CustomCommand {
command,
args,
extra_env,
invocation_strategy,
invocation_location,
} => {
FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy } => {
let mut cmd = Command::new(command);
cmd.envs(extra_env);

match invocation_location {
InvocationLocation::Workspace => {
match invocation_strategy {
InvocationStrategy::Once => {
cmd.current_dir(&self.root);
}
InvocationStrategy::PerWorkspace => {
// FIXME: cmd.current_dir(&affected_workspace);
cmd.current_dir(&self.root);
}
}
match invocation_strategy {
InvocationStrategy::Once => {
cmd.current_dir(&self.root);
}
InvocationLocation::Root(root) => {
cmd.current_dir(root);
InvocationStrategy::PerWorkspace => {
// FIXME: cmd.current_dir(&affected_workspace);
cmd.current_dir(&self.root);
}
}

Expand Down
22 changes: 13 additions & 9 deletions src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,18 +764,22 @@ impl GlobalState {
FlycheckConfig::CargoCommand { .. } => {
crate::flycheck::InvocationStrategy::PerWorkspace
}
FlycheckConfig::CustomCommand { invocation_strategy, .. } => invocation_strategy,
FlycheckConfig::CustomCommand { ref invocation_strategy, .. } => {
invocation_strategy.clone()
}
};

self.flycheck = match invocation_strategy {
crate::flycheck::InvocationStrategy::Once => vec![FlycheckHandle::spawn(
0,
sender,
config,
None,
self.config.root_path().clone(),
None,
)],
crate::flycheck::InvocationStrategy::Once => {
vec![FlycheckHandle::spawn(
0,
sender,
config,
None,
self.config.root_path().clone(),
None,
)]
}
crate::flycheck::InvocationStrategy::PerWorkspace => {
self.workspaces
.iter()
Expand Down
26 changes: 2 additions & 24 deletions src/tools/rust-analyzer/docs/user/generated_config.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,6 @@ Automatically refresh project info via `cargo metadata` on
--
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"`)::
+
--
Expand All @@ -75,8 +65,7 @@ option.
If there are multiple linked projects/workspaces, this command is invoked for
each of them, with the working directory being the workspace root
(i.e., the folder containing the `Cargo.toml`). This can be overwritten
by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#` and
`#rust-analyzer.cargo.buildScripts.invocationLocation#`.
by changing `#rust-analyzer.cargo.buildScripts.invocationStrategy#`.

By default, a cargo invocation will be constructed for the configured
targets and features, with the following base command line:
Expand Down Expand Up @@ -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`,...
--
[[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"`)::
+
--
Expand Down Expand Up @@ -250,8 +229,7 @@ Cargo, you might also want to change
If there are multiple linked projects/workspaces, this command is invoked for
each of them, with the working directory being the workspace root
(i.e., the folder containing the `Cargo.toml`). This can be overwritten
by changing `#rust-analyzer.check.invocationStrategy#` and
`#rust-analyzer.check.invocationLocation#`.
by changing `#rust-analyzer.check.invocationStrategy#`.

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
Expand Down
Loading

0 comments on commit 85f6d15

Please sign in to comment.