Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/flycheck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ impl CargoActor {
Ok(output) if output.status.success() => Ok(()),
Ok(output) => {
Err(io::Error::new(io::ErrorKind::Other, format!(
"Cargo watcher failed, the command produced no valid metadata (exit code: {:?})\nCargo's stderr output:\n{}",
"Cargo watcher failed, the command produced no valid metadata (exit code: {:?}):\n{}",
output.status, error
)))
}
Expand Down
22 changes: 16 additions & 6 deletions crates/project_model/src/build_scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
//! here, but it covers procedural macros as well.

use std::{
io,
path::PathBuf,
process::{Command, Stdio},
};

use anyhow::Result;
use cargo_metadata::{camino::Utf8Path, Message};
use la_arena::ArenaMap;
use paths::AbsPathBuf;
Expand Down Expand Up @@ -80,7 +80,7 @@ impl WorkspaceBuildScripts {
config: &CargoConfig,
workspace: &CargoWorkspace,
progress: &dyn Fn(String),
) -> Result<WorkspaceBuildScripts> {
) -> io::Result<WorkspaceBuildScripts> {
let mut cmd = Self::build_command(config);

if config.wrap_rustc_in_build_scripts {
Expand All @@ -107,12 +107,12 @@ impl WorkspaceBuildScripts {
by_id.insert(workspace[package].id.clone(), package);
}

let mut callback_err = None;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were never using this "error case" here, but once set we stop working through the build script output, so this might be the cause of #9720 not working for people without any apparent reason?

let mut cfg_err = None;
let mut stderr = String::new();
let output = stdx::process::streaming_output(
cmd,
&mut |line| {
if callback_err.is_some() {
if cfg_err.is_some() {
return;
}

Expand All @@ -126,7 +126,7 @@ impl WorkspaceBuildScripts {
match message {
Message::BuildScriptExecuted(message) => {
let package = match by_id.get(&message.package_id.repr) {
Some(it) => *it,
Some(&it) => it,
None => return,
};
let cfgs = {
Expand All @@ -135,7 +135,7 @@ impl WorkspaceBuildScripts {
match cfg.parse::<CfgFlag>() {
Ok(it) => acc.push(it),
Err(err) => {
callback_err = Some(anyhow::format_err!(
cfg_err = Some(format!(
"invalid cfg from cargo-metadata: {}",
err
));
Expand Down Expand Up @@ -191,6 +191,11 @@ impl WorkspaceBuildScripts {

for package in workspace.packages() {
let package_build_data = &mut res.outputs[package];
tracing::info!(
"{} BuildScriptOutput: {:?}",
workspace[package].manifest.parent().display(),
package_build_data,
);
// inject_cargo_env(package, package_build_data);
if let Some(out_dir) = &package_build_data.out_dir {
// NOTE: cargo and rustc seem to hide non-UTF-8 strings from env! and option_env!()
Expand All @@ -200,6 +205,11 @@ impl WorkspaceBuildScripts {
}
}

if let Some(cfg_err) = cfg_err {
stderr.push_str(&cfg_err);
stderr.push('\n');
}

if !output.status.success() {
if stderr.is_empty() {
stderr = "cargo check failed".to_string();
Expand Down
4 changes: 3 additions & 1 deletion crates/project_model/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ impl ProjectWorkspace {
) -> Result<WorkspaceBuildScripts> {
match self {
ProjectWorkspace::Cargo { cargo, .. } => {
WorkspaceBuildScripts::run(config, cargo, progress)
WorkspaceBuildScripts::run(config, cargo, progress).with_context(|| {
format!("Failed to run build scripts for {}", &cargo.workspace_root().display())
})
}
ProjectWorkspace::Json { .. } | ProjectWorkspace::DetachedFiles { .. } => {
Ok(WorkspaceBuildScripts::default())
Expand Down
4 changes: 3 additions & 1 deletion crates/rust-analyzer/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ fn setup_logging(log_file: Option<&Path>) -> Result<()> {
None => None,
};
let filter = env::var("RA_LOG").ok();
logger::Logger::new(log_file, filter.as_deref()).install()?;
// deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually useful
// information in there for debugging
logger::Logger::new(log_file, filter.as_deref().or(Some("error"))).install()?;
Comment on lines +122 to +124
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any arguments against this? The way this PR is structured now is that we do small error popups for build errors and others that only appear on project load and put the bulkier information into the server logs as most notification UIs (or maybe really only VSCode who knows) don't handle big messages that well.


profile::init();

Expand Down
6 changes: 5 additions & 1 deletion crates/rust-analyzer/src/global_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ impl GlobalState {

for file in changed_files {
if !file.is_created_or_deleted() {
// FIXME: https://github.com/rust-analyzer/rust-analyzer/issues/11357
let crates = self.analysis_host.raw_database().relevant_crates(file.file_id);
let crate_graph = self.analysis_host.raw_database().crate_graph();

Expand Down Expand Up @@ -255,6 +256,7 @@ impl GlobalState {
let request = self.req_queue.outgoing.register(R::METHOD.to_string(), params, handler);
self.send(request.into());
}

pub(crate) fn complete_request(&mut self, response: lsp_server::Response) {
let handler = self
.req_queue
Expand All @@ -281,6 +283,7 @@ impl GlobalState {
.incoming
.register(request.id.clone(), (request.method.clone(), request_received));
}

pub(crate) fn respond(&mut self, response: lsp_server::Response) {
if let Some((method, start)) = self.req_queue.incoming.complete(response.id.clone()) {
if let Some(err) = &response.error {
Expand All @@ -294,6 +297,7 @@ impl GlobalState {
self.send(response.into());
}
}

pub(crate) fn cancel(&mut self, request_id: lsp_server::RequestId) {
if let Some(response) = self.req_queue.incoming.cancel(request_id) {
self.send(response.into());
Expand All @@ -307,7 +311,7 @@ impl GlobalState {

impl Drop for GlobalState {
fn drop(&mut self) {
self.analysis_host.request_cancellation()
self.analysis_host.request_cancellation();
}
}

Expand Down
20 changes: 20 additions & 0 deletions crates/rust-analyzer/src/lsp_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,26 @@ impl GlobalState {
)
}

/// Sends a notification to the client containing the error `message`.
/// If `additional_info` is [`Some`], appends a note to the notification telling to check the logs.
/// This will always log `message` + `additional_info` to the server's error log.
pub(crate) fn show_and_log_error(&mut self, message: String, additional_info: Option<String>) {
let mut message = message;
match additional_info {
Some(additional_info) => {
tracing::error!("{}\n\n{}", &message, &additional_info);
if tracing::enabled!(tracing::Level::ERROR) {
message.push_str("\n\nCheck the server logs for additional info.");
}
}
None => tracing::error!("{}", &message),
}

self.send_notification::<lsp_types::notification::ShowMessage>(
lsp_types::ShowMessageParams { typ: lsp_types::MessageType::ERROR, message },
)
}

/// rust-analyzer is resilient -- if it fails, this doesn't usually affect
/// the user experience. Part of that is that we deliberately hide panics
/// from the user.
Expand Down
12 changes: 4 additions & 8 deletions crates/rust-analyzer/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,7 @@ impl GlobalState {
&& self.config.detached_files().is_empty()
&& self.config.notifications().cargo_toml_not_found
{
self.show_message(
lsp_types::MessageType::ERROR,
"rust-analyzer failed to discover workspace".to_string(),
);
self.show_and_log_error("rust-analyzer failed to discover workspace".to_string(), None);
};

if self.config.did_save_text_document_dynamic_registration() {
Expand Down Expand Up @@ -406,9 +403,9 @@ impl GlobalState {
flycheck::Progress::DidCancel => (Progress::End, None),
flycheck::Progress::DidFinish(result) => {
if let Err(err) = result {
self.show_message(
lsp_types::MessageType::ERROR,
format!("cargo check failed: {}", err),
self.show_and_log_error(
"cargo check failed".to_string(),
Some(err.to_string()),
);
}
(Progress::End, None)
Expand Down Expand Up @@ -564,7 +561,6 @@ impl GlobalState {
if self.workspaces.is_empty() && !self.is_quiescent() {
self.respond(lsp_server::Response::new_err(
req.id,
// FIXME: i32 should impl From<ErrorCode> (from() guarantees lossless conversion)
lsp_server::ErrorCode::ContentModified as i32,
"waiting for cargo metadata or cargo check".to_owned(),
));
Expand Down
63 changes: 30 additions & 33 deletions crates/rust-analyzer/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ impl GlobalState {
status.message =
Some("Reload required due to source changes of a procedural macro.".into())
}
if let Some(error) = self.fetch_build_data_error() {
if let Err(_) = self.fetch_build_data_error() {
status.health = lsp_ext::Health::Warning;
status.message = Some(error)
status.message =
Some("Failed to run build scripts of some packages, check the logs.".to_string());
}
if !self.config.cargo_autoreload()
&& self.is_quiescent()
Expand All @@ -84,7 +85,7 @@ impl GlobalState {
status.message = Some("Workspace reload required".to_string())
}

if let Some(error) = self.fetch_workspace_error() {
if let Err(error) = self.fetch_workspace_error() {
status.health = lsp_ext::Health::Error;
status.message = Some(error)
}
Expand Down Expand Up @@ -167,17 +168,20 @@ impl GlobalState {
let _p = profile::span("GlobalState::switch_workspaces");
tracing::info!("will switch workspaces");

if let Some(error_message) = self.fetch_workspace_error() {
tracing::error!("failed to switch workspaces: {}", error_message);
if let Err(error_message) = self.fetch_workspace_error() {
self.show_and_log_error(error_message, None);
if !self.workspaces.is_empty() {
// It only makes sense to switch to a partially broken workspace
// if we don't have any workspace at all yet.
return;
}
}

if let Some(error_message) = self.fetch_build_data_error() {
tracing::error!("failed to switch build data: {}", error_message);
if let Err(error) = self.fetch_build_data_error() {
self.show_and_log_error(
"rust-analyzer failed to run build scripts".to_string(),
Some(error),
);
}

let workspaces = self
Expand Down Expand Up @@ -277,20 +281,18 @@ impl GlobalState {
let project_folders = ProjectFolders::new(&self.workspaces, &files_config.exclude);

if self.proc_macro_client.is_none() {
self.proc_macro_client = match self.config.proc_macro_srv() {
None => None,
Some((path, args)) => match ProcMacroServer::spawn(path.clone(), args) {
Ok(it) => Some(it),
if let Some((path, args)) = self.config.proc_macro_srv() {
match ProcMacroServer::spawn(path.clone(), args) {
Ok(it) => self.proc_macro_client = Some(it),
Err(err) => {
tracing::error!(
"Failed to run proc_macro_srv from path {}, error: {:?}",
path.display(),
err
);
None
}
},
};
}
}
}

let watch = match files_config.watcher {
Expand Down Expand Up @@ -348,7 +350,7 @@ impl GlobalState {
tracing::info!("did switch workspaces");
}

fn fetch_workspace_error(&self) -> Option<String> {
fn fetch_workspace_error(&self) -> Result<(), String> {
let mut buf = String::new();

for ws in self.fetch_workspaces_queue.last_op_result() {
Expand All @@ -358,35 +360,30 @@ impl GlobalState {
}

if buf.is_empty() {
return None;
return Ok(());
}

Some(buf)
Err(buf)
}

fn fetch_build_data_error(&self) -> Option<String> {
let mut buf = "rust-analyzer failed to run build scripts:\n".to_string();
let mut has_errors = false;
fn fetch_build_data_error(&self) -> Result<(), String> {
let mut buf = String::new();

for ws in &self.fetch_build_data_queue.last_op_result().1 {
match ws {
Ok(data) => {
if let Some(err) = data.error() {
has_errors = true;
stdx::format_to!(buf, "{:#}\n", err);
}
}
Err(err) => {
has_errors = true;
stdx::format_to!(buf, "{:#}\n", err);
}
Ok(data) => match data.error() {
Some(stderr) => stdx::format_to!(buf, "{:#}\n", stderr),
_ => (),
},
// io errors
Err(err) => stdx::format_to!(buf, "{:#}\n", err),
}
}

if has_errors {
Some(buf)
if buf.is_empty() {
Ok(())
} else {
None
Err(buf)
}
}

Expand Down