Skip to content

Commit

Permalink
fix(cli): exit on non-compilation Cargo errors, closes #3930 (#3942)
Browse files Browse the repository at this point in the history
  • Loading branch information
lucasfernog committed Apr 22, 2022
1 parent 81705bb commit b562288
Show file tree
Hide file tree
Showing 12 changed files with 148 additions and 79 deletions.
6 changes: 6 additions & 0 deletions .changes/cli-compilation-error-exit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"cli.rs": patch
"cli.js": patch
---

Exit CLI when Cargo returns a non-compilation error in `tauri dev`.
6 changes: 6 additions & 0 deletions .changes/command-stdio-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"tauri": patch
"api": patch
---

**Breaking change:** The process Command API stdio lines now includes the trailing `\r`.
5 changes: 5 additions & 0 deletions .changes/io-read-line-util.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"tauri-utils": patch
---

Added the `io` module with the `read_line` method.
1 change: 1 addition & 0 deletions core/tauri-utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ json5 = { version = "0.4", optional = true }
json-patch = "0.2"
glob = { version = "0.3.0", optional = true }
walkdir = { version = "2", optional = true }
memchr = "2.4"

[target."cfg(target_os = \"linux\")".dependencies]
heck = "0.4"
Expand Down
49 changes: 49 additions & 0 deletions core/tauri-utils/src/io.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2019-2021 Tauri Programme within The Commons Conservancy
// SPDX-License-Identifier: Apache-2.0
// SPDX-License-Identifier: MIT

//! IO helpers.

use std::io::BufRead;

/// Read a line breaking in both \n and \r.
///
/// Adapted from https://doc.rust-lang.org/std/io/trait.BufRead.html#method.read_line
pub fn read_line<R: BufRead + ?Sized>(r: &mut R, buf: &mut Vec<u8>) -> std::io::Result<usize> {
let mut read = 0;
loop {
let (done, used) = {
let available = match r.fill_buf() {
Ok(n) => n,
Err(ref e) if e.kind() == std::io::ErrorKind::Interrupted => continue,
Err(e) => return Err(e),
};
match memchr::memchr(b'\n', available) {
Some(i) => {
let end = i + 1;
buf.extend_from_slice(&available[..end]);
(true, end)
}
None => match memchr::memchr(b'\r', available) {
Some(i) => {
let end = i + 1;
buf.extend_from_slice(&available[..end]);
(true, end)
}
None => {
buf.extend_from_slice(available);
(false, available.len())
}
},
}
};
r.consume(used);
read += used;
if done || used == 0 {
if buf.ends_with(&[b'\n']) {
buf.pop();
}
return Ok(read);
}
}
}
1 change: 1 addition & 0 deletions core/tauri-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
pub mod assets;
pub mod config;
pub mod html;
pub mod io;
pub mod platform;
/// Prepare application resources and sidecars.
#[cfg(feature = "resources")]
Expand Down
3 changes: 1 addition & 2 deletions core/tauri/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ attohttpc = { version = "0.19", features = [ "json", "form" ], optional = true }
open = { version = "2.0", optional = true }
shared_child = { version = "1.0", optional = true }
os_pipe = { version = "1.0", optional = true }
memchr = { version = "2.4", optional = true }
rfd = { version = "0.8", optional = true }
raw-window-handle = "0.4.2"
minisign-verify = { version = "0.2", optional = true }
Expand Down Expand Up @@ -138,7 +137,7 @@ http-api = [ "attohttpc" ]
shell-open-api = [ "open", "regex", "tauri-macros/shell-scope" ]
fs-extract-api = [ "zip" ]
reqwest-client = [ "reqwest", "bytes" ]
process-command-api = [ "shared_child", "os_pipe", "memchr" ]
process-command-api = [ "shared_child", "os_pipe" ]
dialog = [ "rfd" ]
notification = [ "notify-rust" ]
cli = [ "clap" ]
Expand Down
50 changes: 2 additions & 48 deletions core/tauri/src/api/process/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use std::{
collections::HashMap,
io::{BufRead, BufReader, Write},
io::{BufReader, Write},
path::PathBuf,
process::{Command as StdCommand, Stdio},
sync::{Arc, Mutex, RwLock},
Expand Down Expand Up @@ -384,7 +384,7 @@ fn spawn_pipe_reader<F: Fn(String) -> CommandEvent + Send + Copy + 'static>(
let mut buf = Vec::new();
loop {
buf.clear();
match read_command_output(&mut reader, &mut buf) {
match tauri_utils::io::read_line(&mut reader, &mut buf) {
Ok(n) => {
if n == 0 {
break;
Expand All @@ -407,52 +407,6 @@ fn spawn_pipe_reader<F: Fn(String) -> CommandEvent + Send + Copy + 'static>(
});
}

// adapted from https://doc.rust-lang.org/std/io/trait.BufRead.html#method.read_line
fn read_command_output<R: BufRead + ?Sized>(
r: &mut R,
buf: &mut Vec<u8>,
) -> std::io::Result<usize> {
let mut read = 0;
loop {
let (done, used) = {
let available = match r.fill_buf() {
Ok(n) => n,
Err(ref e) if e.kind() == std::io::ErrorKind::Interrupted => continue,
Err(e) => return Err(e),
};
match memchr::memchr(b'\n', available) {
Some(i) => {
let end = i + 1;
buf.extend_from_slice(&available[..end]);
(true, end)
}
None => match memchr::memchr(b'\r', available) {
Some(i) => {
let end = i + 1;
buf.extend_from_slice(&available[..end]);
(true, end)
}
None => {
buf.extend_from_slice(available);
(false, available.len())
}
},
}
};
r.consume(used);
read += used;
if done || used == 0 {
if buf.ends_with(&[b'\n']) {
buf.pop();
}
if buf.ends_with(&[b'\r']) {
buf.pop();
}
return Ok(read);
}
}
}

// tests for the commands functions.
#[cfg(test)]
mod test {
Expand Down
2 changes: 1 addition & 1 deletion examples/api/src-tauri/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions tooling/cli/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tooling/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ url = { version = "2.2", features = [ "serde" ] }
os_pipe = "1"
ignore = "0.4"
ctrlc = "3.2"
term_size = "0.3"

[target."cfg(windows)".dependencies]
encode_unicode = "0.3"
Expand Down
91 changes: 63 additions & 28 deletions tooling/cli/src/dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ use std::{
env::set_current_dir,
ffi::OsStr,
fs::FileType,
io::BufReader,
path::{Path, PathBuf},
process::{exit, Command},
sync::{
atomic::{AtomicBool, Ordering},
mpsc::{channel, Receiver, Sender},
mpsc::channel,
Arc, Mutex,
},
time::Duration,
Expand Down Expand Up @@ -189,8 +190,7 @@ fn command_internal(options: Options) -> Result<()> {
cargo_features.extend(features.clone());
}

let (child_wait_tx, child_wait_rx) = channel();
let child_wait_rx = Arc::new(Mutex::new(child_wait_rx));
let manually_killed_app = Arc::new(AtomicBool::default());

if std::env::var_os("TAURI_SKIP_DEVSERVER_CHECK") != Some("true".into()) {
if let AppUrl::Url(WindowUrl::External(dev_server_url)) = config
Expand Down Expand Up @@ -256,13 +256,12 @@ fn command_internal(options: Options) -> Result<()> {
&runner,
&manifest,
&cargo_features,
child_wait_rx.clone(),
manually_killed_app.clone(),
)?;
let shared_process = Arc::new(Mutex::new(process));
if let Err(e) = watch(
shared_process.clone(),
child_wait_tx,
child_wait_rx,
manually_killed_app,
tauri_path,
merge_config,
config,
Expand Down Expand Up @@ -306,8 +305,7 @@ fn lookup<F: FnMut(FileType, PathBuf)>(dir: &Path, mut f: F) {
#[allow(clippy::too_many_arguments)]
fn watch(
process: Arc<Mutex<Arc<SharedChild>>>,
child_wait_tx: Sender<()>,
child_wait_rx: Arc<Mutex<Receiver<()>>>,
manually_killed_app: Arc<AtomicBool>,
tauri_path: PathBuf,
merge_config: Option<String>,
config: ConfigHandle,
Expand Down Expand Up @@ -350,7 +348,7 @@ fn watch(
// When tauri.conf.json is changed, rewrite_manifest will be called
// which will trigger the watcher again
// So the app should only be started when a file other than tauri.conf.json is changed
let _ = child_wait_tx.send(());
manually_killed_app.store(true, Ordering::Relaxed);
let mut p = process.lock().unwrap();
p.kill().with_context(|| "failed to kill app process")?;
// wait for the process to exit
Expand All @@ -364,7 +362,7 @@ fn watch(
&runner,
&manifest,
&cargo_features,
child_wait_rx.clone(),
manually_killed_app.clone(),
)?;
}
}
Expand Down Expand Up @@ -412,10 +410,19 @@ fn start_app(
runner: &str,
manifest: &Manifest,
features: &[String],
child_wait_rx: Arc<Mutex<Receiver<()>>>,
manually_killed_app: Arc<AtomicBool>,
) -> Result<Arc<SharedChild>> {
let mut command = Command::new(runner);
command.arg("run");
command
.env(
"CARGO_TERM_PROGRESS_WIDTH",
term_size::dimensions_stderr()
.map(|(w, _)| w)
.unwrap_or(80)
.to_string(),
)
.env("CARGO_TERM_PROGRESS_WHEN", "always");
command.arg("run").arg("--color").arg("always");

if !options.args.contains(&"--no-default-features".into()) {
let manifest_features = manifest.features();
Expand Down Expand Up @@ -454,34 +461,62 @@ fn start_app(
command.args(&options.args);
}

command.pipe().unwrap();
command.stdout(os_pipe::dup_stdout().unwrap());
command.stderr(std::process::Stdio::piped());

let child =
SharedChild::spawn(&mut command).with_context(|| format!("failed to run {}", runner))?;
let child_arc = Arc::new(child);
let child_stderr = child_arc.take_stderr().unwrap();
let mut stderr = BufReader::new(child_stderr);
let stderr_lines = Arc::new(Mutex::new(Vec::new()));
let stderr_lines_ = stderr_lines.clone();
std::thread::spawn(move || {
let mut buf = Vec::new();
let mut lines = stderr_lines_.lock().unwrap();
loop {
buf.clear();
match tauri_utils::io::read_line(&mut stderr, &mut buf) {
Ok(s) if s == 0 => break,
_ => (),
}
let line = String::from_utf8_lossy(&buf).into_owned();
if line.ends_with('\r') {
eprint!("{}", line);
} else {
eprintln!("{}", line);
}
lines.push(line);
}
});

let child_clone = child_arc.clone();
let exit_on_panic = options.exit_on_panic;
std::thread::spawn(move || {
let status = child_clone.wait().expect("failed to wait on child");

if exit_on_panic {
// we exit if the status is a success code (app closed) or code is 101 (compilation error)
// if the process wasn't killed by the file watcher
if (status.success() || status.code() == Some(101))
// `child_wait_rx` indicates that the process was killed by the file watcher
&& child_wait_rx
.lock()
.expect("failed to get child_wait_rx lock")
.try_recv()
.is_err()
{
if !manually_killed_app.load(Ordering::Relaxed) {
kill_before_dev_process();
exit(status.code().unwrap_or(0));
}
} else {
let is_cargo_compile_error = stderr_lines
.lock()
.unwrap()
.last()
.map(|l| l.contains("could not compile"))
.unwrap_or_default();
stderr_lines.lock().unwrap().clear();

// if we're no exiting on panic, we only exit if:
// - the status is a success code (app closed)
// - status code is the Cargo error code
// - and error is not a cargo compilation error (using stderr heuristics)
if status.success() || (status.code() == Some(101) && !is_cargo_compile_error) {
kill_before_dev_process();
exit(0);
exit(status.code().unwrap_or(1));
}
} else if status.success() {
// if we're no exiting on panic, we only exit if the status is a success code (app closed)
kill_before_dev_process();
exit(0);
}
});

Expand Down

0 comments on commit b562288

Please sign in to comment.