Auto merge of #13466 - Veykril:invocation-location, r=Veykril

Implement invocation location config

This allows setting the working directory for build-scripts on flycheck
Complements rust-lang/rust-analyzer#13128

This will be followed up by one more PR that adds a few simple interpolation vars for `overrideCommand`, with that we should cover the needs for most build systems I believe.

Author: bors

crates/flycheck/src/lib.rs

@@ -28,6 +28,13 @@ pub enum InvocationStrategy {
     PerWorkspace,
 }
 
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
+pub enum InvocationLocation {
+    Root(AbsPathBuf),
+    #[default]
+    Workspace,
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum FlycheckConfig {
     CargoCommand {
@@ -39,13 +46,13 @@ pub enum FlycheckConfig {
         features: Vec<String>,
         extra_args: Vec<String>,
         extra_env: FxHashMap<String, String>,
-        invocation_strategy: InvocationStrategy,
     },
     CustomCommand {
         command: String,
         args: Vec<String>,
         extra_env: FxHashMap<String, String>,
         invocation_strategy: InvocationStrategy,
+        invocation_location: InvocationLocation,
     },
 }
 
@@ -275,7 +282,7 @@ impl FlycheckActor {
     }
 
     fn check_command(&self) -> Command {
-        let (mut cmd, args, invocation_strategy) = match &self.config {
+        let (mut cmd, args) = match &self.config {
             FlycheckConfig::CargoCommand {
                 command,
                 target_triple,
@@ -285,7 +292,6 @@ impl FlycheckActor {
                 extra_args,
                 features,
                 extra_env,
-                invocation_strategy,
             } => {
                 let mut cmd = Command::new(toolchain::cargo());
                 cmd.arg(command);
@@ -309,18 +315,40 @@ impl FlycheckActor {
                     }
                 }
                 cmd.envs(extra_env);
-                (cmd, extra_args, invocation_strategy)
+                (cmd, extra_args)
             }
-            FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy } => {
+            FlycheckConfig::CustomCommand {
+                command,
+                args,
+                extra_env,
+                invocation_strategy,
+                invocation_location,
+            } => {
                 let mut cmd = Command::new(command);
                 cmd.envs(extra_env);
-                (cmd, args, invocation_strategy)
+
+                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);
+                            }
+                        }
+                    }
+                    InvocationLocation::Root(root) => {
+                        cmd.current_dir(root);
+                    }
+                }
+
+                (cmd, args)
             }
         };
-        match invocation_strategy {
-            InvocationStrategy::PerWorkspace => cmd.current_dir(&self.root),
-            InvocationStrategy::Once => cmd.args(args),
-        };
+
+        cmd.args(args);
         cmd
     }
 

crates/project-model/src/build_scripts.rs

@@ -21,7 +21,8 @@ use semver::Version;
 use serde::Deserialize;
 
 use crate::{
-    cfg_flag::CfgFlag, CargoConfig, CargoFeatures, CargoWorkspace, InvocationStrategy, Package,
+    cfg_flag::CfgFlag, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
+    InvocationStrategy, Package,
 };
 
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
@@ -55,10 +56,7 @@ impl BuildScriptOutput {
 }
 
 impl WorkspaceBuildScripts {
-    fn build_command(
-        config: &CargoConfig,
-        workspace_root: Option<&path::Path>,
-    ) -> io::Result<Command> {
+    fn build_command(config: &CargoConfig, current_dir: &path::Path) -> io::Result<Command> {
         let mut cmd = match config.run_build_script_command.as_deref() {
             Some([program, args @ ..]) => {
                 let mut cmd = Command::new(program);
@@ -94,14 +92,11 @@ impl WorkspaceBuildScripts {
                     }
                 }
 
-                if let Some(workspace_root) = workspace_root {
-                    cmd.current_dir(workspace_root);
-                }
-
                 cmd
             }
         };
 
+        cmd.current_dir(current_dir);
         cmd.envs(&config.extra_env);
         if config.wrap_rustc_in_build_scripts {
             // Setup RUSTC_WRAPPER to point to `rust-analyzer` binary itself. We use
@@ -124,19 +119,21 @@ impl WorkspaceBuildScripts {
     ) -> io::Result<WorkspaceBuildScripts> {
         const RUST_1_62: Version = Version::new(1, 62, 0);
 
-        let workspace_root: &path::Path = &workspace.workspace_root().as_ref();
+        let current_dir = match &config.invocation_location {
+            InvocationLocation::Root(root) if config.run_build_script_command.is_some() => {
+                root.as_path()
+            }
+            _ => &workspace.workspace_root(),
+        }
+        .as_ref();
 
-        match Self::run_per_ws(
-            Self::build_command(config, Some(workspace_root))?,
-            workspace,
-            progress,
-        ) {
+        match Self::run_per_ws(Self::build_command(config, current_dir)?, workspace, progress) {
             Ok(WorkspaceBuildScripts { error: Some(error), .. })
                 if toolchain.as_ref().map_or(false, |it| *it >= RUST_1_62) =>
             {
                 // building build scripts failed, attempt to build with --keep-going so
                 // that we potentially get more build data
-                let mut cmd = Self::build_command(config, Some(workspace_root))?;
+                let mut cmd = Self::build_command(config, current_dir)?;
                 cmd.args(&["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1");
                 let mut res = Self::run_per_ws(cmd, workspace, progress)?;
                 res.error = Some(error);
@@ -154,7 +151,17 @@ impl WorkspaceBuildScripts {
         progress: &dyn Fn(String),
     ) -> io::Result<Vec<WorkspaceBuildScripts>> {
         assert_eq!(config.invocation_strategy, InvocationStrategy::Once);
-        let cmd = Self::build_command(config, None)?;
+
+        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 cmd = Self::build_command(config, current_dir.as_path().as_ref())?;
         // NB: Cargo.toml could have been modified between `cargo metadata` and
         // `cargo check`. We shouldn't assume that package ids we see here are
         // exactly those from `config`.

crates/project-model/src/cargo_workspace.rs

@@ -14,7 +14,7 @@ use rustc_hash::FxHashMap;
 use serde::Deserialize;
 use serde_json::from_value;
 
-use crate::{utf8_stdout, ManifestPath};
+use crate::{utf8_stdout, InvocationLocation, ManifestPath};
 use crate::{CfgOverrides, InvocationStrategy};
 
 /// [`CargoWorkspace`] represents the logical structure of, well, a Cargo
@@ -107,6 +107,7 @@ 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,
 }
 
 impl CargoConfig {

crates/project-model/src/lib.rs

@@ -164,3 +164,10 @@ pub enum InvocationStrategy {
     #[default]
     PerWorkspace,
 }
+
+#[derive(Clone, Debug, Default, PartialEq, Eq)]
+pub enum InvocationLocation {
+    Root(AbsPathBuf),
+    #[default]
+    Workspace,
+}

crates/project-model/src/workspace.rs

@@ -332,7 +332,9 @@ impl ProjectWorkspace {
         config: &CargoConfig,
         progress: &dyn Fn(String),
     ) -> Vec<Result<WorkspaceBuildScripts>> {
-        if let InvocationStrategy::PerWorkspace = config.invocation_strategy {
+        if matches!(config.invocation_strategy, InvocationStrategy::PerWorkspace)
+            || config.run_build_script_command.is_some()
+        {
             return workspaces.iter().map(|it| it.run_build_scripts(config, progress)).collect();
         }
 

crates/rust-analyzer/src/config.rs

@@ -69,10 +69,16 @@ config_data! {
         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 = "\"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 from the
-        /// corresponding workspace root.
-        /// If `once` is set, the command will be executed once in the project root.
+        /// If `per_workspace` is set, the command will be executed for each workspace.
+        /// If `once` is set, the command will be executed once.
         /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
         /// is set.
         cargo_buildScripts_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
@@ -129,10 +135,17 @@ config_data! {
         ///
         /// Set to `"all"` to pass `--all-features` to Cargo.
         checkOnSave_features: Option<CargoFeaturesDef>      = "null",
+        /// 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.cargo.checkOnSave.invocationStrategy#` is set to `once`.
+        /// - "root": run checks in the project's root directory.
+        /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
+        /// is set.
+        checkOnSave_invocationLocation: InvocationLocation = "\"workspace\"",
         /// Specifies the invocation strategy to use when running the checkOnSave command.
-        /// If `per_workspace` is set, the command will be executed for each workspace from the
-        /// corresponding workspace root.
-        /// If `once` is set, the command will be executed once in the project root.
+        /// If `per_workspace` is set, the command will be executed for each workspace.
+        /// If `once` is set, the command will be executed once.
         /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
         /// is set.
         checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
@@ -1074,6 +1087,12 @@ impl Config {
                 InvocationStrategy::Once => project_model::InvocationStrategy::Once,
                 InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace,
             },
+            invocation_location: match self.data.cargo_buildScripts_invocationLocation {
+                InvocationLocation::Root => {
+                    project_model::InvocationLocation::Root(self.root_path.clone())
+                }
+                InvocationLocation::Workspace => project_model::InvocationLocation::Workspace,
+            },
             run_build_script_command: self.data.cargo_buildScripts_overrideCommand.clone(),
             extra_env: self.data.cargo_extraEnv.clone(),
         }
@@ -1097,10 +1116,6 @@ impl Config {
         if !self.data.checkOnSave_enable {
             return None;
         }
-        let invocation_strategy = match self.data.checkOnSave_invocationStrategy {
-            InvocationStrategy::Once => flycheck::InvocationStrategy::Once,
-            InvocationStrategy::PerWorkspace => flycheck::InvocationStrategy::PerWorkspace,
-        };
         let flycheck_config = match &self.data.checkOnSave_overrideCommand {
             Some(args) if !args.is_empty() => {
                 let mut args = args.clone();
@@ -1109,7 +1124,18 @@ impl Config {
                     command,
                     args,
                     extra_env: self.check_on_save_extra_env(),
-                    invocation_strategy,
+                    invocation_strategy: match self.data.checkOnSave_invocationStrategy {
+                        InvocationStrategy::Once => flycheck::InvocationStrategy::Once,
+                        InvocationStrategy::PerWorkspace => {
+                            flycheck::InvocationStrategy::PerWorkspace
+                        }
+                    },
+                    invocation_location: match self.data.checkOnSave_invocationLocation {
+                        InvocationLocation::Root => {
+                            flycheck::InvocationLocation::Root(self.root_path.clone())
+                        }
+                        InvocationLocation::Workspace => flycheck::InvocationLocation::Workspace,
+                    },
                 }
             }
             Some(_) | None => FlycheckConfig::CargoCommand {
@@ -1139,7 +1165,6 @@ impl Config {
                 },
                 extra_args: self.data.checkOnSave_extraArgs.clone(),
                 extra_env: self.check_on_save_extra_env(),
-                invocation_strategy,
             },
         };
         Some(flycheck_config)
@@ -1618,6 +1643,13 @@ enum InvocationStrategy {
     PerWorkspace,
 }
 
+#[derive(Deserialize, Debug, Clone)]
+#[serde(rename_all = "snake_case")]
+enum InvocationLocation {
+    Root,
+    Workspace,
+}
+
 #[derive(Deserialize, Debug, Clone)]
 #[serde(untagged)]
 enum LifetimeElisionDef {
@@ -2036,8 +2068,16 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
             "type": "string",
             "enum": ["per_workspace", "once"],
             "enumDescriptions": [
-                "The command will be executed for each workspace from the corresponding workspace root.",
-                "The command will be executed once in the project root."
+                "The command will be executed for each workspace.",
+                "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."
             ],
         },
         _ => panic!("missing entry for {}: {}", ty, default),

crates/rust-analyzer/src/reload.rs

@@ -473,8 +473,10 @@ impl GlobalState {
         };
 
         let sender = self.flycheck_sender.clone();
-        let (FlycheckConfig::CargoCommand { invocation_strategy, .. }
-        | FlycheckConfig::CustomCommand { invocation_strategy, .. }) = config;
+        let invocation_strategy = match config {
+            FlycheckConfig::CargoCommand { .. } => flycheck::InvocationStrategy::PerWorkspace,
+            FlycheckConfig::CustomCommand { invocation_strategy, .. } => invocation_strategy,
+        };
 
         self.flycheck = match invocation_strategy {
             flycheck::InvocationStrategy::Once => vec![FlycheckHandle::spawn(