Skip to content

Commit

Permalink
Add $manifest_path substitution for cargo override commands
Browse files Browse the repository at this point in the history
  • Loading branch information
Veykril committed Sep 22, 2023
1 parent 59bcbaf commit 32fca82
Show file tree
Hide file tree
Showing 10 changed files with 109 additions and 61 deletions.
70 changes: 39 additions & 31 deletions crates/flycheck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ impl FlycheckHandle {
sender: Box<dyn Fn(Message) + Send>,
config: FlycheckConfig,
workspace_root: AbsPathBuf,
project_file: AbsPathBuf,
) -> FlycheckHandle {
let actor = FlycheckActor::new(id, sender, config, workspace_root);
let actor = FlycheckActor::new(id, sender, config, workspace_root, project_file);
let (sender, receiver) = unbounded::<StateChange>();
let thread = stdx::thread::Builder::new(stdx::thread::ThreadIntent::Worker)
.name("Flycheck".to_owned())
Expand All @@ -100,8 +101,8 @@ impl FlycheckHandle {
}

/// Schedule a re-start of the cargo check worker.
pub fn restart(&self) {
self.sender.send(StateChange::Restart).unwrap();
pub fn restart(&self, saved_file: Option<AbsPathBuf>) {
self.sender.send(StateChange::Restart { saved_file }).unwrap();
}

/// Stop this cargo check worker.
Expand Down Expand Up @@ -152,7 +153,7 @@ pub enum Progress {
}

enum StateChange {
Restart,
Restart { saved_file: Option<AbsPathBuf> },
Cancel,
}

Expand All @@ -165,6 +166,8 @@ struct FlycheckActor {
/// Either the workspace root of the workspace we are flychecking,
/// or the project root of the project.
root: AbsPathBuf,
/// The Cargo.toml or rust-project.json file of the project.
project_file: 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
Expand All @@ -184,9 +187,17 @@ impl FlycheckActor {
sender: Box<dyn Fn(Message) + Send>,
config: FlycheckConfig,
workspace_root: AbsPathBuf,
project_file: AbsPathBuf,
) -> FlycheckActor {
tracing::info!(%id, ?workspace_root, "Spawning flycheck");
FlycheckActor { id, sender, config, root: workspace_root, command_handle: None }
FlycheckActor {
id,
sender,
config,
root: workspace_root,
project_file,
command_handle: None,
}
}

fn report_progress(&self, progress: Progress) {
Expand All @@ -212,7 +223,7 @@ impl FlycheckActor {
tracing::debug!(flycheck_id = self.id, "flycheck cancelled");
self.cancel_check_process();
}
Event::RequestStateChange(StateChange::Restart) => {
Event::RequestStateChange(StateChange::Restart { saved_file }) => {
// Cancel the previously spawned process
self.cancel_check_process();
while let Ok(restart) = inbox.recv_timeout(Duration::from_millis(50)) {
Expand All @@ -222,7 +233,7 @@ impl FlycheckActor {
}
}

let command = self.check_command();
let command = self.check_command(saved_file);
let formatted_command = format!("{:?}", command);

tracing::debug!(?command, "will restart flycheck");
Expand Down Expand Up @@ -296,8 +307,10 @@ impl FlycheckActor {
}
}

fn check_command(&self) -> Command {
let (mut cmd, args) = match &self.config {
fn check_command(&self, _saved_file: Option<AbsPathBuf>) -> Command {
// FIXME: Figure out the story for exposing the saved file to the custom flycheck command
// as it can be absent
match &self.config {
FlycheckConfig::CargoCommand {
command,
target_triples,
Expand All @@ -312,7 +325,7 @@ impl FlycheckActor {
let mut cmd = Command::new(toolchain::cargo());
cmd.arg(command);
cmd.current_dir(&self.root);
cmd.arg("--workspace");
cmd.args(&["--workspace", "--manifest-path"]).arg(self.project_file.as_os_str());

cmd.arg(if *ansi_color_output {
"--message-format=json-diagnostic-rendered-ansi"
Expand Down Expand Up @@ -341,7 +354,8 @@ impl FlycheckActor {
}
}
cmd.envs(extra_env);
(cmd, extra_args)
cmd.args(extra_args);
cmd
}
FlycheckConfig::CustomCommand {
command,
Expand All @@ -352,30 +366,24 @@ impl FlycheckActor {
} => {
let mut cmd = Command::new(command);
cmd.envs(extra_env);
args.iter().for_each(|arg| {
match invocation_strategy {
InvocationStrategy::Once => cmd.arg(arg),
InvocationStrategy::PerWorkspace => cmd.arg(arg.replace(
"$manifest_path",
&self.project_file.as_os_str().to_string_lossy(),
)),
};
});

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);
}
}
InvocationLocation::Workspace => cmd.current_dir(&self.root),
InvocationLocation::Root(root) => cmd.current_dir(root),
};

(cmd, args)
cmd
}
};

cmd.args(args);
cmd
}
}

fn send(&self, check_task: Message) {
Expand Down
3 changes: 3 additions & 0 deletions crates/paths/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ impl AbsPath {
pub fn exists(&self) -> bool {
self.0.exists()
}
pub fn with_extension<S: AsRef<OsStr>>(&self, extension: S) -> AbsPathBuf {
AbsPathBuf::try_from(self.0.with_extension(extension)).unwrap()
}
// endregion:delegate-methods
}

Expand Down
28 changes: 23 additions & 5 deletions crates/project-model/src/build_scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,21 @@ impl WorkspaceBuildScripts {
fn build_command(
config: &CargoConfig,
allowed_features: &FxHashSet<String>,
manifest_path: Option<AbsPathBuf>,
) -> io::Result<Command> {
let mut cmd = match config.run_build_script_command.as_deref() {
Some([program, args @ ..]) => {
let mut cmd = Command::new(program);
cmd.args(args);
for arg in args {
if let Some(manifest_path) = &manifest_path {
cmd.arg(arg.replace(
"$manifest_path",
&manifest_path.as_os_str().to_string_lossy(),
));
} else {
cmd.arg(arg);
}
}
cmd
}
_ => {
Expand Down Expand Up @@ -139,7 +149,11 @@ impl WorkspaceBuildScripts {
let allowed_features = workspace.workspace_features();

match Self::run_per_ws(
Self::build_command(config, &allowed_features)?,
Self::build_command(
config,
&allowed_features,
Some(workspace.workspace_root().join("Cargo.toml")),
)?,
workspace,
current_dir,
progress,
Expand All @@ -149,8 +163,12 @@ impl WorkspaceBuildScripts {
{
// 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, &allowed_features)?;
cmd.args(["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1");
let mut cmd = Self::build_command(
config,
&allowed_features,
Some(workspace.workspace_root().join("Cargo.toml")),
)?;
cmd.args(&["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1");
let mut res = Self::run_per_ws(cmd, workspace, current_dir, progress)?;
res.error = Some(error);
Ok(res)
Expand All @@ -177,7 +195,7 @@ impl WorkspaceBuildScripts {
))
}
};
let cmd = Self::build_command(config, &Default::default())?;
let cmd = Self::build_command(config, &Default::default(), 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`.
Expand Down
1 change: 1 addition & 0 deletions crates/project-model/src/project_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pub struct ProjectJson {
pub(crate) sysroot: Option<AbsPathBuf>,
/// e.g. `path/to/sysroot/lib/rustlib/src/rust`
pub(crate) sysroot_src: Option<AbsPathBuf>,
/// the folder containing `rust-project.json`)
project_root: AbsPathBuf,
crates: Vec<Crate>,
}
Expand Down
12 changes: 8 additions & 4 deletions crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,10 @@ config_data! {
/// 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.
/// If `once` is set, the command will be executed once.
/// - `per_workspace`: Executes the command for each workspace separately.
/// Allows the use of the `$manifest_path` substitution variable in the override command
/// which will be replaced by the path of the workspace's Cargo.toml.
/// - `once`: Executes the command once.
/// This config only has an effect when `#rust-analyzer.cargo.buildScripts.overrideCommand#`
/// is set.
cargo_buildScripts_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
Expand Down Expand Up @@ -164,8 +166,10 @@ config_data! {
/// is set.
check_invocationLocation | checkOnSave_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.
/// - `per_workspace`: Executes the command for each workspace separately.
/// Allows the use of the `$manifest_path` substitution variable in the override command
/// which will be replaced by the path of the workspace's Cargo.toml or rust-project.json.
/// - `once`: Executes the command once.
/// This config only has an effect when `#rust-analyzer.cargo.check.overrideCommand#`
/// is set.
check_invocationStrategy | checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
Expand Down
22 changes: 12 additions & 10 deletions crates/rust-analyzer/src/handlers/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use lsp_types::{
DidOpenTextDocumentParams, DidSaveTextDocumentParams, WorkDoneProgressCancelParams,
};
use triomphe::Arc;
use vfs::{AbsPathBuf, ChangeKind, VfsPath};
use vfs::{AbsPath, AbsPathBuf, ChangeKind, VfsPath};

use crate::{
config::Config,
Expand Down Expand Up @@ -132,21 +132,22 @@ pub(crate) fn handle_did_save_text_document(
) -> anyhow::Result<()> {
if let Ok(vfs_path) = from_proto::vfs_path(&params.text_document.uri) {
// Re-fetch workspaces if a workspace related file has changed
if let Some(abs_path) = vfs_path.as_path() {
let abs_path = vfs_path.as_path();
if let Some(abs_path) = abs_path {
if reload::should_refresh_for_change(abs_path, ChangeKind::Modify) {
state
.fetch_workspaces_queue
.request_op(format!("DidSaveTextDocument {abs_path}"), false);
}
}

if !state.config.check_on_save() || run_flycheck(state, vfs_path) {
if !state.config.check_on_save() || run_flycheck(state, &vfs_path, abs_path) {
return Ok(());
}
} else if state.config.check_on_save() {
// No specific flycheck was triggered, so let's trigger all of them.
for flycheck in state.flycheck.iter() {
flycheck.restart();
flycheck.restart(None);
}
}
Ok(())
Expand Down Expand Up @@ -232,10 +233,11 @@ pub(crate) fn handle_did_change_watched_files(
Ok(())
}

fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
fn run_flycheck(state: &mut GlobalState, vfs_path: &VfsPath, abs_path: Option<&AbsPath>) -> bool {
let _p = profile::span("run_flycheck");

let file_id = state.vfs.read().0.file_id(&vfs_path);
let file_id = state.vfs.read().0.file_id(vfs_path);
let abs_path = abs_path.map(AbsPath::to_owned);
if let Some(file_id) = file_id {
let world = state.snapshot();
let mut updated = false;
Expand Down Expand Up @@ -287,15 +289,15 @@ fn run_flycheck(state: &mut GlobalState, vfs_path: VfsPath) -> bool {
for (id, _) in workspace_ids.clone() {
if id == flycheck.id() {
updated = true;
flycheck.restart();
flycheck.restart(abs_path.clone());
continue;
}
}
}
// No specific flycheck was triggered, so let's trigger all of them.
if !updated {
for flycheck in world.flycheck.iter() {
flycheck.restart();
flycheck.restart(abs_path.clone());
}
}
Ok(())
Expand Down Expand Up @@ -330,14 +332,14 @@ pub(crate) fn handle_run_flycheck(
let _p = profile::span("handle_run_flycheck");
if let Some(text_document) = params.text_document {
if let Ok(vfs_path) = from_proto::vfs_path(&text_document.uri) {
if run_flycheck(state, vfs_path) {
if run_flycheck(state, &vfs_path, vfs_path.as_path()) {
return Ok(());
}
}
}
// No specific flycheck was triggered, so let's trigger all of them.
for flycheck in state.flycheck.iter() {
flycheck.restart();
flycheck.restart(None);
}
Ok(())
}
3 changes: 1 addition & 2 deletions crates/rust-analyzer/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::{

use always_assert::always;
use crossbeam_channel::{select, Receiver};
use flycheck::FlycheckHandle;
use ide_db::base_db::{SourceDatabaseExt, VfsPath};
use lsp_server::{Connection, Notification, Request};
use lsp_types::notification::Notification as _;
Expand Down Expand Up @@ -299,7 +298,7 @@ impl GlobalState {
if became_quiescent {
if self.config.check_on_save() {
// Project has loaded properly, kick off initial flycheck
self.flycheck.iter().for_each(FlycheckHandle::restart);
self.flycheck.iter().for_each(|flycheck| flycheck.restart(None));
}
if self.config.prefill_caches() {
self.prime_caches_queue.request_op("became quiescent".to_string(), ());
Expand Down
Loading

0 comments on commit 32fca82

Please sign in to comment.