From 3e95bff949c5fd5507a354ed1884fd0e77018ccc Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Fri, 9 Jun 2023 15:26:11 +0800 Subject: [PATCH 01/10] Disable logging unless the $RUST_LOG variable is set --- Cargo.lock | 1 - lib/cli/Cargo.toml | 1 - lib/cli/src/cli.rs | 2 ++ lib/cli/src/commands/run.rs | 5 ----- lib/cli/src/logging.rs | 20 ++++---------------- 5 files changed, 6 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3f14cdd5c7b..ecfc25c73c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5640,7 +5640,6 @@ dependencies = [ "cfg-if 1.0.0", "chrono", "clap 4.3.4", - "clap-verbosity-flag", "colored 2.0.0", "dialoguer", "dirs", diff --git a/lib/cli/Cargo.toml b/lib/cli/Cargo.toml index 061f52667da..d8674d5bbf9 100644 --- a/lib/cli/Cargo.toml +++ b/lib/cli/Cargo.toml @@ -91,7 +91,6 @@ object = "0.30.0" wasm-coredump-builder = { version = "0.1.11", optional = true } tracing = { version = "0.1" } tracing-subscriber = { version = "0.3", features = [ "env-filter", "fmt" ] } -clap-verbosity-flag = "2" async-trait = "0.1.68" tokio = { version = "1.28.1", features = ["macros", "rt-multi-thread"] } once_cell = "1.17.1" diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index 2c88d6fbeff..a31683fd5f0 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -223,6 +223,8 @@ pub fn wasmer_main() { } fn wasmer_main_inner() -> Result<(), anyhow::Error> { + crate::logging::set_up_logging(); + // We try to run wasmer with the normal arguments. // Eg. `wasmer ` // In case that fails, we fallback trying the Run subcommand directly. diff --git a/lib/cli/src/commands/run.rs b/lib/cli/src/commands/run.rs index 85505b18da1..f2dea753a1d 100644 --- a/lib/cli/src/commands/run.rs +++ b/lib/cli/src/commands/run.rs @@ -16,7 +16,6 @@ use std::{ use anyhow::{Context, Error}; use clap::Parser; -use clap_verbosity_flag::WarnLevel; use once_cell::sync::Lazy; use sha2::{Digest, Sha256}; use tempfile::NamedTempFile; @@ -58,8 +57,6 @@ static WASMER_HOME: Lazy = Lazy::new(|| { /// The unstable `wasmer run` subcommand. #[derive(Debug, Parser)] pub struct Run { - #[clap(flatten)] - verbosity: clap_verbosity_flag::Verbosity, /// The Wasmer home directory. #[clap(long = "wasmer-dir", env = "WASMER_DIR", default_value = WASMER_HOME.as_os_str())] wasmer_dir: PathBuf, @@ -92,7 +89,6 @@ impl Run { } fn execute_inner(&self) -> Result<(), Error> { - crate::logging::set_up_logging(self.verbosity.log_level_filter()); let runtime = tokio::runtime::Builder::new_multi_thread() .enable_all() .build()?; @@ -360,7 +356,6 @@ impl Run { }; let store = StoreOptions::default(); Ok(Run { - verbosity: clap_verbosity_flag::Verbosity::new(0, 0), wasmer_dir: WASMER_HOME.clone(), store, wasi: Wasi::for_binfmt_interpreter()?, diff --git a/lib/cli/src/logging.rs b/lib/cli/src/logging.rs index 54ce76c6275..987c3fa6446 100644 --- a/lib/cli/src/logging.rs +++ b/lib/cli/src/logging.rs @@ -1,11 +1,12 @@ //! Logging functions for the debug feature. use tracing_subscriber::{ - filter::Directive, fmt, layer::SubscriberExt, util::SubscriberInitExt, EnvFilter, + fmt, layer::SubscriberExt, util::SubscriberInitExt, EnvFilter, }; -/// Subroutine to instantiate the loggers -pub fn set_up_logging(level: log::LevelFilter) { +/// Initialize logging based on the `$RUST_LOG` environment variable. Logs will +/// be disabled when `$RUST_LOG` isn't set. +pub fn set_up_logging() { let fmt_layer = fmt::layer() .with_target(true) .with_span_events(fmt::format::FmtSpan::CLOSE) @@ -15,7 +16,6 @@ pub fn set_up_logging(level: log::LevelFilter) { .compact(); let filter_layer = EnvFilter::builder() - .with_default_directive(log_directive(level)) .from_env_lossy(); tracing_subscriber::registry() @@ -34,15 +34,3 @@ fn should_emit_colors() -> bool { isatty::stderr_isatty() && std::env::var_os("NO_COLOR").is_none() } -fn log_directive(level: log::LevelFilter) -> Directive { - let tracing_level = match level { - log::LevelFilter::Off => tracing::level_filters::LevelFilter::OFF, - log::LevelFilter::Error => tracing::level_filters::LevelFilter::ERROR, - log::LevelFilter::Warn => tracing::level_filters::LevelFilter::WARN, - log::LevelFilter::Info => tracing::level_filters::LevelFilter::INFO, - log::LevelFilter::Debug => tracing::level_filters::LevelFilter::DEBUG, - log::LevelFilter::Trace => tracing::level_filters::LevelFilter::TRACE, - }; - - tracing_level.into() -} From c2c19df75e285065293520171b09b5592c257ba9 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Fri, 9 Jun 2023 15:30:06 +0800 Subject: [PATCH 02/10] Rustfmt --- lib/cli/src/logging.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/cli/src/logging.rs b/lib/cli/src/logging.rs index 987c3fa6446..77442fc5040 100644 --- a/lib/cli/src/logging.rs +++ b/lib/cli/src/logging.rs @@ -1,8 +1,6 @@ //! Logging functions for the debug feature. -use tracing_subscriber::{ - fmt, layer::SubscriberExt, util::SubscriberInitExt, EnvFilter, -}; +use tracing_subscriber::{fmt, layer::SubscriberExt, util::SubscriberInitExt, EnvFilter}; /// Initialize logging based on the `$RUST_LOG` environment variable. Logs will /// be disabled when `$RUST_LOG` isn't set. @@ -15,8 +13,7 @@ pub fn set_up_logging() { .with_writer(std::io::stderr) .compact(); - let filter_layer = EnvFilter::builder() - .from_env_lossy(); + let filter_layer = EnvFilter::builder().from_env_lossy(); tracing_subscriber::registry() .with(filter_layer) @@ -33,4 +30,3 @@ pub fn set_up_logging() { fn should_emit_colors() -> bool { isatty::stderr_isatty() && std::env::var_os("NO_COLOR").is_none() } - From 1953c0a57c248e93321aaa6a7b640ccf65185f58 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Wed, 14 Jun 2023 22:10:16 +0800 Subject: [PATCH 03/10] Added a logging abstraction and cleaned up the main argument parsing logic --- lib/cli/src/cli.rs | 208 +++++++++++++++++------------------------ lib/cli/src/logging.rs | 114 +++++++++++++++++----- 2 files changed, 177 insertions(+), 145 deletions(-) diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index a31683fd5f0..7a227596ea1 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -14,29 +14,65 @@ use crate::commands::{ #[cfg(feature = "static-artifact-create")] use crate::commands::{CreateObj, GenCHeader}; use crate::error::PrettyError; -use clap::{error::ErrorKind, CommandFactory, Parser}; +use clap::{CommandFactory, Parser}; + +/// The main function for the Wasmer CLI tool. +pub fn wasmer_main() { + // We allow windows to print properly colors + #[cfg(windows)] + colored::control::set_virtual_terminal(true).unwrap(); + + PrettyError::report(wasmer_main_inner()) +} + +fn wasmer_main_inner() -> Result<(), anyhow::Error> { + if is_binfmt_interpreter() { + todo!(); + } + + let args = Args::parse(); + + if args.version { + return print_version(args.output.is_verbose()); + } + + args.execute() +} + +/// Command-line arguments for the Wasmer CLI. +#[derive(Parser, Debug)] +#[clap(about, author)] +#[cfg_attr(feature = "headless", clap(name = "wasmer-headless"))] +#[cfg_attr(not(feature = "headless"), clap(name = "wasmer-headless"))] +pub struct Args { + /// Print version info and exit. + #[clap(short = 'V', long)] + version: bool, + #[clap(flatten)] + output: crate::logging::Output, + #[clap(subcommand)] + cmd: Option, +} + +impl Args { + fn execute(self) -> Result<(), anyhow::Error> { + if self.version { + return print_version(self.output.is_verbose()); + } + + if let Some(cmd) = self.cmd { + cmd.execute() + } else { + let help = Args::command().render_long_help(); + eprintln!("{help}"); + Ok(()) + } + } +} #[derive(Parser, Debug)] -#[cfg_attr( - not(feature = "headless"), - clap( - name = "wasmer", - about = concat!("wasmer ", env!("CARGO_PKG_VERSION")), - version, - author - ) -)] -#[cfg_attr( - feature = "headless", - clap( - name = "wasmer-headless", - about = concat!("wasmer ", env!("CARGO_PKG_VERSION")), - version, - author - ) -)] /// The options for the wasmer Command Line Interface -enum WasmerCLIOptions { +enum Cmd { /// Login into a wasmer.io-like registry Login(Login), @@ -175,7 +211,7 @@ enum WasmerCLIOptions { Namespace(wasmer_deploy_cli::cmd::namespace::CmdNamespace), } -impl WasmerCLIOptions { +impl Cmd { fn execute(self) -> Result<(), anyhow::Error> { use wasmer_deploy_cli::cmd::CliCommand; @@ -213,113 +249,45 @@ impl WasmerCLIOptions { } } -/// The main function for the Wasmer CLI tool. -pub fn wasmer_main() { - // We allow windows to print properly colors - #[cfg(windows)] - colored::control::set_virtual_terminal(true).unwrap(); - - PrettyError::report(wasmer_main_inner()) -} - -fn wasmer_main_inner() -> Result<(), anyhow::Error> { - crate::logging::set_up_logging(); - - // We try to run wasmer with the normal arguments. - // Eg. `wasmer ` - // In case that fails, we fallback trying the Run subcommand directly. - // Eg. `wasmer myfile.wasm --dir=.` - // - // In case we've been run as wasmer-binfmt-interpreter myfile.wasm args, - // we assume that we're registered via binfmt_misc - let args = std::env::args().collect::>(); - let binpath = args.get(0).map(|s| s.as_ref()).unwrap_or(""); - - let firstarg = args.get(1).map(|s| s.as_str()); - let secondarg = args.get(2).map(|s| s.as_str()); - - match (firstarg, secondarg) { - (None, _) | (Some("help"), _) | (Some("--help"), _) => { - return print_help(true); - } - (Some("-h"), _) => { - return print_help(false); - } - (Some("-vV"), _) - | (Some("version"), Some("--verbose")) - | (Some("--version"), Some("--verbose")) => { - return print_version(true); - } - - (Some("-v"), _) | (Some("-V"), _) | (Some("version"), _) | (Some("--version"), _) => { - return print_version(false); - } - _ => {} +fn is_binfmt_interpreter() -> bool { + if !cfg!(linux) { + return false; } - let command = args.get(1); - let options = if cfg!(target_os = "linux") && binpath.ends_with("wasmer-binfmt-interpreter") { - WasmerCLIOptions::Run(Run::from_binfmt_args()) - } else { - match command.unwrap_or(&String::new()).as_ref() { - "add" | "app" | "apps" | "binfmt" | "cache" | "compile" | "config" | "create-obj" - | "create-exe" | "deploy" | "help" | "gen-c-header" | "inspect" | "init" | "login" - | "namespace" | "namespaces" | "publish" | "run" | "run-unstable" | "self-update" - | "validate" | "wast" | "ssh" | "" => WasmerCLIOptions::parse(), - _ => { - WasmerCLIOptions::try_parse_from(args.iter()).unwrap_or_else(|e| { - match e.kind() { - // This fixes a issue that: - // 1. Shows the version twice when doing `wasmer -V` - // 2. Shows the run help (instead of normal help) when doing `wasmer --help` - ErrorKind::DisplayVersion | ErrorKind::DisplayHelp => e.exit(), - _ => WasmerCLIOptions::Run(Run::parse()), - } - }) - } - } - }; - - options.execute() -} - -fn print_help(verbose: bool) -> Result<(), anyhow::Error> { - let mut cmd = WasmerCLIOptions::command(); - if verbose { - let _ = cmd.print_long_help(); - } else { - let _ = cmd.print_help(); - } - Ok(()) + // TODO: Figure out if we actually are running as the binfmt interpreter + false } #[allow(unused_mut, clippy::vec_init_then_push)] fn print_version(verbose: bool) -> Result<(), anyhow::Error> { if !verbose { println!("wasmer {}", env!("CARGO_PKG_VERSION")); - } else { - println!( - "wasmer {} ({} {})", - env!("CARGO_PKG_VERSION"), - env!("WASMER_BUILD_GIT_HASH_SHORT"), - env!("WASMER_BUILD_DATE") - ); - println!("binary: {}", env!("CARGO_PKG_NAME")); - println!("commit-hash: {}", env!("WASMER_BUILD_GIT_HASH")); - println!("commit-date: {}", env!("WASMER_BUILD_DATE")); - println!("host: {}", target_lexicon::HOST); - println!("compiler: {}", { - let mut s = Vec::<&'static str>::new(); - - #[cfg(feature = "singlepass")] - s.push("singlepass"); - #[cfg(feature = "cranelift")] - s.push("cranelift"); - #[cfg(feature = "llvm")] - s.push("llvm"); - - s.join(",") - }); + return Ok(()); } + + println!( + "wasmer {} ({} {})", + env!("CARGO_PKG_VERSION"), + env!("WASMER_BUILD_GIT_HASH_SHORT"), + env!("WASMER_BUILD_DATE") + ); + println!("binary: {}", env!("CARGO_PKG_NAME")); + println!("commit-hash: {}", env!("WASMER_BUILD_GIT_HASH")); + println!("commit-date: {}", env!("WASMER_BUILD_DATE")); + println!("host: {}", target_lexicon::HOST); + + println!("compiler: {}", { + let mut s = Vec::<&'static str>::new(); + + #[cfg(feature = "singlepass")] + s.push("singlepass"); + #[cfg(feature = "cranelift")] + s.push("cranelift"); + #[cfg(feature = "llvm")] + s.push("llvm"); + + s.join(",") + }); + Ok(()) } diff --git a/lib/cli/src/logging.rs b/lib/cli/src/logging.rs index 77442fc5040..72e80a538a8 100644 --- a/lib/cli/src/logging.rs +++ b/lib/cli/src/logging.rs @@ -1,32 +1,96 @@ //! Logging functions for the debug feature. +use tracing::level_filters::LevelFilter; use tracing_subscriber::{fmt, layer::SubscriberExt, util::SubscriberInitExt, EnvFilter}; -/// Initialize logging based on the `$RUST_LOG` environment variable. Logs will -/// be disabled when `$RUST_LOG` isn't set. -pub fn set_up_logging() { - let fmt_layer = fmt::layer() - .with_target(true) - .with_span_events(fmt::format::FmtSpan::CLOSE) - .with_ansi(should_emit_colors()) - .with_thread_ids(true) - .with_writer(std::io::stderr) - .compact(); - - let filter_layer = EnvFilter::builder().from_env_lossy(); - - tracing_subscriber::registry() - .with(filter_layer) - .with(fmt_layer) - .init(); +const WHITELISTED_LOG_TARGETS: &[&str] = &["wasmer", "wasmer_wasix", "virtual_fs"]; + +/// Control the output generated by the CLI. +#[derive(Debug, Clone, PartialEq, clap::Parser)] +pub struct Output { + /// Generate verbose output (repeat for more verbosity) + #[clap(short, long, action = clap::ArgAction::Count, global = true, conflicts_with = "quiet")] + pub verbose: usize, + /// Do not print progress messages. + #[clap(short, long, global = true, conflicts_with = "verbose")] + pub quiet: bool, + /// When to display colored output. + #[clap(long, default_value_t = clap::ColorChoice::Auto, global = true)] + pub color: clap::ColorChoice, } -/// Check whether we should emit ANSI escape codes for log formatting. -/// -/// The `tracing-subscriber` crate doesn't have native support for -/// "--color=always|never|auto", so we implement a poor man's version. -/// -/// For more, see https://github.com/tokio-rs/tracing/issues/2388 -fn should_emit_colors() -> bool { - isatty::stderr_isatty() && std::env::var_os("NO_COLOR").is_none() +impl Output { + /// Has the `--verbose` flag been set? + pub fn is_verbose(&self) -> bool { + self.verbose > 0 + } + + /// Initialize logging based on the `$RUST_LOG` environment variable and + /// command-line flags. + pub fn initialize_logging(&self) { + let fmt_layer = fmt::layer() + .with_target(true) + .with_span_events(fmt::format::FmtSpan::CLOSE) + .with_ansi(self.should_emit_colors()) + .with_thread_ids(true) + .with_writer(std::io::stderr) + .compact(); + + let filter_layer = self.log_filter(); + + tracing_subscriber::registry() + .with(filter_layer) + .with(fmt_layer) + .init(); + } + + fn log_filter(&self) -> EnvFilter { + let default_filters = [ + LevelFilter::OFF, + LevelFilter::WARN, + LevelFilter::INFO, + LevelFilter::DEBUG, + ]; + + // First, we set up the default log level. + let default_level = default_filters + .get(self.verbose) + .copied() + .unwrap_or(LevelFilter::TRACE); + let mut filter = EnvFilter::builder() + .with_default_directive(default_level.into()) + .from_env_lossy(); + + // Next we add level-specific directives, where verbosity=0 means don't + // override anything. Note that these are shifted one level up so we'll + // get something like RUST_LOG="warn,wasmer_wasix=info" + let specific_filters = [LevelFilter::WARN, LevelFilter::INFO, LevelFilter::DEBUG]; + if self.verbose > 0 { + let level = specific_filters + .get(self.verbose) + .copied() + .unwrap_or(LevelFilter::TRACE); + + for target in WHITELISTED_LOG_TARGETS { + let directive = format!("{target}={level}").parse().unwrap(); + filter = filter.add_directive(directive); + } + } + + filter + } + + /// Check whether we should emit ANSI escape codes for log formatting. + /// + /// The `tracing-subscriber` crate doesn't have native support for + /// "--color=always|never|auto", so we implement a poor man's version. + /// + /// For more, see https://github.com/tokio-rs/tracing/issues/2388 + fn should_emit_colors(&self) -> bool { + match self.color { + clap::ColorChoice::Auto => isatty::stderr_isatty(), + clap::ColorChoice::Always => true, + clap::ColorChoice::Never => false, + } + } } From cfe4e09def232bd5b8ff86fd9ae612237ecfeb8d Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 15 Jun 2023 17:19:58 +0800 Subject: [PATCH 04/10] Implement binfmt detection --- lib/cli/src/cli.rs | 68 +++++++++++++++++++--------------- lib/cli/src/commands/binfmt.rs | 4 +- 2 files changed, 41 insertions(+), 31 deletions(-) diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index 7a227596ea1..5268eb9b9d4 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -27,15 +27,11 @@ pub fn wasmer_main() { fn wasmer_main_inner() -> Result<(), anyhow::Error> { if is_binfmt_interpreter() { - todo!(); + Run::from_binfmt_args().execute(); } let args = Args::parse(); - - if args.version { - return print_version(args.output.is_verbose()); - } - + args.output.initialize_logging(); args.execute() } @@ -56,16 +52,22 @@ pub struct Args { impl Args { fn execute(self) -> Result<(), anyhow::Error> { - if self.version { - return print_version(self.output.is_verbose()); + let Args { + cmd, + version, + output, + } = self; + + if version { + return print_version(output.is_verbose()); } - if let Some(cmd) = self.cmd { + if let Some(cmd) = cmd { cmd.execute() } else { - let help = Args::command().render_long_help(); - eprintln!("{help}"); - Ok(()) + Args::command().print_long_help()?; + // Note: clap uses an exit code of 2 when CLI parsing fails + std::process::exit(2); } } } @@ -250,15 +252,22 @@ impl Cmd { } fn is_binfmt_interpreter() -> bool { - if !cfg!(linux) { - return false; + cfg_if::cfg_if! { + if #[cfg(target_os = "linux")] { + use std::{ffi::OsStr, path::PathBuf}; + + // Note: we'll be invoked as + let binary_path = match std::env::args_os().next() { + Some(path) => PathBuf::from(path), + None => return false, + }; + binary_path.file_name() == OsStr::from(Binfmt::FILENAME) + } else { + false + } } - - // TODO: Figure out if we actually are running as the binfmt interpreter - false } -#[allow(unused_mut, clippy::vec_init_then_push)] fn print_version(verbose: bool) -> Result<(), anyhow::Error> { if !verbose { println!("wasmer {}", env!("CARGO_PKG_VERSION")); @@ -276,18 +285,17 @@ fn print_version(verbose: bool) -> Result<(), anyhow::Error> { println!("commit-date: {}", env!("WASMER_BUILD_DATE")); println!("host: {}", target_lexicon::HOST); - println!("compiler: {}", { - let mut s = Vec::<&'static str>::new(); - - #[cfg(feature = "singlepass")] - s.push("singlepass"); - #[cfg(feature = "cranelift")] - s.push("cranelift"); - #[cfg(feature = "llvm")] - s.push("llvm"); - - s.join(",") - }); + let mut compilers = Vec::<&'static str>::new(); + if cfg!(feature = "singlepass") { + compilers.push("singlepass"); + } + if cfg!(feature = "cranelift") { + compilers.push("cranelift"); + } + if cfg!(feature = "llvm") { + compilers.push("llvm"); + } + println!("compiler: {}", compilers.join(",")); Ok(()) } diff --git a/lib/cli/src/commands/binfmt.rs b/lib/cli/src/commands/binfmt.rs index 3c636fa7678..fd069656e16 100644 --- a/lib/cli/src/commands/binfmt.rs +++ b/lib/cli/src/commands/binfmt.rs @@ -53,6 +53,8 @@ fn seccheck(path: &Path) -> Result<()> { } impl Binfmt { + pub const FILENAME: &str = "wasmer-binfmt-interpreter"; + /// execute [Binfmt] pub fn execute(&self) -> Result<()> { if !self.binfmt_misc.exists() { @@ -66,7 +68,7 @@ impl Binfmt { let bin_path_orig: PathBuf = env::current_exe() .and_then(|p| p.canonicalize()) .context("Cannot get path to wasmer executable")?; - let bin_path = temp_dir.path().join("wasmer-binfmt-interpreter"); + let bin_path = temp_dir.path().join(Binfmt::FILENAME); fs::copy(bin_path_orig, &bin_path).context("Copy wasmer binary to temp folder")?; let bin_path = fs::canonicalize(&bin_path).with_context(|| { format!( From 66515fb47ad83caa7bf88355bfdb3eaf4a213b6e Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 15 Jun 2023 18:11:42 +0800 Subject: [PATCH 05/10] Allow "wasmer some/package" to work again --- lib/cli/src/cli.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index 5268eb9b9d4..dd930ee673e 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -30,9 +30,19 @@ fn wasmer_main_inner() -> Result<(), anyhow::Error> { Run::from_binfmt_args().execute(); } - let args = Args::parse(); - args.output.initialize_logging(); - args.execute() + match Args::try_parse() { + Ok(args) => { + args.output.initialize_logging(); + args.execute() + } + Err(e) if e.kind() == clap::error::ErrorKind::InvalidSubcommand => { + // Try to parse it as `wasmer some/package` + Run::parse().execute() + } + Err(e) => { + e.exit(); + } + } } /// Command-line arguments for the Wasmer CLI. From d3b9fb4338d090a11fa4e8331eca28cfbac108cd Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 15 Jun 2023 18:13:15 +0800 Subject: [PATCH 06/10] Fixed a compile error on Linux --- lib/cli/src/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index dd930ee673e..a58cfe66a12 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -271,7 +271,7 @@ fn is_binfmt_interpreter() -> bool { Some(path) => PathBuf::from(path), None => return false, }; - binary_path.file_name() == OsStr::from(Binfmt::FILENAME) + binary_path.file_name().and_then(|f| f.to_str()) == Some(Binfmt::FILENAME) } else { false } From 52546b2e188aae9dfe06d6dd23e341a9f60e0b2c Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 15 Jun 2023 20:22:40 +0800 Subject: [PATCH 07/10] Clippy lints --- lib/cli/src/cli.rs | 6 ++---- lib/cli/src/commands/binfmt.rs | 1 + 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index a58cfe66a12..4def1071edd 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -264,11 +264,9 @@ impl Cmd { fn is_binfmt_interpreter() -> bool { cfg_if::cfg_if! { if #[cfg(target_os = "linux")] { - use std::{ffi::OsStr, path::PathBuf}; - - // Note: we'll be invoked as + // Note: we'll be invoked by the kernel as Binfmt::FILENAME let binary_path = match std::env::args_os().next() { - Some(path) => PathBuf::from(path), + Some(path) => std::path::PathBuf::from(path), None => return false, }; binary_path.file_name().and_then(|f| f.to_str()) == Some(Binfmt::FILENAME) diff --git a/lib/cli/src/commands/binfmt.rs b/lib/cli/src/commands/binfmt.rs index fd069656e16..a4b7a3c6b61 100644 --- a/lib/cli/src/commands/binfmt.rs +++ b/lib/cli/src/commands/binfmt.rs @@ -53,6 +53,7 @@ fn seccheck(path: &Path) -> Result<()> { } impl Binfmt { + /// The filename used to register the wasmer CLI as a binfmt interpreter. pub const FILENAME: &str = "wasmer-binfmt-interpreter"; /// execute [Binfmt] From 2de472179cb6b603ce7e8bd51132fd82f77ab165 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 15 Jun 2023 22:04:28 +0800 Subject: [PATCH 08/10] Enable logging for "wasmer some/package" --- lib/cli/src/cli.rs | 1 + lib/cli/src/logging.rs | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index 4def1071edd..833fa04a094 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -37,6 +37,7 @@ fn wasmer_main_inner() -> Result<(), anyhow::Error> { } Err(e) if e.kind() == clap::error::ErrorKind::InvalidSubcommand => { // Try to parse it as `wasmer some/package` + crate::logging::Output::default().initialize_logging(); Run::parse().execute() } Err(e) => { diff --git a/lib/cli/src/logging.rs b/lib/cli/src/logging.rs index 72e80a538a8..de30b7f5452 100644 --- a/lib/cli/src/logging.rs +++ b/lib/cli/src/logging.rs @@ -6,11 +6,11 @@ use tracing_subscriber::{fmt, layer::SubscriberExt, util::SubscriberInitExt, Env const WHITELISTED_LOG_TARGETS: &[&str] = &["wasmer", "wasmer_wasix", "virtual_fs"]; /// Control the output generated by the CLI. -#[derive(Debug, Clone, PartialEq, clap::Parser)] +#[derive(Debug, Default, Clone, PartialEq, clap::Parser)] pub struct Output { /// Generate verbose output (repeat for more verbosity) #[clap(short, long, action = clap::ArgAction::Count, global = true, conflicts_with = "quiet")] - pub verbose: usize, + pub verbose: u8, /// Do not print progress messages. #[clap(short, long, global = true, conflicts_with = "verbose")] pub quiet: bool, @@ -54,7 +54,7 @@ impl Output { // First, we set up the default log level. let default_level = default_filters - .get(self.verbose) + .get(self.verbose as usize) .copied() .unwrap_or(LevelFilter::TRACE); let mut filter = EnvFilter::builder() @@ -67,7 +67,7 @@ impl Output { let specific_filters = [LevelFilter::WARN, LevelFilter::INFO, LevelFilter::DEBUG]; if self.verbose > 0 { let level = specific_filters - .get(self.verbose) + .get(self.verbose as usize) .copied() .unwrap_or(LevelFilter::TRACE); From d164d2cb0ffcd74658d9910a22da7137a5d44976 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Fri, 16 Jun 2023 16:28:39 +0800 Subject: [PATCH 09/10] Moved the version test to the wasmer-cli's tests/ directory --- Cargo.lock | 5 ++ lib/cli/Cargo.toml | 89 ++++++-------------------- lib/cli/src/cli.rs | 29 ++++++--- lib/cli/tests/version.rs | 68 ++++++++++++++++++++ tests/integration/cli/tests/version.rs | 64 ------------------ 5 files changed, 113 insertions(+), 142 deletions(-) create mode 100644 lib/cli/tests/version.rs delete mode 100644 tests/integration/cli/tests/version.rs diff --git a/Cargo.lock b/Cargo.lock index ecfc25c73c1..13063f9911c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3121,8 +3121,11 @@ checksum = "09963355b9f467184c04017ced4a2ba2d75cbcb4e7462690d388233253d4b1a9" dependencies = [ "anstyle", "difflib", + "float-cmp", "itertools", + "normalize-line-endings", "predicates-core", + "regex", ] [[package]] @@ -5632,6 +5635,7 @@ name = "wasmer-cli" version = "4.0.0-beta.3" dependencies = [ "anyhow", + "assert_cmd 2.0.11", "async-trait", "atty", "bytes", @@ -5653,6 +5657,7 @@ dependencies = [ "object 0.30.4", "once_cell", "pathdiff", + "predicates 3.0.3", "prettytable-rs", "regex", "reqwest", diff --git a/lib/cli/Cargo.toml b/lib/cli/Cargo.toml index d8674d5bbf9..9c5e00c8556 100644 --- a/lib/cli/Cargo.toml +++ b/lib/cli/Cargo.toml @@ -125,84 +125,33 @@ unix_mode = "0.1.3" [features] # Don't add the compiler features in default, please add them on the Makefile # since we might want to autoconfigure them depending on the availability on the host. -default = [ - "sys", - "wat", - "wast", - "compiler", - "wasmer-artifact-create", - "static-artifact-create", -] +default = ["sys", "wat", "wast", "compiler", "wasmer-artifact-create", "static-artifact-create"] backend = [] -coredump = [ - "wasm-coredump-builder", -] -sys = [ - "compiler", - "wasmer-vm", -] -jsc = [ - "backend", - "wasmer/jsc", - "wasmer/std" -] +coredump = ["wasm-coredump-builder"] +sys = ["compiler", "wasmer-vm"] +jsc = ["backend", "wasmer/jsc", "wasmer/std"] wast = ["wasmer-wast"] -host-net = [ "virtual-net/host-net" ] +host-net = ["virtual-net/host-net"] wat = ["wasmer/wat"] -compiler = [ - "backend", - "wasmer/compiler", - "wasmer-compiler/translator", - "wasmer-compiler/compiler" -] -wasmer-artifact-create = ["compiler", - "wasmer/wasmer-artifact-load", - "wasmer/wasmer-artifact-create", - "wasmer-compiler/wasmer-artifact-load", - "wasmer-compiler/wasmer-artifact-create", - "wasmer-object", - ] -static-artifact-create = ["compiler", - "wasmer/static-artifact-load", - "wasmer/static-artifact-create", - "wasmer-compiler/static-artifact-load", - "wasmer-compiler/static-artifact-create", - "wasmer-object", - ] -wasmer-artifact-load = ["compiler", - "wasmer/wasmer-artifact-load", - "wasmer-compiler/wasmer-artifact-load", - ] -static-artifact-load = ["compiler", - "wasmer/static-artifact-load", - "wasmer-compiler/static-artifact-load", - ] -experimental-io-devices = [ - "wasmer-wasix-experimental-io-devices", -] -singlepass = [ - "wasmer-compiler-singlepass", - "compiler", -] -cranelift = [ - "wasmer-compiler-cranelift", - "compiler", -] -llvm = [ - "wasmer-compiler-llvm", - "compiler", -] +compiler = ["backend", "wasmer/compiler", "wasmer-compiler/translator", "wasmer-compiler/compiler"] +wasmer-artifact-create = ["compiler", "wasmer/wasmer-artifact-load", "wasmer/wasmer-artifact-create", "wasmer-compiler/wasmer-artifact-load", "wasmer-compiler/wasmer-artifact-create", "wasmer-object"] +static-artifact-create = ["compiler", "wasmer/static-artifact-load", "wasmer/static-artifact-create", "wasmer-compiler/static-artifact-load", "wasmer-compiler/static-artifact-create", "wasmer-object"] +wasmer-artifact-load = ["compiler", "wasmer/wasmer-artifact-load", "wasmer-compiler/wasmer-artifact-load"] +static-artifact-load = ["compiler", "wasmer/static-artifact-load", "wasmer-compiler/static-artifact-load"] +experimental-io-devices = ["wasmer-wasix-experimental-io-devices"] +singlepass = ["wasmer-compiler-singlepass", "compiler"] +cranelift = ["wasmer-compiler-cranelift", "compiler"] +llvm = ["wasmer-compiler-llvm", "compiler"] disable-all-logging = ["wasmer-wasix/disable-all-logging", "log/release_max_level_off"] headless = [] headless-minimal = ["headless", "disable-all-logging"] # Optional -enable-serde = [ - "wasmer/enable-serde", - "wasmer-vm/enable-serde", - "wasmer-compiler/enable-serde", - "wasmer-wasix/enable-serde", -] +enable-serde = ["wasmer/enable-serde", "wasmer-vm/enable-serde", "wasmer-compiler/enable-serde", "wasmer-wasix/enable-serde"] + +[dev-dependencies] +assert_cmd = "2.0.11" +predicates = "3.0.3" [target.'cfg(target_os = "windows")'.dependencies] colored = "2.0.0" diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index 833fa04a094..fcc3bd9ae67 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -35,12 +35,18 @@ fn wasmer_main_inner() -> Result<(), anyhow::Error> { args.output.initialize_logging(); args.execute() } - Err(e) if e.kind() == clap::error::ErrorKind::InvalidSubcommand => { - // Try to parse it as `wasmer some/package` - crate::logging::Output::default().initialize_logging(); - Run::parse().execute() - } Err(e) => { + if e.kind() == clap::error::ErrorKind::InvalidSubcommand { + if let Ok(run) = Run::try_parse() { + // Try to parse the command using the `wasmer some/package` + // shorthand. Note that this has discoverability issues + // because it's not shown as part of the main argument + // parser's help, but that's fine. + crate::logging::Output::default().initialize_logging(); + run.execute(); + } + } + e.exit(); } } @@ -48,9 +54,16 @@ fn wasmer_main_inner() -> Result<(), anyhow::Error> { /// Command-line arguments for the Wasmer CLI. #[derive(Parser, Debug)] -#[clap(about, author)] -#[cfg_attr(feature = "headless", clap(name = "wasmer-headless"))] -#[cfg_attr(not(feature = "headless"), clap(name = "wasmer-headless"))] +#[clap(author, version)] +#[clap(disable_version_flag = true)] // handled manually +#[cfg_attr(feature = "headless", clap( + name = "wasmer-headless", + about = concat!("wasmer-headless ", env!("CARGO_PKG_VERSION")), +))] +#[cfg_attr(not(feature = "headless"), clap( + name = "wasmer", + about = concat!("wasmer ", env!("CARGO_PKG_VERSION")), +))] pub struct Args { /// Print version info and exit. #[clap(short = 'V', long)] diff --git a/lib/cli/tests/version.rs b/lib/cli/tests/version.rs new file mode 100644 index 00000000000..1dcb7e83079 --- /dev/null +++ b/lib/cli/tests/version.rs @@ -0,0 +1,68 @@ +use assert_cmd::Command; + +const WASMER_VERSION: &str = env!("CARGO_PKG_VERSION"); + +#[test] +fn short_version_string() { + let version_number = format!("wasmer {WASMER_VERSION}"); + + Command::cargo_bin("wasmer") + .unwrap() + .arg("--version") + .assert() + .success() + .stdout(predicates::str::contains(&version_number)); + + Command::cargo_bin("wasmer") + .unwrap() + .arg("-V") + .assert() + .success() + .stdout(predicates::str::contains(&version_number)); +} + +#[test] +fn long_version_string() { + let long_version_number = format!( + "wasmer {} ({} {})", + env!("CARGO_PKG_VERSION"), + env!("WASMER_BUILD_GIT_HASH_SHORT"), + env!("WASMER_BUILD_DATE") + ); + + Command::cargo_bin("wasmer") + .unwrap() + .arg("--version") + .arg("--verbose") + .assert() + .success() + .stdout(predicates::str::contains(&long_version_number)) + .stdout(predicates::str::contains("binary:")); + + Command::cargo_bin("wasmer") + .unwrap() + .arg("-Vv") + .assert() + .success() + .stdout(predicates::str::contains(&long_version_number)) + .stdout(predicates::str::contains("binary:")); +} + +#[test] +fn help_text_contains_version() { + let version_number = format!("wasmer {WASMER_VERSION}"); + + Command::cargo_bin("wasmer") + .unwrap() + .arg("-h") + .assert() + .success() + .stdout(predicates::str::contains(&version_number)); + + Command::cargo_bin("wasmer") + .unwrap() + .arg("--help") + .assert() + .success() + .stdout(predicates::str::contains(&version_number)); +} diff --git a/tests/integration/cli/tests/version.rs b/tests/integration/cli/tests/version.rs deleted file mode 100644 index f304749d732..00000000000 --- a/tests/integration/cli/tests/version.rs +++ /dev/null @@ -1,64 +0,0 @@ -use anyhow::bail; -use std::process::Command; -use wasmer_integration_tests_cli::get_wasmer_path; - -const WASMER_VERSION: &str = env!("CARGO_PKG_VERSION"); - -#[test] -fn version_string_is_correct() -> anyhow::Result<()> { - let expected_version_output = format!("wasmer {}\n", WASMER_VERSION); - let wasmer_path = get_wasmer_path(); - - let outputs = [ - Command::new(&wasmer_path).arg("--version").output()?, - Command::new(&wasmer_path).arg("-V").output()?, - ]; - - for output in &outputs { - if !output.status.success() { - bail!( - "version failed with: stdout: {}\n\nstderr: {}", - std::str::from_utf8(&output.stdout) - .expect("stdout is not utf8! need to handle arbitrary bytes"), - std::str::from_utf8(&output.stderr) - .expect("stderr is not utf8! need to handle arbitrary bytes") - ); - } - - let stdout_output = std::str::from_utf8(&output.stdout).unwrap(); - assert_eq!(stdout_output, &expected_version_output); - } - - Ok(()) -} - -#[test] -fn help_text_contains_version() -> anyhow::Result<()> { - let expected_version_output = format!("wasmer {}", WASMER_VERSION); - let wasmer_path = get_wasmer_path(); - - let outputs = [ - Command::new(&wasmer_path).arg("--help").output()?, - Command::new(&wasmer_path).arg("-h").output()?, - ]; - - for output in &outputs { - if !output.status.success() { - bail!( - "version failed with: stdout: {}\n\nstderr: {}", - std::str::from_utf8(&output.stdout) - .expect("stdout is not utf8! need to handle arbitrary bytes"), - std::str::from_utf8(&output.stderr) - .expect("stderr is not utf8! need to handle arbitrary bytes") - ); - } - - let stdout_output = std::str::from_utf8(&output.stdout).unwrap(); - assert_eq!( - stdout_output.lines().next().unwrap(), - &expected_version_output - ); - } - - Ok(()) -} From e620278a27c199ab6e81bcdd89b5115052b88fea Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Fri, 16 Jun 2023 16:37:19 +0800 Subject: [PATCH 10/10] Fixed up some issues with the "wasmer run" detection logic --- lib/cli/src/cli.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/cli/src/cli.rs b/lib/cli/src/cli.rs index fcc3bd9ae67..134cf3cbf08 100644 --- a/lib/cli/src/cli.rs +++ b/lib/cli/src/cli.rs @@ -36,7 +36,12 @@ fn wasmer_main_inner() -> Result<(), anyhow::Error> { args.execute() } Err(e) => { - if e.kind() == clap::error::ErrorKind::InvalidSubcommand { + let might_be_wasmer_run = matches!( + e.kind(), + clap::error::ErrorKind::InvalidSubcommand | clap::error::ErrorKind::UnknownArgument + ); + + if might_be_wasmer_run { if let Ok(run) = Run::try_parse() { // Try to parse the command using the `wasmer some/package` // shorthand. Note that this has discoverability issues