aboutsummaryrefslogtreecommitdiff
path: root/crates/ra_cargo_watch
diff options
context:
space:
mode:
authorveetaha <[email protected]>2020-03-21 21:30:33 +0000
committerveetaha <[email protected]>2020-03-21 21:37:15 +0000
commitce73c43848aa394530693967b8a6e9343f7b99b8 (patch)
treeafc49f519ddabc5218565bde517ff6fe800e7aa0 /crates/ra_cargo_watch
parent59ba386bee974b56b546eb7e8fdec6721ab0d08a (diff)
ra_cargo_watch: return Result<> from run_cargo(), and don't read stderr for now
As stated by matklad, reading the stderr should be done alngside with stdout via select() (or I guess poll()), there is no such implementation in stdlib, since it is quite low level and platform-dependent and it also requires quite a bit of unrelated code we don't use it for now. As referenced by bjorn3, there is an implementation of the needed read2() function in rustc compiletest. The better solution will be to extract this function to a separate crate in future: https://github.com/rust-analyzer/rust-analyzer/pull/3632#discussion_r395605298
Diffstat (limited to 'crates/ra_cargo_watch')
-rw-r--r--crates/ra_cargo_watch/src/lib.rs78
1 files changed, 43 insertions, 35 deletions
diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs
index deb763af9..35b0cb608 100644
--- a/crates/ra_cargo_watch/src/lib.rs
+++ b/crates/ra_cargo_watch/src/lib.rs
@@ -8,9 +8,10 @@ use lsp_types::{
8 WorkDoneProgressEnd, WorkDoneProgressReport, 8 WorkDoneProgressEnd, WorkDoneProgressReport,
9}; 9};
10use std::{ 10use std::{
11 error, fmt,
11 io::{BufRead, BufReader}, 12 io::{BufRead, BufReader},
12 path::{Path, PathBuf}, 13 path::{Path, PathBuf},
13 process::{Child, Command, Stdio}, 14 process::{Command, Stdio},
14 thread::JoinHandle, 15 thread::JoinHandle,
15 time::Instant, 16 time::Instant,
16}; 17};
@@ -70,10 +71,10 @@ impl std::ops::Drop for CheckWatcher {
70 fn drop(&mut self) { 71 fn drop(&mut self) {
71 if let Some(handle) = self.handle.take() { 72 if let Some(handle) = self.handle.take() {
72 // Take the sender out of the option 73 // Take the sender out of the option
73 let recv = self.cmd_send.take(); 74 let cmd_send = self.cmd_send.take();
74 75
75 // Dropping the sender finishes the thread loop 76 // Dropping the sender finishes the thread loop
76 drop(recv); 77 drop(cmd_send);
77 78
78 // Join the thread, it should finish shortly. We don't really care 79 // Join the thread, it should finish shortly. We don't really care
79 // whether it panicked, so it is safe to ignore the result 80 // whether it panicked, so it is safe to ignore the result
@@ -246,11 +247,21 @@ enum CheckEvent {
246 End, 247 End,
247} 248}
248 249
250#[derive(Debug)]
251pub struct CargoError(String);
252
253impl fmt::Display for CargoError {
254 fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
255 write!(f, "Cargo failed: {}", self.0)
256 }
257}
258impl error::Error for CargoError {}
259
249pub fn run_cargo( 260pub fn run_cargo(
250 args: &[String], 261 args: &[String],
251 current_dir: Option<&Path>, 262 current_dir: Option<&Path>,
252 on_message: &mut dyn FnMut(cargo_metadata::Message) -> bool, 263 on_message: &mut dyn FnMut(cargo_metadata::Message) -> bool,
253) -> Child { 264) -> Result<(), CargoError> {
254 let mut command = Command::new("cargo"); 265 let mut command = Command::new("cargo");
255 if let Some(current_dir) = current_dir { 266 if let Some(current_dir) = current_dir {
256 command.current_dir(current_dir); 267 command.current_dir(current_dir);
@@ -259,7 +270,7 @@ pub fn run_cargo(
259 let mut child = command 270 let mut child = command
260 .args(args) 271 .args(args)
261 .stdout(Stdio::piped()) 272 .stdout(Stdio::piped())
262 .stderr(Stdio::piped()) 273 .stderr(Stdio::null())
263 .stdin(Stdio::null()) 274 .stdin(Stdio::null())
264 .spawn() 275 .spawn()
265 .expect("couldn't launch cargo"); 276 .expect("couldn't launch cargo");
@@ -273,6 +284,8 @@ pub fn run_cargo(
273 // simply skip a line if it doesn't parse, which just ignores any 284 // simply skip a line if it doesn't parse, which just ignores any
274 // erroneus output. 285 // erroneus output.
275 let stdout = BufReader::new(child.stdout.take().unwrap()); 286 let stdout = BufReader::new(child.stdout.take().unwrap());
287 let mut read_at_least_one_message = false;
288
276 for line in stdout.lines() { 289 for line in stdout.lines() {
277 let line = match line { 290 let line = match line {
278 Ok(line) => line, 291 Ok(line) => line,
@@ -291,12 +304,27 @@ pub fn run_cargo(
291 } 304 }
292 }; 305 };
293 306
307 read_at_least_one_message = true;
308
294 if !on_message(message) { 309 if !on_message(message) {
295 break; 310 break;
296 } 311 }
297 } 312 }
298 313
299 child 314 // It is okay to ignore the result, as it only errors if the process is already dead
315 let _ = child.kill();
316
317 let err_msg = match child.wait() {
318 Ok(exit_code) if !exit_code.success() && !read_at_least_one_message => {
319 // FIXME: Read the stderr to display the reason, see `read2()` reference in PR comment:
320 // https://github.com/rust-analyzer/rust-analyzer/pull/3632#discussion_r395605298
321 format!("the command produced no valid metadata:\n cargo {}", args.join(" "))
322 }
323 Err(err) => format!("io error: {:?}", err),
324 Ok(_) => return Ok(()),
325 };
326
327 Err(CargoError(err_msg))
300} 328}
301 329
302impl WatchThread { 330impl WatchThread {
@@ -325,7 +353,7 @@ impl WatchThread {
325 // which will break out of the loop, and continue the shutdown 353 // which will break out of the loop, and continue the shutdown
326 let _ = message_send.send(CheckEvent::Begin); 354 let _ = message_send.send(CheckEvent::Begin);
327 355
328 let mut child = run_cargo(&args, Some(&workspace_root), &mut |message| { 356 let res = run_cargo(&args, Some(&workspace_root), &mut |message| {
329 // Skip certain kinds of messages to only spend time on what's useful 357 // Skip certain kinds of messages to only spend time on what's useful
330 match &message { 358 match &message {
331 Message::CompilerArtifact(artifact) if artifact.fresh => return true, 359 Message::CompilerArtifact(artifact) if artifact.fresh => return true,
@@ -334,39 +362,19 @@ impl WatchThread {
334 _ => {} 362 _ => {}
335 } 363 }
336 364
337 match message_send.send(CheckEvent::Msg(message)) { 365 // if the send channel was closed, so we want to shutdown
338 Ok(()) => {} 366 message_send.send(CheckEvent::Msg(message)).is_ok()
339 Err(_err) => {
340 // The send channel was closed, so we want to shutdown
341 return false;
342 }
343 };
344
345 true
346 }); 367 });
347 368
369 if let Err(err) = res {
370 // FIXME: make the `message_send` to be `Sender<Result<CheckEvent, CargoError>>`
371 // to display user-caused misconfiguration errors instead of just logging them here
372 log::error!("Cargo watcher failed {:?}", err);
373 }
374
348 // We can ignore any error here, as we are already in the progress 375 // We can ignore any error here, as we are already in the progress
349 // of shutting down. 376 // of shutting down.
350 let _ = message_send.send(CheckEvent::End); 377 let _ = message_send.send(CheckEvent::End);
351
352 // It is okay to ignore the result, as it only errors if the process is already dead
353 let _ = child.kill();
354
355 // Again, we are resilient to errors, so we don't try to panic here
356 match child.wait_with_output() {
357 Ok(output) => match output.status.code() {
358 Some(0) | None => {}
359 Some(exit_code) => {
360 let output =
361 std::str::from_utf8(&output.stderr).unwrap_or("<bad utf8 output>");
362
363 if !output.contains("could not compile") {
364 log::error!("Cargo failed with exit code {} {}", exit_code, output);
365 }
366 }
367 },
368 Err(err) => log::error!("Cargo io error: {:?}", err),
369 }
370 })) 378 }))
371 } else { 379 } else {
372 None 380 None