From 7db50294a3e458b7dd00ee646eee75a7ec933e3b Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 26 Sep 2022 15:58:55 +0200 Subject: [PATCH] {manifest-path} interpolation --- crates/flycheck/src/lib.rs | 69 +++++++++++------------ crates/project-model/src/build_scripts.rs | 66 +++++++++++++++------- crates/project-model/src/lib.rs | 3 +- crates/project-model/src/workspace.rs | 4 +- crates/rust-analyzer/src/config.rs | 38 ++++++------- crates/rust-analyzer/src/reload.rs | 3 +- docs/user/generated_config.adoc | 26 +++++---- editors/code/package.json | 12 ++-- 8 files changed, 118 insertions(+), 103 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index c3976e6b7a8..cdb3c2969c8 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -6,7 +6,6 @@ use std::{ fmt, io, - path::Path, process::{ChildStderr, ChildStdout, Command, Stdio}, time::Duration, }; @@ -25,7 +24,6 @@ pub use cargo_metadata::diagnostic::{ #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] pub enum InvocationStrategy { OnceInRoot, - PerWorkspaceWithManifestPath, #[default] PerWorkspace, } @@ -153,7 +151,9 @@ struct FlycheckActor { id: usize, sender: Box, config: FlycheckConfig, - workspace_root: AbsPathBuf, + /// Either the workspace root of the workspace we are flychecking, + /// or the project root of the project. + root: AbsPathBuf, /// CargoHandle exists to wrap around the communication needed to be able to /// run `cargo check` without blocking. Currently the Rust standard library /// doesn't provide a way to read sub-process output without blocking, so we @@ -175,7 +175,7 @@ impl FlycheckActor { workspace_root: AbsPathBuf, ) -> FlycheckActor { tracing::info!(%id, ?workspace_root, "Spawning flycheck"); - FlycheckActor { id, sender, config, workspace_root, cargo_handle: None } + FlycheckActor { id, sender, config, root: workspace_root, cargo_handle: None } } fn report_progress(&self, progress: Progress) { @@ -210,20 +210,7 @@ impl FlycheckActor { } } - let mut command = self.check_command(); - let invocation_strategy = self.invocation_strategy(); - match invocation_strategy { - InvocationStrategy::OnceInRoot => (), - InvocationStrategy::PerWorkspaceWithManifestPath => { - command.arg("--manifest-path"); - command.arg(<_ as AsRef>::as_ref( - &self.workspace_root.join("Cargo.toml"), - )); - } - InvocationStrategy::PerWorkspace => { - command.current_dir(&self.workspace_root); - } - } + let command = self.check_command(); tracing::debug!(?command, "will restart flycheck"); match CargoHandle::spawn(command) { Ok(cargo_handle) => { @@ -265,7 +252,7 @@ impl FlycheckActor { CargoMessage::Diagnostic(msg) => { self.send(Message::AddDiagnostic { id: self.id, - workspace_root: self.workspace_root.clone(), + workspace_root: self.root.clone(), diagnostic: msg, }); } @@ -287,15 +274,8 @@ impl FlycheckActor { } } - fn invocation_strategy(&self) -> InvocationStrategy { - match self.config { - FlycheckConfig::CargoCommand { invocation_strategy, .. } - | FlycheckConfig::CustomCommand { invocation_strategy, .. } => invocation_strategy, - } - } - fn check_command(&self) -> Command { - let mut cmd = match &self.config { + let (mut cmd, args, invocation_strategy) = match &self.config { FlycheckConfig::CargoCommand { command, target_triple, @@ -305,13 +285,11 @@ impl FlycheckActor { extra_args, features, extra_env, - invocation_strategy: _, + invocation_strategy, } => { let mut cmd = Command::new(toolchain::cargo()); cmd.arg(command); - cmd.current_dir(&self.workspace_root); - cmd.args(&["--workspace", "--message-format=json", "--manifest-path"]) - .arg(self.workspace_root.join("Cargo.toml").as_os_str()); + cmd.args(&["--workspace", "--message-format=json"]); if let Some(target) = target_triple { cmd.args(&["--target", target.as_str()]); @@ -330,18 +308,35 @@ impl FlycheckActor { cmd.arg(features.join(" ")); } } - cmd.args(extra_args); cmd.envs(extra_env); - cmd + (cmd, extra_args, invocation_strategy) } - FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy: _ } => { + FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy } => { let mut cmd = Command::new(command); - cmd.args(args); cmd.envs(extra_env); - cmd + (cmd, args, invocation_strategy) } }; - cmd.current_dir(&self.workspace_root); + if let InvocationStrategy::PerWorkspace = invocation_strategy { + let mut with_manifest_path = false; + for arg in args { + if let Some(_) = arg.find("$manifest_path") { + with_manifest_path = true; + cmd.arg(arg.replace( + "$manifest_path", + &self.root.join("Cargo.toml").display().to_string(), + )); + } else { + cmd.arg(arg); + } + } + + if !with_manifest_path { + cmd.current_dir(&self.root); + } + } else { + cmd.args(args); + } cmd } diff --git a/crates/project-model/src/build_scripts.rs b/crates/project-model/src/build_scripts.rs index 6e6654e74e1..12be67d9a0c 100644 --- a/crates/project-model/src/build_scripts.rs +++ b/crates/project-model/src/build_scripts.rs @@ -55,11 +55,37 @@ impl BuildScriptOutput { } impl WorkspaceBuildScripts { - fn build_command(config: &CargoConfig) -> io::Result { + fn build_command( + config: &CargoConfig, + workspace_root: Option<&path::Path>, + ) -> io::Result { let mut cmd = match config.run_build_script_command.as_deref() { Some([program, args @ ..]) => { let mut cmd = Command::new(program); - cmd.args(args); + + // FIXME: strategy and workspace root are coupled, express that in code + if let (InvocationStrategy::PerWorkspace, Some(workspace_root)) = + (config.invocation_strategy, workspace_root) + { + let mut with_manifest_path = false; + for arg in args { + if let Some(_) = arg.find("$manifest_path") { + with_manifest_path = true; + cmd.arg(arg.replace( + "$manifest_path", + &workspace_root.join("Cargo.toml").display().to_string(), + )); + } else { + cmd.arg(arg); + } + } + + if !with_manifest_path { + cmd.current_dir(workspace_root); + } + } else { + cmd.args(args); + } cmd } _ => { @@ -90,9 +116,15 @@ impl WorkspaceBuildScripts { } } } + + if let Some(workspace_root) = workspace_root { + cmd.current_dir(workspace_root); + } + cmd } }; + cmd.envs(&config.extra_env); if config.wrap_rustc_in_build_scripts { // Setup RUSTC_WRAPPER to point to `rust-analyzer` binary itself. We use @@ -115,15 +147,21 @@ impl WorkspaceBuildScripts { ) -> io::Result { const RUST_1_62: Version = Version::new(1, 62, 0); - match Self::run_per_ws(Self::build_command(config)?, config, workspace, progress) { + let workspace_root: &path::Path = &workspace.workspace_root().as_ref(); + + match Self::run_per_ws( + Self::build_command(config, Some(workspace_root))?, + 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)?; + let mut cmd = Self::build_command(config, Some(workspace_root))?; cmd.args(&["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1"); - let mut res = Self::run_per_ws(cmd, config, workspace, progress)?; + let mut res = Self::run_per_ws(cmd, workspace, progress)?; res.error = Some(error); Ok(res) } @@ -139,7 +177,7 @@ impl WorkspaceBuildScripts { progress: &dyn Fn(String), ) -> io::Result> { assert_eq!(config.invocation_strategy, InvocationStrategy::OnceInRoot); - let cmd = Self::build_command(config)?; + let cmd = Self::build_command(config, None)?; // 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`. @@ -187,24 +225,10 @@ impl WorkspaceBuildScripts { } fn run_per_ws( - mut cmd: Command, - config: &CargoConfig, + cmd: Command, workspace: &CargoWorkspace, progress: &dyn Fn(String), ) -> io::Result { - let workspace_root: &path::Path = &workspace.workspace_root().as_ref(); - - match config.invocation_strategy { - InvocationStrategy::OnceInRoot => (), - InvocationStrategy::PerWorkspaceWithManifestPath => { - cmd.arg("--manifest-path"); - cmd.arg(workspace_root.join("Cargo.toml")); - } - InvocationStrategy::PerWorkspace => { - cmd.current_dir(workspace_root); - } - } - let mut res = WorkspaceBuildScripts::default(); let outputs = &mut res.outputs; // NB: Cargo.toml could have been modified between `cargo metadata` and diff --git a/crates/project-model/src/lib.rs b/crates/project-model/src/lib.rs index 13a86901f71..956c872ccc6 100644 --- a/crates/project-model/src/lib.rs +++ b/crates/project-model/src/lib.rs @@ -158,10 +158,9 @@ fn utf8_stdout(mut cmd: Command) -> Result { Ok(stdout.trim().to_string()) } -#[derive(Clone, Debug, Default, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] pub enum InvocationStrategy { OnceInRoot, - PerWorkspaceWithManifestPath, #[default] PerWorkspace, } diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs index 01f5157093a..561bad3a6ab 100644 --- a/crates/project-model/src/workspace.rs +++ b/crates/project-model/src/workspace.rs @@ -323,9 +323,7 @@ impl ProjectWorkspace { config: &CargoConfig, progress: &dyn Fn(String), ) -> Vec> { - if let InvocationStrategy::PerWorkspaceWithManifestPath | InvocationStrategy::PerWorkspace = - config.invocation_strategy - { + if let InvocationStrategy::PerWorkspace = config.invocation_strategy { return workspaces.iter().map(|it| it.run_build_scripts(config, progress)).collect(); } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 9f022d6add1..3bf28cb7b47 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/crates/rust-analyzer/src/config.rs @@ -70,12 +70,15 @@ config_data! { /// Run build scripts (`build.rs`) for more precise code analysis. cargo_buildScripts_enable: bool = "true", /// Specifies the invocation strategy to use when running the build scripts command. - /// If `per_workspace_with_manifest_path` is set, the command will be executed for each - /// workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and - /// the command will be executed from the project root. - /// If `per_workspace` is set, the command will be executed for each workspace and the - /// command will be executed from the corresponding workspace root. + /// If `per_workspace` is set, the command will be executed for each workspace and all + /// occurrences of `$manifest_path` in the command will be replaced by the corresponding + /// manifest path of the workspace that the command is being invoked for. If interpolation + /// for the manifest path happens at least once, the commands will be executed from the + /// project root, otherwise the commands will be executed from the corresponding workspace + /// root. /// If `once_in_root` is set, the command will be executed once in the project root. + /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#` + /// is set. cargo_buildScripts_invocationStrategy: InvocationStrategy = "\"per_workspace\"", /// Override the command rust-analyzer uses to run build scripts and /// build procedural macros. The command is required to output json @@ -131,12 +134,15 @@ config_data! { /// Set to `"all"` to pass `--all-features` to Cargo. checkOnSave_features: Option = "null", /// Specifies the invocation strategy to use when running the checkOnSave command. - /// If `per_workspace_with_manifest_path` is set, the command will be executed for each - /// workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and - /// the command will be executed from the project root. - /// If `per_workspace` is set, the command will be executed for each workspace and the - /// command will be executed from the corresponding workspace root. + /// If `per_workspace` is set, the command will be executed for each workspace and all + /// occurrences of `$manifest_path` in the command will be replaced by the corresponding + /// manifest path of the workspace that the command is being invoked for. If interpolation + /// for the manifest path happens at least once, the commands will be executed from the + /// project root, otherwise the commands will be executed from the corresponding workspace + /// root. /// If `once_in_root` is set, the command will be executed once in the project root. + /// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#` + /// is set. checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"", /// Whether to pass `--no-default-features` to Cargo. Defaults to /// `#rust-analyzer.cargo.noDefaultFeatures#`. @@ -1074,9 +1080,6 @@ impl Config { wrap_rustc_in_build_scripts: self.data.cargo_buildScripts_useRustcWrapper, invocation_strategy: match self.data.cargo_buildScripts_invocationStrategy { InvocationStrategy::OnceInRoot => project_model::InvocationStrategy::OnceInRoot, - InvocationStrategy::PerWorkspaceWithManifestPath => { - project_model::InvocationStrategy::PerWorkspaceWithManifestPath - } InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace, }, run_build_script_command: self.data.cargo_buildScripts_overrideCommand.clone(), @@ -1104,9 +1107,6 @@ impl Config { } let invocation_strategy = match self.data.checkOnSave_invocationStrategy { InvocationStrategy::OnceInRoot => flycheck::InvocationStrategy::OnceInRoot, - InvocationStrategy::PerWorkspaceWithManifestPath => { - flycheck::InvocationStrategy::PerWorkspaceWithManifestPath - } InvocationStrategy::PerWorkspace => flycheck::InvocationStrategy::PerWorkspace, }; let flycheck_config = match &self.data.checkOnSave_overrideCommand { @@ -1623,7 +1623,6 @@ enum CargoFeaturesDef { #[serde(rename_all = "snake_case")] enum InvocationStrategy { OnceInRoot, - PerWorkspaceWithManifestPath, PerWorkspace, } @@ -2043,10 +2042,9 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json }, "InvocationStrategy" => set! { "type": "string", - "enum": ["per_workspace", "per_workspace_with_manifest_path", "once_in_root"], + "enum": ["per_workspace", "once_in_root"], "enumDescriptions": [ - "The command will be executed for each workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and the command will be executed from the project root.", - "The command will be executed for each workspace and the command will be executed from the corresponding workspace root.", + "The command will be executed for each workspace and `{manifest-path}` usages will be interpolated with the corresponding workspace manifests. If `{manifest-path}` is used, the commands will be executed in the project root, otherwise in the corresponding workspace roots.", "The command will be executed once in the project root." ], }, diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 5382790f6e7..af7a51a68ef 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -483,8 +483,7 @@ impl GlobalState { config.clone(), self.config.root_path().clone(), )], - flycheck::InvocationStrategy::PerWorkspaceWithManifestPath - | flycheck::InvocationStrategy::PerWorkspace => { + flycheck::InvocationStrategy::PerWorkspace => { self.workspaces .iter() .enumerate() diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc index 3ced42ef72e..3948d8f7e72 100644 --- a/docs/user/generated_config.adoc +++ b/docs/user/generated_config.adoc @@ -28,12 +28,15 @@ Run build scripts (`build.rs`) for more precise code analysis. + -- Specifies the invocation strategy to use when running the build scripts command. -If `per_workspace_with_manifest_path` is set, the command will be executed for each -workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and -the command will be executed from the project root. -If `per_workspace` is set, the command will be executed for each workspace and the -command will be executed from the corresponding workspace root. +If `per_workspace` is set, the command will be executed for each workspace and all +occurrences of `$manifest_path` in the command will be replaced by the corresponding +manifest path of the workspace that the command is being invoked for. If interpolation +for the manifest path happens at least once, the commands will be executed from the +project root, otherwise the commands will be executed from the corresponding workspace +root. If `once_in_root` is set, the command will be executed once in the project root. +This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#` +is set. -- [[rust-analyzer.cargo.buildScripts.overrideCommand]]rust-analyzer.cargo.buildScripts.overrideCommand (default: `null`):: + @@ -133,12 +136,15 @@ Set to `"all"` to pass `--all-features` to Cargo. + -- Specifies the invocation strategy to use when running the checkOnSave command. -If `per_workspace_with_manifest_path` is set, the command will be executed for each -workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and -the command will be executed from the project root. -If `per_workspace` is set, the command will be executed for each workspace and the -command will be executed from the corresponding workspace root. +If `per_workspace` is set, the command will be executed for each workspace and all +occurrences of `$manifest_path` in the command will be replaced by the corresponding +manifest path of the workspace that the command is being invoked for. If interpolation +for the manifest path happens at least once, the commands will be executed from the +project root, otherwise the commands will be executed from the corresponding workspace +root. If `once_in_root` is set, the command will be executed once in the project root. +This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#` +is set. -- [[rust-analyzer.checkOnSave.noDefaultFeatures]]rust-analyzer.checkOnSave.noDefaultFeatures (default: `null`):: + diff --git a/editors/code/package.json b/editors/code/package.json index 3af32685fda..8c40a3878f5 100644 --- a/editors/code/package.json +++ b/editors/code/package.json @@ -422,17 +422,15 @@ "type": "boolean" }, "rust-analyzer.cargo.buildScripts.invocationStrategy": { - "markdownDescription": "Specifies the invocation strategy to use when running the build scripts command.\nIf `per_workspace_with_manifest_path` is set, the command will be executed for each\nworkspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and\nthe command will be executed from the project root.\nIf `per_workspace` is set, the command will be executed for each workspace and the\ncommand will be executed from the corresponding workspace root.\nIf `once_in_root` is set, the command will be executed once in the project root.", + "markdownDescription": "Specifies the invocation strategy to use when running the build scripts command.\nIf `per_workspace` is set, the command will be executed for each workspace and all\noccurrences of `$manifest_path` in the command will be replaced by the corresponding\nmanifest path of the workspace that the command is being invoked for. If interpolation\nfor the manifest path happens at least once, the commands will be executed from the\nproject root, otherwise the commands will be executed from the corresponding workspace\nroot.\nIf `once_in_root` is set, the command will be executed once in the project root.\nThis config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`\nis set.", "default": "per_workspace", "type": "string", "enum": [ "per_workspace", - "per_workspace_with_manifest_path", "once_in_root" ], "enumDescriptions": [ - "The command will be executed for each workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and the command will be executed from the project root.", - "The command will be executed for each workspace and the command will be executed from the corresponding workspace root.", + "The command will be executed for each workspace and `{manifest-path}` usages will be interpolated with the corresponding workspace manifests. If `{manifest-path}` is used, the commands will be executed in the project root, otherwise in the corresponding workspace roots.", "The command will be executed once in the project root." ] }, @@ -562,17 +560,15 @@ ] }, "rust-analyzer.checkOnSave.invocationStrategy": { - "markdownDescription": "Specifies the invocation strategy to use when running the checkOnSave command.\nIf `per_workspace_with_manifest_path` is set, the command will be executed for each\nworkspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and\nthe command will be executed from the project root.\nIf `per_workspace` is set, the command will be executed for each workspace and the\ncommand will be executed from the corresponding workspace root.\nIf `once_in_root` is set, the command will be executed once in the project root.", + "markdownDescription": "Specifies the invocation strategy to use when running the checkOnSave command.\nIf `per_workspace` is set, the command will be executed for each workspace and all\noccurrences of `$manifest_path` in the command will be replaced by the corresponding\nmanifest path of the workspace that the command is being invoked for. If interpolation\nfor the manifest path happens at least once, the commands will be executed from the\nproject root, otherwise the commands will be executed from the corresponding workspace\nroot.\nIf `once_in_root` is set, the command will be executed once in the project root.\nThis config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`\nis set.", "default": "per_workspace", "type": "string", "enum": [ "per_workspace", - "per_workspace_with_manifest_path", "once_in_root" ], "enumDescriptions": [ - "The command will be executed for each workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and the command will be executed from the project root.", - "The command will be executed for each workspace and the command will be executed from the corresponding workspace root.", + "The command will be executed for each workspace and `{manifest-path}` usages will be interpolated with the corresponding workspace manifests. If `{manifest-path}` is used, the commands will be executed in the project root, otherwise in the corresponding workspace roots.", "The command will be executed once in the project root." ] },