From 1ff81cc8cb7dce883564449c601fb6d1c5895fbf Mon Sep 17 00:00:00 2001 From: Julien Cretin Date: Wed, 2 Oct 2024 17:21:41 +0200 Subject: [PATCH] Batch of CLI fixes and improvements (#629) --- crates/cli-tools/CHANGELOG.md | 6 ++- crates/cli-tools/Cargo.lock | 1 + crates/cli-tools/Cargo.toml | 1 + crates/cli-tools/src/action.rs | 17 +++++-- crates/cli-tools/src/error.rs | 24 +++++++++ crates/cli-tools/src/fs.rs | 60 ++++++++++++++++++++-- crates/cli-tools/src/lib.rs | 9 ++-- crates/cli/CHANGELOG.md | 1 + crates/cli/Cargo.lock | 63 ++++++++++++++++++++++++ crates/cli/Cargo.toml | 1 + crates/cli/src/main.rs | 2 + crates/protocol-tokio/CHANGELOG.md | 2 +- crates/protocol-tokio/Cargo.toml | 6 +-- crates/protocol-tokio/src/common.rs | 41 +++------------ crates/protocol-tokio/src/device.rs | 4 +- crates/protocol-tokio/src/host.rs | 16 ++---- crates/protocol/crates/schema/Cargo.lock | 7 +++ crates/runner-host/Cargo.lock | 1 + crates/xtask/Cargo.lock | 1 + crates/xtask/src/main.rs | 8 +-- 20 files changed, 199 insertions(+), 72 deletions(-) create mode 100644 crates/cli-tools/src/error.rs diff --git a/crates/cli-tools/CHANGELOG.md b/crates/cli-tools/CHANGELOG.md index 5dd40c2d..fa18e15d 100644 --- a/crates/cli-tools/CHANGELOG.md +++ b/crates/cli-tools/CHANGELOG.md @@ -10,6 +10,8 @@ ### Minor +- Extend `fs::write()` first parameter to set the `OpenOptions` too +- Add `error::root_cause_is()` to check the `anyhow::Error` root cause - Add `action::PlatformLock` for locking a platform protocol - Expose `action::Transfer` for transfers from host to device - Add `action::AppletExitStatus` to get the applet exit status @@ -25,8 +27,10 @@ ### Patch +- Fix incorrect error with UNIX and TCP platform protocols +- Only print commands and file system operations when warnings are logged - Update dependencies ## 0.1.0 - + diff --git a/crates/cli-tools/Cargo.lock b/crates/cli-tools/Cargo.lock index d3f8fc0a..f27a40d1 100644 --- a/crates/cli-tools/Cargo.lock +++ b/crates/cli-tools/Cargo.lock @@ -583,6 +583,7 @@ dependencies = [ "data-encoding", "humantime", "indicatif", + "log", "rusb", "serde", "tokio", diff --git a/crates/cli-tools/Cargo.toml b/crates/cli-tools/Cargo.toml index f4aed11b..7848bc49 100644 --- a/crates/cli-tools/Cargo.toml +++ b/crates/cli-tools/Cargo.toml @@ -20,6 +20,7 @@ cargo_metadata = { version = "0.18.1", default-features = false, optional = true data-encoding = { version = "2.6.0", default-features = false, features = ["std"], optional = true } humantime = { version = "2.1.0", default-features = false, optional = true } indicatif = { version = "0.17.8", default-features = false, optional = true } +log = { version = "0.4.21", default-features = false } rusb = { version = "0.9.4", default-features = false, optional = true } serde = { version = "1.0.202", default-features = false, features = ["derive"] } toml = { version = "0.8.13", default-features = false, features = ["display", "parse"] } diff --git a/crates/cli-tools/src/action.rs b/crates/cli-tools/src/action.rs index 257ab9b7..7d4a5a50 100644 --- a/crates/cli-tools/src/action.rs +++ b/crates/cli-tools/src/action.rs @@ -24,6 +24,7 @@ use tokio::process::Command; use wasefire_protocol::{self as service, applet, Connection, ConnectionExt}; use wasefire_wire::{self as wire, Yoke}; +use crate::error::root_cause_is; use crate::{cmd, fs}; mod protocol; @@ -373,10 +374,18 @@ async fn final_call( connection.send(&S::request(request)).await?; match connection.receive::().await { Ok(x) => proof(x)?, - Err(e) => match e.downcast_ref::() { - Some(rusb::Error::NoDevice) => Ok(()), - _ => Err(e), - }, + Err(e) => { + if root_cause_is::(&e, |x| matches!(x, rusb::Error::NoDevice)) { + return Ok(()); + } + if root_cause_is::(&e, |x| { + use std::io::ErrorKind::*; + matches!(x.kind(), NotConnected | BrokenPipe | UnexpectedEof) + }) { + return Ok(()); + } + Err(e) + } } } diff --git a/crates/cli-tools/src/error.rs b/crates/cli-tools/src/error.rs new file mode 100644 index 00000000..55a49b87 --- /dev/null +++ b/crates/cli-tools/src/error.rs @@ -0,0 +1,24 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::error::Error; + +/// Checks the root cause of an error. +/// +/// Returns whether the root cause of the error is of the provided type and satisfies the predicate. +pub fn root_cause_is( + error: &anyhow::Error, predicate: impl FnOnce(&E) -> bool, +) -> bool { + error.root_cause().downcast_ref::().map_or(false, predicate) +} diff --git a/crates/cli-tools/src/fs.rs b/crates/cli-tools/src/fs.rs index 9f1f205f..0b67c163 100644 --- a/crates/cli-tools/src/fs.rs +++ b/crates/cli-tools/src/fs.rs @@ -15,12 +15,14 @@ //! Wrappers around `tokio::fs` with descriptive errors. use std::fs::Metadata; +use std::future::Future; use std::io::ErrorKind; use std::path::{Component, Path, PathBuf}; use anyhow::{Context, Result}; use serde::de::DeserializeOwned; use serde::Serialize; +use tokio::fs::{File, OpenOptions}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; pub async fn canonicalize(path: impl AsRef) -> Result { @@ -144,12 +146,62 @@ pub async fn touch(path: impl AsRef) -> Result<()> { write(path, "").await } -pub async fn write(path: impl AsRef, contents: impl AsRef<[u8]>) -> Result<()> { - let name = path.as_ref().display(); +pub struct WriteParams> { + path: P, + options: OpenOptions, +} + +impl> WriteParams

{ + pub fn new(path: P) -> Self { + WriteParams { path, options: OpenOptions::new() } + } + pub fn options(&mut self) -> &mut OpenOptions { + &mut self.options + } +} + +pub trait WriteFile { + fn path(&self) -> &Path; + fn open(self) -> impl Future>; +} + +impl> WriteFile for WriteParams

{ + fn path(&self) -> &Path { + self.path.as_ref() + } + async fn open(self) -> Result { + (&self).open().await + } +} + +impl> WriteFile for &WriteParams

{ + fn path(&self) -> &Path { + (*self).path() + } + async fn open(self) -> Result { + Ok(self.options.open(&self.path).await?) + } +} + +impl> WriteFile for P { + fn path(&self) -> &Path { + self.as_ref() + } + async fn open(self) -> Result { + let mut params = WriteParams::new(self); + params.options().write(true).create(true).truncate(true); + params.open().await + } +} + +pub async fn write(file: impl WriteFile, contents: impl AsRef<[u8]>) -> Result<()> { + let name = format!("{}", file.path().display()); let contents = contents.as_ref(); - create_parent(path.as_ref()).await?; + create_parent(file.path()).await?; debug!("write > {name:?}"); - tokio::fs::write(path.as_ref(), contents).await.with_context(|| format!("writing {name}"))?; + let mut file = file.open().await.with_context(|| format!("creating {name}"))?; + file.write_all(contents).await.with_context(|| format!("writing {name}"))?; + file.flush().await.with_context(|| format!("flushing {name}"))?; Ok(()) } diff --git a/crates/cli-tools/src/lib.rs b/crates/cli-tools/src/lib.rs index ae52d7f7..957676ff 100644 --- a/crates/cli-tools/src/lib.rs +++ b/crates/cli-tools/src/lib.rs @@ -24,13 +24,16 @@ macro_rules! debug { ($($x:tt)*) => { - print!("\x1b[1;36m"); - print!($($x)*); - println!("\x1b[m"); + if log::log_enabled!(log::Level::Warn) { + print!("\x1b[1;36m"); + print!($($x)*); + println!("\x1b[m"); + } }; } #[cfg(feature = "action")] pub mod action; pub mod cmd; +pub mod error; pub mod fs; diff --git a/crates/cli/CHANGELOG.md b/crates/cli/CHANGELOG.md index ace34e29..7318de10 100644 --- a/crates/cli/CHANGELOG.md +++ b/crates/cli/CHANGELOG.md @@ -9,6 +9,7 @@ ### Minor +- Support `RUST_LOG` to control logging - Add `platform-lock` to lock a platform protocol - Add `applet-exit-status` to get an applet exit status - Implement `applet-{install,uninstall}` for applet management diff --git a/crates/cli/Cargo.lock b/crates/cli/Cargo.lock index 6f7e26ea..14431d6a 100644 --- a/crates/cli/Cargo.lock +++ b/crates/cli/Cargo.lock @@ -17,6 +17,15 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" +[[package]] +name = "aho-corasick" +version = "1.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e60d3430d3a69478ad0993f19238d2df97c507009a52b3c10addcd7f6bcb916" +dependencies = [ + "memchr", +] + [[package]] name = "anstream" version = "0.6.14" @@ -239,6 +248,29 @@ version = "0.3.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f" +[[package]] +name = "env_filter" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f2c92ceda6ceec50f43169f9ee8424fe2db276791afde7b2cd8bc084cb376ab" +dependencies = [ + "log", + "regex", +] + +[[package]] +name = "env_logger" +version = "0.11.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e13fa619b91fb2381732789fc5de83b45675e882f66623b7d8cb4f643017018d" +dependencies = [ + "anstream", + "anstyle", + "env_filter", + "humantime", + "log", +] + [[package]] name = "equivalent" version = "1.0.1" @@ -488,6 +520,35 @@ dependencies = [ "bitflags", ] +[[package]] +name = "regex" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38200e5ee88914975b69f657f0801b6f6dccafd44fd9326302a4aaeecfacb1d8" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "368758f23274712b504848e9d5a6f010445cc8b87a7cdb4d7cbee666c1288da3" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" + [[package]] name = "rusb" version = "0.9.4" @@ -726,6 +787,7 @@ dependencies = [ "anyhow", "clap", "clap_complete", + "env_logger", "tokio", "wasefire-cli-tools", ] @@ -740,6 +802,7 @@ dependencies = [ "data-encoding", "humantime", "indicatif", + "log", "rusb", "serde", "tokio", diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index fc43528d..b7518146 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -19,6 +19,7 @@ path = "src/main.rs" anyhow = { version = "1.0.86", default-features = false } clap = { version = "4.5.4", default-features = false, features = ["default", "derive", "env"] } clap_complete = { version = "4.5.2", default-features = false } +env_logger = { version = "0.11.3", default-features = false, features = ["default"] } wasefire-cli-tools = { version = "0.2.0-git", path = "../cli-tools", features = ["action"] } [dependencies.tokio] diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 54e122cb..a4d44d0b 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -82,6 +82,7 @@ enum Action { #[command(flatten)] action: action::AppletRpc, }, + PlatformList(action::PlatformList), /// Prints the platform update metadata (possibly binary output). @@ -170,6 +171,7 @@ impl Completion { #[tokio::main] async fn main() -> Result<()> { + env_logger::init(); let flags = Flags::parse(); let dir = std::env::current_dir()?; match flags.action { diff --git a/crates/protocol-tokio/CHANGELOG.md b/crates/protocol-tokio/CHANGELOG.md index 979be374..ecd0fabd 100644 --- a/crates/protocol-tokio/CHANGELOG.md +++ b/crates/protocol-tokio/CHANGELOG.md @@ -2,4 +2,4 @@ ## 0.1.0-git - + diff --git a/crates/protocol-tokio/Cargo.toml b/crates/protocol-tokio/Cargo.toml index 605a999d..c5482aac 100644 --- a/crates/protocol-tokio/Cargo.toml +++ b/crates/protocol-tokio/Cargo.toml @@ -17,7 +17,7 @@ features = ["device"] [dependencies] anyhow = { version = "1.0.86", default-features = false, features = ["std"] } wasefire-error = { version = "0.1.2-git", path = "../error", optional = true } -wasefire-logger = { version = "0.1.6-git", path = "../logger" } +wasefire-logger = { version = "0.1.6-git", path = "../logger", optional = true } wasefire-one-of = { version = "0.1.0-git", path = "../one-of" } [dependencies.tokio] @@ -38,9 +38,9 @@ features = ["host"] optional = true [features] -log = ["wasefire-board-api?/log", "wasefire-logger/log"] +log = ["wasefire-board-api?/log", "wasefire-logger?/log"] # Exactly one of host or device must be selected. -device = ["dep:wasefire-board-api", "dep:wasefire-error"] +device = ["dep:wasefire-board-api", "dep:wasefire-error", "dep:wasefire-logger"] host = ["dep:wasefire-protocol"] [lints] diff --git a/crates/protocol-tokio/src/common.rs b/crates/protocol-tokio/src/common.rs index e7904bf6..74680fc3 100644 --- a/crates/protocol-tokio/src/common.rs +++ b/crates/protocol-tokio/src/common.rs @@ -14,49 +14,22 @@ //! Message format between device and host. -use std::io::ErrorKind; +use std::io::Result; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; -use wasefire_logger as log; -pub(crate) async fn read(stream: &mut S) -> Result, ()> { +pub(crate) async fn read(stream: &mut S) -> Result> { let mut length = [0; 4]; - read_exact(stream, &mut length).await?; + stream.read_exact(&mut length).await?; let length = u32::from_be_bytes(length); let mut message = vec![0; length as usize]; - read_exact(stream, &mut message).await?; + stream.read_exact(&mut message).await?; Ok(message.into_boxed_slice()) } -pub(crate) async fn write(stream: &mut S, message: &[u8]) -> Result<(), ()> { +pub(crate) async fn write(stream: &mut S, message: &[u8]) -> Result<()> { let length = message.len() as u32; - write_all(stream, &length.to_be_bytes()).await?; - write_all(stream, message).await?; + stream.write_all(&length.to_be_bytes()).await?; + stream.write_all(message).await?; Ok(()) } - -async fn read_exact(stream: &mut S, buffer: &mut [u8]) -> Result<(), ()> { - match stream.read_exact(buffer).await { - Ok(_) => Ok(()), - Err(x) => match x.kind() { - ErrorKind::NotConnected | ErrorKind::BrokenPipe | ErrorKind::UnexpectedEof => { - log::debug!("read_exact {:?}", x.kind()); - Err(()) - } - _ => panic!("{x}"), - }, - } -} - -async fn write_all(stream: &mut S, buffer: &[u8]) -> Result<(), ()> { - match stream.write_all(buffer).await { - Ok(_) => Ok(()), - Err(x) => match x.kind() { - ErrorKind::NotConnected | ErrorKind::BrokenPipe => { - log::debug!("write_all {:?}", x.kind()); - Err(()) - } - _ => panic!("{x}"), - }, - } -} diff --git a/crates/protocol-tokio/src/device.rs b/crates/protocol-tokio/src/device.rs index 6b01e3db..39bbed7c 100644 --- a/crates/protocol-tokio/src/device.rs +++ b/crates/protocol-tokio/src/device.rs @@ -101,7 +101,7 @@ impl Pipe { } match Self::manage_stream::(&mut stream, &shared, &push).await { Ok(()) => (), - Err(()) => match &mut *shared.state.lock().unwrap() { + Err(_) => match &mut *shared.state.lock().unwrap() { State::Disabled => (), x => *x = State::Accept, }, @@ -116,7 +116,7 @@ impl Pipe { async fn manage_stream( stream: &mut L::Stream, shared: &Shared, push: &P, - ) -> Result<(), ()> { + ) -> std::io::Result<()> { loop { enum Action { Receive, diff --git a/crates/protocol-tokio/src/host.rs b/crates/protocol-tokio/src/host.rs index ede4f3c4..0744578f 100644 --- a/crates/protocol-tokio/src/host.rs +++ b/crates/protocol-tokio/src/host.rs @@ -15,7 +15,7 @@ use std::net::SocketAddr; use std::path::Path; -use anyhow::{bail, Result}; +use anyhow::Result; use tokio::io::{AsyncRead, AsyncWrite}; use tokio::net::{TcpStream, UnixStream}; use wasefire_protocol::DynFuture; @@ -43,20 +43,10 @@ impl Connection { impl wasefire_protocol::Connection for Connection { fn read(&mut self) -> DynFuture> { - Box::pin(async move { - match read(&mut self.stream).await { - Ok(x) => Ok(x), - Err(()) => bail!("Error reading from the device."), - } - }) + Box::pin(async move { Ok(read(&mut self.stream).await?) }) } fn write<'a>(&'a mut self, request: &'a [u8]) -> DynFuture<'a, ()> { - Box::pin(async move { - match write(&mut self.stream, request).await { - Ok(()) => Ok(()), - Err(()) => bail!("Error writing to the device."), - } - }) + Box::pin(async move { Ok(write(&mut self.stream, request).await?) }) } } diff --git a/crates/protocol/crates/schema/Cargo.lock b/crates/protocol/crates/schema/Cargo.lock index 6f876a26..3e2ec8fa 100644 --- a/crates/protocol/crates/schema/Cargo.lock +++ b/crates/protocol/crates/schema/Cargo.lock @@ -118,6 +118,12 @@ dependencies = [ "scopeguard", ] +[[package]] +name = "log" +version = "0.4.22" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" + [[package]] name = "memchr" version = "2.7.2" @@ -393,6 +399,7 @@ name = "wasefire-cli-tools" version = "0.2.0-git" dependencies = [ "anyhow", + "log", "serde", "tokio", "toml", diff --git a/crates/runner-host/Cargo.lock b/crates/runner-host/Cargo.lock index 1d8f04bf..1da98178 100644 --- a/crates/runner-host/Cargo.lock +++ b/crates/runner-host/Cargo.lock @@ -1859,6 +1859,7 @@ name = "wasefire-cli-tools" version = "0.2.0-git" dependencies = [ "anyhow", + "log", "serde", "tokio", "toml", diff --git a/crates/xtask/Cargo.lock b/crates/xtask/Cargo.lock index a43e3e95..015177b1 100644 --- a/crates/xtask/Cargo.lock +++ b/crates/xtask/Cargo.lock @@ -1851,6 +1851,7 @@ dependencies = [ "data-encoding", "humantime", "indicatif", + "log", "rusb", "serde", "tokio", diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index 151419ba..9cf5e39c 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -16,7 +16,6 @@ use std::cmp::Reverse; use std::collections::BinaryHeap; -use std::error::Error; use std::ffi::OsString; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -28,6 +27,7 @@ use probe_rs::{flashing, Permissions, Session}; use rustc_demangle::demangle; use tokio::process::Command; use tokio::sync::OnceCell; +use wasefire_cli_tools::error::root_cause_is; use wasefire_cli_tools::{action, cmd, fs}; mod footprint; @@ -708,12 +708,6 @@ impl Wait { } } -fn root_cause_is( - error: &anyhow::Error, predicate: impl FnOnce(&E) -> bool, -) -> bool { - error.root_cause().downcast_ref::().map_or(false, predicate) -} - async fn ensure_command(cmd: &[&str]) -> Result<()> { let mut wrapper = Command::new("./scripts/wrapper.sh"); wrapper.args(cmd);