Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cli): exit on non-compilation Cargo errors, closes #3930 #3942

Merged
merged 8 commits into from
Apr 22, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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