Skip to content

Commit 7db5029

Browse files
VeykrilVeykril
committed
{manifest-path} interpolation
1 parent 5174b65 commit 7db5029

File tree

8 files changed

+118
-103
lines changed

8 files changed

+118
-103
lines changed

‎crates/flycheck/src/lib.rs

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
use std::{
88
fmt, io,
9-
path::Path,
109
process::{ChildStderr, ChildStdout, Command, Stdio},
1110
time::Duration,
1211
};
@@ -25,7 +24,6 @@ pub use cargo_metadata::diagnostic::{
2524
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
2625
pub enum InvocationStrategy {
2726
OnceInRoot,
28-
PerWorkspaceWithManifestPath,
2927
#[default]
3028
PerWorkspace,
3129
}
@@ -153,7 +151,9 @@ struct FlycheckActor {
153151
id: usize,
154152
sender: Box<dyn Fn(Message) + Send>,
155153
config: FlycheckConfig,
156-
workspace_root: AbsPathBuf,
154+
/// Either the workspace root of the workspace we are flychecking,
155+
/// or the project root of the project.
156+
root: AbsPathBuf,
157157
/// CargoHandle exists to wrap around the communication needed to be able to
158158
/// run `cargo check` without blocking. Currently the Rust standard library
159159
/// doesn't provide a way to read sub-process output without blocking, so we
@@ -175,7 +175,7 @@ impl FlycheckActor {
175175
workspace_root: AbsPathBuf,
176176
) -> FlycheckActor {
177177
tracing::info!(%id, ?workspace_root, "Spawning flycheck");
178-
FlycheckActor { id, sender, config, workspace_root, cargo_handle: None }
178+
FlycheckActor { id, sender, config, root: workspace_root, cargo_handle: None }
179179
}
180180

181181
fn report_progress(&self, progress: Progress) {
@@ -210,20 +210,7 @@ impl FlycheckActor {
210210
}
211211
}
212212

213-
let mut command = self.check_command();
214-
let invocation_strategy = self.invocation_strategy();
215-
match invocation_strategy {
216-
InvocationStrategy::OnceInRoot => (),
217-
InvocationStrategy::PerWorkspaceWithManifestPath => {
218-
command.arg("--manifest-path");
219-
command.arg(<_ as AsRef<Path>>::as_ref(
220-
&self.workspace_root.join("Cargo.toml"),
221-
));
222-
}
223-
InvocationStrategy::PerWorkspace => {
224-
command.current_dir(&self.workspace_root);
225-
}
226-
}
213+
let command = self.check_command();
227214
tracing::debug!(?command, "will restart flycheck");
228215
match CargoHandle::spawn(command) {
229216
Ok(cargo_handle) => {
@@ -265,7 +252,7 @@ impl FlycheckActor {
265252
CargoMessage::Diagnostic(msg) => {
266253
self.send(Message::AddDiagnostic {
267254
id: self.id,
268-
workspace_root: self.workspace_root.clone(),
255+
workspace_root: self.root.clone(),
269256
diagnostic: msg,
270257
});
271258
}
@@ -287,15 +274,8 @@ impl FlycheckActor {
287274
}
288275
}
289276

290-
fn invocation_strategy(&self) -> InvocationStrategy {
291-
match self.config {
292-
FlycheckConfig::CargoCommand { invocation_strategy, .. }
293-
| FlycheckConfig::CustomCommand { invocation_strategy, .. } => invocation_strategy,
294-
}
295-
}
296-
297277
fn check_command(&self) -> Command {
298-
let mut cmd = match &self.config {
278+
let (mut cmd, args, invocation_strategy) = match &self.config {
299279
FlycheckConfig::CargoCommand {
300280
command,
301281
target_triple,
@@ -305,13 +285,11 @@ impl FlycheckActor {
305285
extra_args,
306286
features,
307287
extra_env,
308-
invocation_strategy: _,
288+
invocation_strategy,
309289
} => {
310290
let mut cmd = Command::new(toolchain::cargo());
311291
cmd.arg(command);
312-
cmd.current_dir(&self.workspace_root);
313-
cmd.args(&["--workspace", "--message-format=json", "--manifest-path"])
314-
.arg(self.workspace_root.join("Cargo.toml").as_os_str());
292+
cmd.args(&["--workspace", "--message-format=json"]);
315293

316294
if let Some(target) = target_triple {
317295
cmd.args(&["--target", target.as_str()]);
@@ -330,18 +308,35 @@ impl FlycheckActor {
330308
cmd.arg(features.join(" "));
331309
}
332310
}
333-
cmd.args(extra_args);
334311
cmd.envs(extra_env);
335-
cmd
312+
(cmd, extra_args, invocation_strategy)
336313
}
337-
FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy: _ } => {
314+
FlycheckConfig::CustomCommand { command, args, extra_env, invocation_strategy } => {
338315
let mut cmd = Command::new(command);
339-
cmd.args(args);
340316
cmd.envs(extra_env);
341-
cmd
317+
(cmd, args, invocation_strategy)
342318
}
343319
};
344-
cmd.current_dir(&self.workspace_root);
320+
if let InvocationStrategy::PerWorkspace = invocation_strategy {
321+
let mut with_manifest_path = false;
322+
for arg in args {
323+
if let Some(_) = arg.find("$manifest_path") {
324+
with_manifest_path = true;
325+
cmd.arg(arg.replace(
326+
"$manifest_path",
327+
&self.root.join("Cargo.toml").display().to_string(),
328+
));
329+
} else {
330+
cmd.arg(arg);
331+
}
332+
}
333+
334+
if !with_manifest_path {
335+
cmd.current_dir(&self.root);
336+
}
337+
} else {
338+
cmd.args(args);
339+
}
345340
cmd
346341
}
347342

‎crates/project-model/src/build_scripts.rs

Lines changed: 45 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,37 @@ impl BuildScriptOutput {
5555
}
5656

5757
impl WorkspaceBuildScripts {
58-
fn build_command(config: &CargoConfig) -> io::Result<Command> {
58+
fn build_command(
59+
config: &CargoConfig,
60+
workspace_root: Option<&path::Path>,
61+
) -> io::Result<Command> {
5962
let mut cmd = match config.run_build_script_command.as_deref() {
6063
Some([program, args @ ..]) => {
6164
let mut cmd = Command::new(program);
62-
cmd.args(args);
65+
66+
// FIXME: strategy and workspace root are coupled, express that in code
67+
if let (InvocationStrategy::PerWorkspace, Some(workspace_root)) =
68+
(config.invocation_strategy, workspace_root)
69+
{
70+
let mut with_manifest_path = false;
71+
for arg in args {
72+
if let Some(_) = arg.find("$manifest_path") {
73+
with_manifest_path = true;
74+
cmd.arg(arg.replace(
75+
"$manifest_path",
76+
&workspace_root.join("Cargo.toml").display().to_string(),
77+
));
78+
} else {
79+
cmd.arg(arg);
80+
}
81+
}
82+
83+
if !with_manifest_path {
84+
cmd.current_dir(workspace_root);
85+
}
86+
} else {
87+
cmd.args(args);
88+
}
6389
cmd
6490
}
6591
_ => {
@@ -90,9 +116,15 @@ impl WorkspaceBuildScripts {
90116
}
91117
}
92118
}
119+
120+
if let Some(workspace_root) = workspace_root {
121+
cmd.current_dir(workspace_root);
122+
}
123+
93124
cmd
94125
}
95126
};
127+
96128
cmd.envs(&config.extra_env);
97129
if config.wrap_rustc_in_build_scripts {
98130
// Setup RUSTC_WRAPPER to point to `rust-analyzer` binary itself. We use
@@ -115,15 +147,21 @@ impl WorkspaceBuildScripts {
115147
) -> io::Result<WorkspaceBuildScripts> {
116148
const RUST_1_62: Version = Version::new(1, 62, 0);
117149

118-
match Self::run_per_ws(Self::build_command(config)?, config, workspace, progress) {
150+
let workspace_root: &path::Path = &workspace.workspace_root().as_ref();
151+
152+
match Self::run_per_ws(
153+
Self::build_command(config, Some(workspace_root))?,
154+
workspace,
155+
progress,
156+
) {
119157
Ok(WorkspaceBuildScripts { error: Some(error), .. })
120158
if toolchain.as_ref().map_or(false, |it| *it >= RUST_1_62) =>
121159
{
122160
// building build scripts failed, attempt to build with --keep-going so
123161
// that we potentially get more build data
124-
let mut cmd = Self::build_command(config)?;
162+
let mut cmd = Self::build_command(config, Some(workspace_root))?;
125163
cmd.args(&["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1");
126-
let mut res = Self::run_per_ws(cmd, config, workspace, progress)?;
164+
let mut res = Self::run_per_ws(cmd, workspace, progress)?;
127165
res.error = Some(error);
128166
Ok(res)
129167
}
@@ -139,7 +177,7 @@ impl WorkspaceBuildScripts {
139177
progress: &dyn Fn(String),
140178
) -> io::Result<Vec<WorkspaceBuildScripts>> {
141179
assert_eq!(config.invocation_strategy, InvocationStrategy::OnceInRoot);
142-
let cmd = Self::build_command(config)?;
180+
let cmd = Self::build_command(config, None)?;
143181
// NB: Cargo.toml could have been modified between `cargo metadata` and
144182
// `cargo check`. We shouldn't assume that package ids we see here are
145183
// exactly those from `config`.
@@ -187,24 +225,10 @@ impl WorkspaceBuildScripts {
187225
}
188226

189227
fn run_per_ws(
190-
mut cmd: Command,
191-
config: &CargoConfig,
228+
cmd: Command,
192229
workspace: &CargoWorkspace,
193230
progress: &dyn Fn(String),
194231
) -> io::Result<WorkspaceBuildScripts> {
195-
let workspace_root: &path::Path = &workspace.workspace_root().as_ref();
196-
197-
match config.invocation_strategy {
198-
InvocationStrategy::OnceInRoot => (),
199-
InvocationStrategy::PerWorkspaceWithManifestPath => {
200-
cmd.arg("--manifest-path");
201-
cmd.arg(workspace_root.join("Cargo.toml"));
202-
}
203-
InvocationStrategy::PerWorkspace => {
204-
cmd.current_dir(workspace_root);
205-
}
206-
}
207-
208232
let mut res = WorkspaceBuildScripts::default();
209233
let outputs = &mut res.outputs;
210234
// NB: Cargo.toml could have been modified between `cargo metadata` and

‎crates/project-model/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,9 @@ fn utf8_stdout(mut cmd: Command) -> Result<String> {
158158
Ok(stdout.trim().to_string())
159159
}
160160

161-
#[derive(Clone, Debug, Default, PartialEq, Eq)]
161+
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
162162
pub enum InvocationStrategy {
163163
OnceInRoot,
164-
PerWorkspaceWithManifestPath,
165164
#[default]
166165
PerWorkspace,
167166
}

‎crates/project-model/src/workspace.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,7 @@ impl ProjectWorkspace {
323323
config: &CargoConfig,
324324
progress: &dyn Fn(String),
325325
) -> Vec<Result<WorkspaceBuildScripts>> {
326-
if let InvocationStrategy::PerWorkspaceWithManifestPath | InvocationStrategy::PerWorkspace =
327-
config.invocation_strategy
328-
{
326+
if let InvocationStrategy::PerWorkspace = config.invocation_strategy {
329327
return workspaces.iter().map(|it| it.run_build_scripts(config, progress)).collect();
330328
}
331329

‎crates/rust-analyzer/src/config.rs

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,15 @@ config_data! {
7070
/// Run build scripts (`build.rs`) for more precise code analysis.
7171
cargo_buildScripts_enable: bool = "true",
7272
/// Specifies the invocation strategy to use when running the build scripts command.
73-
/// If `per_workspace_with_manifest_path` is set, the command will be executed for each
74-
/// workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and
75-
/// the command will be executed from the project root.
76-
/// If `per_workspace` is set, the command will be executed for each workspace and the
77-
/// command will be executed from the corresponding workspace root.
73+
/// If `per_workspace` is set, the command will be executed for each workspace and all
74+
/// occurrences of `$manifest_path` in the command will be replaced by the corresponding
75+
/// manifest path of the workspace that the command is being invoked for. If interpolation
76+
/// for the manifest path happens at least once, the commands will be executed from the
77+
/// project root, otherwise the commands will be executed from the corresponding workspace
78+
/// root.
7879
/// If `once_in_root` is set, the command will be executed once in the project root.
80+
/// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
81+
/// is set.
7982
cargo_buildScripts_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
8083
/// Override the command rust-analyzer uses to run build scripts and
8184
/// build procedural macros. The command is required to output json
@@ -131,12 +134,15 @@ config_data! {
131134
/// Set to `"all"` to pass `--all-features` to Cargo.
132135
checkOnSave_features: Option<CargoFeaturesDef> = "null",
133136
/// Specifies the invocation strategy to use when running the checkOnSave command.
134-
/// If `per_workspace_with_manifest_path` is set, the command will be executed for each
135-
/// workspace, `--manifest-path {workspace-dir}` will be passed to the invoked command and
136-
/// the command will be executed from the project root.
137-
/// If `per_workspace` is set, the command will be executed for each workspace and the
138-
/// command will be executed from the corresponding workspace root.
137+
/// If `per_workspace` is set, the command will be executed for each workspace and all
138+
/// occurrences of `$manifest_path` in the command will be replaced by the corresponding
139+
/// manifest path of the workspace that the command is being invoked for. If interpolation
140+
/// for the manifest path happens at least once, the commands will be executed from the
141+
/// project root, otherwise the commands will be executed from the corresponding workspace
142+
/// root.
139143
/// If `once_in_root` is set, the command will be executed once in the project root.
144+
/// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
145+
/// is set.
140146
checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
141147
/// Whether to pass `--no-default-features` to Cargo. Defaults to
142148
/// `#rust-analyzer.cargo.noDefaultFeatures#`.
@@ -1074,9 +1080,6 @@ impl Config {
10741080
wrap_rustc_in_build_scripts: self.data.cargo_buildScripts_useRustcWrapper,
10751081
invocation_strategy: match self.data.cargo_buildScripts_invocationStrategy {
10761082
InvocationStrategy::OnceInRoot => project_model::InvocationStrategy::OnceInRoot,
1077-
InvocationStrategy::PerWorkspaceWithManifestPath => {
1078-
project_model::InvocationStrategy::PerWorkspaceWithManifestPath
1079-
}
10801083
InvocationStrategy::PerWorkspace => project_model::InvocationStrategy::PerWorkspace,
10811084
},
10821085
run_build_script_command: self.data.cargo_buildScripts_overrideCommand.clone(),
@@ -1104,9 +1107,6 @@ impl Config {
11041107
}
11051108
let invocation_strategy = match self.data.checkOnSave_invocationStrategy {
11061109
InvocationStrategy::OnceInRoot => flycheck::InvocationStrategy::OnceInRoot,
1107-
InvocationStrategy::PerWorkspaceWithManifestPath => {
1108-
flycheck::InvocationStrategy::PerWorkspaceWithManifestPath
1109-
}
11101110
InvocationStrategy::PerWorkspace => flycheck::InvocationStrategy::PerWorkspace,
11111111
};
11121112
let flycheck_config = match &self.data.checkOnSave_overrideCommand {
@@ -1623,7 +1623,6 @@ enum CargoFeaturesDef {
16231623
#[serde(rename_all = "snake_case")]
16241624
enum InvocationStrategy {
16251625
OnceInRoot,
1626-
PerWorkspaceWithManifestPath,
16271626
PerWorkspace,
16281627
}
16291628

@@ -2043,10 +2042,9 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
20432042
},
20442043
"InvocationStrategy" => set! {
20452044
"type": "string",
2046-
"enum": ["per_workspace", "per_workspace_with_manifest_path", "once_in_root"],
2045+
"enum": ["per_workspace", "once_in_root"],
20472046
"enumDescriptions": [
2048-
"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.",
2049-
"The command will be executed for each workspace and the command will be executed from the corresponding workspace root.",
2047+
"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.",
20502048
"The command will be executed once in the project root."
20512049
],
20522050
},

‎crates/rust-analyzer/src/reload.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,7 @@ impl GlobalState {
483483
config.clone(),
484484
self.config.root_path().clone(),
485485
)],
486-
flycheck::InvocationStrategy::PerWorkspaceWithManifestPath
487-
| flycheck::InvocationStrategy::PerWorkspace => {
486+
flycheck::InvocationStrategy::PerWorkspace => {
488487
self.workspaces
489488
.iter()
490489
.enumerate()

0 commit comments

Comments
 (0)