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 Nov 2, 2022
1 parent d022e0e commit aa6d45b
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 71 deletions.
96 changes: 44 additions & 52 deletions crates/flycheck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,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::<Restart>();
let thread = jod_thread::Builder::new()
.name("Flycheck".to_owned())
Expand All @@ -96,8 +97,8 @@ impl FlycheckHandle {
}

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

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

enum Restart {
Yes,
Yes { saved_file: Option<AbsPathBuf> },
No,
}

Expand All @@ -161,12 +162,14 @@ 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
/// have to wrap sub-processes output handling in a thread and pass messages
/// back over a channel.
cargo_handle: Option<CargoHandle>,
cargo_handle: Option<(CargoHandle, String)>,
}

enum Event {
Expand All @@ -180,17 +183,18 @@ 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, cargo_handle: None }
FlycheckActor { id, sender, config, root: workspace_root, project_file, cargo_handle: None }
}

fn report_progress(&self, progress: Progress) {
self.send(Message::Progress { id: self.id, progress });
}

fn next_event(&self, inbox: &Receiver<Restart>) -> Option<Event> {
let check_chan = self.cargo_handle.as_ref().map(|cargo| &cargo.receiver);
let check_chan = self.cargo_handle.as_ref().map(|(cargo, _)| &cargo.receiver);
if let Ok(msg) = inbox.try_recv() {
// give restarts a preference so check outputs don't block a restart or stop
return Some(Event::Restart(msg));
Expand All @@ -204,10 +208,8 @@ impl FlycheckActor {
fn run(mut self, inbox: Receiver<Restart>) {
'event: while let Some(event) = self.next_event(&inbox) {
match event {
Event::Restart(Restart::No) => {
self.cancel_check_process();
}
Event::Restart(Restart::Yes) => {
Event::Restart(Restart::No) => self.cancel_check_process(),
Event::Restart(Restart::Yes { saved_file }) => {
// Cancel the previously spawned process
self.cancel_check_process();
while let Ok(restart) = inbox.recv_timeout(Duration::from_millis(50)) {
Expand All @@ -217,22 +219,18 @@ impl FlycheckActor {
}
}

let command = self.check_command();
tracing::debug!(?command, "will restart flycheck");
let command = self.check_command(saved_file);
let command_string = format!("{:?}", command);
tracing::debug!(?command, "restarting flycheck");
match CargoHandle::spawn(command) {
Ok(cargo_handle) => {
tracing::debug!(
command = ?self.check_command(),
"did restart flycheck"
);
self.cargo_handle = Some(cargo_handle);
self.cargo_handle = Some((cargo_handle, command_string));
self.report_progress(Progress::DidStart);
}
Err(error) => {
self.report_progress(Progress::DidFailToRestart(format!(
"Failed to run the following command: {:?} error={}",
self.check_command(),
error
command_string, error
)));
}
}
Expand All @@ -241,12 +239,12 @@ impl FlycheckActor {
tracing::debug!(flycheck_id = self.id, "flycheck finished");

// Watcher finished
let cargo_handle = self.cargo_handle.take().unwrap();
let (cargo_handle, cmd_string) = self.cargo_handle.take().unwrap();
let res = cargo_handle.join();
if res.is_err() {
tracing::error!(
"Flycheck failed to run the following command: {:?}",
self.check_command()
command = cmd_string,
"Flycheck failed to run the following command"
);
}
self.report_progress(Progress::DidFinish(res));
Expand All @@ -271,18 +269,17 @@ impl FlycheckActor {
}

fn cancel_check_process(&mut self) {
if let Some(cargo_handle) = self.cargo_handle.take() {
tracing::debug!(
command = ?self.check_command(),
"did cancel flycheck"
);
if let Some((cargo_handle, cmd_string)) = self.cargo_handle.take() {
tracing::debug!(command = cmd_string, "did cancel flycheck");
cargo_handle.cancel();
self.report_progress(Progress::DidCancel);
}
}

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_triple,
Expand All @@ -297,7 +294,7 @@ impl FlycheckActor {
cmd.arg(command);
cmd.current_dir(&self.root);
cmd.args(&["--workspace", "--message-format=json", "--manifest-path"])
.arg(self.root.join("Cargo.toml").as_os_str());
.arg(self.project_file.as_os_str());

if let Some(target) = target_triple {
cmd.args(&["--target", target.as_str()]);
Expand All @@ -317,7 +314,8 @@ impl FlycheckActor {
}
}
cmd.envs(extra_env);
(cmd, extra_args)
cmd.args(extra_args);
cmd
}
FlycheckConfig::CustomCommand {
command,
Expand All @@ -328,30 +326,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 @@ -208,6 +208,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
30 changes: 25 additions & 5 deletions crates/project-model/src/build_scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,23 @@ impl BuildScriptOutput {
}

impl WorkspaceBuildScripts {
fn build_command(config: &CargoConfig) -> io::Result<Command> {
fn build_command(
config: &CargoConfig,
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 @@ -126,13 +138,21 @@ impl WorkspaceBuildScripts {
}
.as_ref();

match Self::run_per_ws(Self::build_command(config)?, workspace, current_dir, progress) {
match Self::run_per_ws(
Self::build_command(config, Some(workspace.workspace_root().join("Cargo.toml")))?,
workspace,
current_dir,
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.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);
Expand Down Expand Up @@ -160,7 +180,7 @@ impl WorkspaceBuildScripts {
))
}
};
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`.
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 @@ -21,6 +21,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 @@ -77,8 +77,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 @@ -144,8 +146,10 @@ config_data! {
/// 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.
/// 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.buildScripts.overrideCommand#`
/// is set.
checkOnSave_invocationStrategy: InvocationStrategy = "\"per_workspace\"",
Expand Down
16 changes: 9 additions & 7 deletions crates/rust-analyzer/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use std::{

use always_assert::always;
use crossbeam_channel::{select, Receiver};
use flycheck::FlycheckHandle;
use ide_db::base_db::{SourceDatabaseExt, VfsPath};
use itertools::Itertools;
use lsp_server::{Connection, Notification, Request};
Expand Down Expand Up @@ -288,7 +287,7 @@ impl GlobalState {

if became_quiescent {
// 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 Expand Up @@ -783,12 +782,15 @@ impl GlobalState {
.on::<lsp_types::notification::DidSaveTextDocument>(|this, params| {
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 saved_file_path = if let Some(abs_path) = vfs_path.as_path() {
if reload::should_refresh_for_change(&abs_path, ChangeKind::Modify) {
this.fetch_workspaces_queue
.request_op(format!("DidSaveTextDocument {}", abs_path.display()));
}
}
Some(abs_path.to_path_buf())
} else {
None
};

let file_id = this.vfs.read().0.file_id(&vfs_path);
if let Some(file_id) = file_id {
Expand Down Expand Up @@ -848,15 +850,15 @@ impl GlobalState {
for (id, _) in workspace_ids.clone() {
if id == flycheck.id() {
updated = true;
flycheck.restart();
flycheck.restart(saved_file_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(saved_file_path.clone());
}
}
Ok(())
Expand All @@ -872,7 +874,7 @@ impl GlobalState {

// No specific flycheck was triggered, so let's trigger all of them.
for flycheck in this.flycheck.iter() {
flycheck.restart();
flycheck.restart(None);
}
Ok(())
})?
Expand Down
Loading

0 comments on commit aa6d45b

Please sign in to comment.