Require wasmtime options are first when running modules

ci/run-wasi-crypto-example.sh

@@ -7,4 +7,4 @@ pushd "$RUST_BINDINGS"
 cargo build --release --target=wasm32-wasi
 popd
 
-cargo run --features wasi-crypto -- run "$RUST_BINDINGS/target/wasm32-wasi/release/wasi-crypto-guest.wasm" --wasi-modules=experimental-wasi-crypto
+cargo run --features wasi-crypto -- run --wasi-modules=experimental-wasi-crypto "$RUST_BINDINGS/target/wasm32-wasi/release/wasi-crypto-guest.wasm"

ci/run-wasi-nn-example.sh

@@ -33,7 +33,7 @@ cp target/wasm32-wasi/release/wasi-nn-example.wasm $TMP_DIR
 popd
 
 # Run the example in Wasmtime (note that the example uses `fixture` as the expected location of the model/tensor files).
-cargo run -- run --mapdir fixture::$TMP_DIR $TMP_DIR/wasi-nn-example.wasm --wasi-modules=experimental-wasi-nn
+cargo run -- run --mapdir fixture::$TMP_DIR --wasi-modules=experimental-wasi-nn $TMP_DIR/wasi-nn-example.wasm
 
 # Clean up the temporary directory only if it was not specified (users may want to keep the directory around).
 if [[ $REMOVE_TMP_DIR -eq 1 ]]; then

src/bin/wasmtime.rs

@@ -4,7 +4,7 @@
 //! See `wasmtime --help` for usage.
 
 use anyhow::Result;
-use clap::{error::ErrorKind, Parser};
+use clap::Parser;
 use wasmtime_cli::commands::{
     CompileCommand, ConfigCommand, ExploreCommand, RunCommand, SettingsCommand, WastCommand,
 };
@@ -27,10 +27,24 @@ use wasmtime_cli::commands::{
                   \n\
                   Invoking a specific function (e.g. `add`) in a WebAssembly module:\n\
                   \n  \
-                  wasmtime example.wasm --invoke add 1 2\n"
+                  wasmtime example.wasm --invoke add 1 2\n",
+
+    // This option enables the pattern below where we ask clap to parse twice
+    // sorta: once where it's trying to find a subcommand and once assuming
+    // a subcommand doesn't get passed. Clap should then, apparently,
+    // fill in the `subcommand` if found and otherwise fill in the
+    // `RunCommand`.
+    args_conflicts_with_subcommands = true
 )]
-enum Wasmtime {
-    // !!! IMPORTANT: if subcommands are added or removed, update `parse_module` in `src/commands/run.rs`. !!!
+struct Wasmtime {
+    #[clap(subcommand)]
+    subcommand: Option<Subcommand>,
+    #[clap(flatten)]
+    run: RunCommand,
+}
+
+#[derive(Parser)]
+enum Subcommand {
     /// Controls Wasmtime configuration settings
     Config(ConfigCommand),
     /// Compiles a WebAssembly module.
@@ -48,26 +62,20 @@ enum Wasmtime {
 impl Wasmtime {
     /// Executes the command.
     pub fn execute(self) -> Result<()> {
-        match self {
-            Self::Config(c) => c.execute(),
-            Self::Compile(c) => c.execute(),
-            Self::Explore(c) => c.execute(),
-            Self::Run(c) => c.execute(),
-            Self::Settings(c) => c.execute(),
-            Self::Wast(c) => c.execute(),
+        let subcommand = self.subcommand.unwrap_or(Subcommand::Run(self.run));
+        match subcommand {
+            Subcommand::Config(c) => c.execute(),
+            Subcommand::Compile(c) => c.execute(),
+            Subcommand::Explore(c) => c.execute(),
+            Subcommand::Run(c) => c.execute(),
+            Subcommand::Settings(c) => c.execute(),
+            Subcommand::Wast(c) => c.execute(),
         }
     }
 }
 
 fn main() -> Result<()> {
-    Wasmtime::try_parse()
-        .unwrap_or_else(|e| match e.kind() {
-            ErrorKind::InvalidSubcommand | ErrorKind::UnknownArgument => {
-                Wasmtime::Run(RunCommand::parse())
-            }
-            _ => e.exit(),
-        })
-        .execute()
+    Wasmtime::parse().execute()
 }
 
 #[test]

src/commands/run.rs

@@ -1,14 +1,11 @@
 //! The module that implements the `wasmtime run` command.
 
 use anyhow::{anyhow, bail, Context as _, Result};
-use clap::builder::{OsStringValueParser, TypedValueParser};
 use clap::Parser;
 use once_cell::sync::Lazy;
-use std::ffi::OsStr;
-use std::ffi::OsString;
 use std::fs::File;
 use std::io::Write;
-use std::path::{Component, Path, PathBuf};
+use std::path::{Path, PathBuf};
 use std::thread;
 use std::time::Duration;
 use wasmtime::{
@@ -39,18 +36,6 @@ use wasmtime_wasi_threads::WasiThreadsCtx;
 #[cfg(feature = "wasi-http")]
 use wasmtime_wasi_http::WasiHttp;
 
-fn parse_module(s: OsString) -> anyhow::Result<PathBuf> {
-    // Do not accept wasmtime subcommand names as the module name
-    match s.to_str() {
-        Some("help") | Some("config") | Some("run") | Some("wast") | Some("compile") => {
-            bail!("module name cannot be the same as a subcommand")
-        }
-        #[cfg(unix)]
-        Some("-") => Ok(PathBuf::from("/dev/stdin")),
-        _ => Ok(s.into()),
-    }
-}
-
 fn parse_env_var(s: &str) -> Result<(String, Option<String>)> {
     let mut parts = s.splitn(2, '=');
     Ok((
@@ -111,7 +96,7 @@ static AFTER_HELP: Lazy<String> = Lazy::new(|| crate::FLAG_EXPLANATIONS.to_strin
 
 /// Runs a WebAssembly module
 #[derive(Parser)]
-#[structopt(name = "run", trailing_var_arg = true, after_help = AFTER_HELP.as_str())]
+#[structopt(name = "run", after_help = AFTER_HELP.as_str())]
 pub struct RunCommand {
     #[clap(flatten)]
     common: CommonOptions,
@@ -174,14 +159,6 @@ pub struct RunCommand {
     #[clap(long = "mapdir", number_of_values = 1, value_name = "GUEST_DIR::HOST_DIR", value_parser = parse_map_dirs)]
     map_dirs: Vec<(String, String)>,
 
-    /// The path of the WebAssembly module to run
-    #[clap(
-        required = true,
-        value_name = "MODULE",
-        value_parser = OsStringValueParser::new().try_map(parse_module),
-    )]
-    module: PathBuf,
-
     /// Load the given WebAssembly module before the main module
     #[clap(
         long = "preload",
@@ -225,11 +202,6 @@ pub struct RunCommand {
     #[clap(long = "coredump-on-trap", value_name = "PATH")]
     coredump_on_trap: Option<String>,
 
-    // NOTE: this must come last for trailing varargs
-    /// The arguments to pass to the module
-    #[clap(value_name = "ARGS")]
-    module_args: Vec<String>,
-
     /// Maximum size, in bytes, that a linear memory is allowed to reach.
     ///
     /// Growth beyond this limit will cause `memory.grow` instructions in
@@ -261,6 +233,14 @@ pub struct RunCommand {
     /// memory, for example.
     #[clap(long)]
     trap_on_grow_failure: bool,
+
+    /// The WebAssembly module to run and arguments to pass to it.
+    ///
+    /// Arguments passed to the wasm module will be configured as WASI CLI
+    /// arguments unless the `--invoke` CLI argument is passed in which case
+    /// arguments will be interpreted as arguments to the function specified.
+    #[clap(value_name = "WASM", trailing_var_arg = true, required = true)]
+    module_and_args: Vec<PathBuf>,
 }
 
 #[derive(Clone)]
@@ -303,13 +283,13 @@ impl RunCommand {
 
         // Make wasi available by default.
         let preopen_dirs = self.compute_preopen_dirs()?;
-        let argv = self.compute_argv();
+        let argv = self.compute_argv()?;
 
         let mut linker = Linker::new(&engine);
         linker.allow_unknown_exports(self.allow_unknown_exports);
 
         // Read the wasm module binary either as `*.wat` or a raw binary.
-        let module = self.load_module(linker.engine(), &self.module)?;
+        let module = self.load_module(linker.engine(), &self.module_and_args[0])?;
         let mut modules = vec![(String::new(), module.clone())];
 
         let host = Host::default();
@@ -370,8 +350,12 @@ impl RunCommand {
         // Load the main wasm module.
         match self
             .load_main_module(&mut store, &mut linker, module, modules, &argv[0])
-            .with_context(|| format!("failed to run main module `{}`", self.module.display()))
-        {
+            .with_context(|| {
+                format!(
+                    "failed to run main module `{}`",
+                    self.module_and_args[0].display()
+                )
+            }) {
             Ok(()) => (),
             Err(e) => {
                 // Exit the process if Wasmtime understands the error;
@@ -420,27 +404,25 @@ impl RunCommand {
         Ok(listeners)
     }
 
-    fn compute_argv(&self) -> Vec<String> {
+    fn compute_argv(&self) -> Result<Vec<String>> {
         let mut result = Vec::new();
 
-        // Add argv[0], which is the program name. Only include the base name of the
-        // main wasm module, to avoid leaking path information.
-        result.push(
-            self.module
-                .components()
-                .next_back()
-                .map(Component::as_os_str)
-                .and_then(OsStr::to_str)
-                .unwrap_or("")
-                .to_owned(),
-        );
-
-        // Add the remaining arguments.
-        for arg in self.module_args.iter() {
-            result.push(arg.clone());
+        for (i, arg) in self.module_and_args.iter().enumerate() {
+            // For argv[0], which is the program name. Only include the base
+            // name of the main wasm module, to avoid leaking path information.
+            let arg = if i == 0 {
+                arg.components().next_back().unwrap().as_os_str()
+            } else {
+                arg.as_ref()
+            };
+            result.push(
+                arg.to_str()
+                    .ok_or_else(|| anyhow!("failed to convert {arg:?} to utf-8"))?
+                    .to_string(),
+            );
         }
 
-        result
+        Ok(result)
     }
 
     fn setup_epoch_handler(
@@ -541,9 +523,10 @@ impl RunCommand {
         }
 
         // Use "" as a default module name.
-        linker
-            .module(&mut *store, "", &module)
-            .context(format!("failed to instantiate {:?}", self.module))?;
+        linker.module(&mut *store, "", &module).context(format!(
+            "failed to instantiate {:?}",
+            self.module_and_args[0]
+        ))?;
 
         // If a function to invoke was given, invoke it.
         let func = if let Some(name) = &self.invoke {
@@ -584,7 +567,7 @@ impl RunCommand {
                  is experimental and may break in the future"
             );
         }
-        let mut args = self.module_args.iter();
+        let mut args = self.module_and_args.iter().skip(1);
         let mut values = Vec::new();
         for ty in ty.params() {
             let val = match args.next() {
@@ -597,6 +580,9 @@ impl RunCommand {
                     }
                 }
             };
+            let val = val
+                .to_str()
+                .ok_or_else(|| anyhow!("argument is not valid utf-8: {val:?}"))?;
             values.push(match ty {
                 // TODO: integer parsing here should handle hexadecimal notation
                 // like `0x0...`, but the Rust standard library currently only
@@ -623,7 +609,9 @@ impl RunCommand {
         if let Err(err) = invoke_res {
             let err = if err.is::<wasmtime::Trap>() {
                 if let Some(coredump_path) = self.coredump_on_trap.as_ref() {
-                    let source_name = self.module.to_str().unwrap_or_else(|| "unknown");
+                    let source_name = self.module_and_args[0]
+                        .to_str()
+                        .unwrap_or_else(|| "unknown");
 
                     if let Err(coredump_err) = generate_coredump(&err, &source_name, coredump_path)
                     {
@@ -664,6 +652,12 @@ impl RunCommand {
     }
 
     fn load_module(&self, engine: &Engine, path: &Path) -> Result<Module> {
+        let path = match path.to_str() {
+            #[cfg(unix)]
+            Some("-") => "/dev/stdin".as_ref(),
+            _ => path,
+        };
+
         if self.allow_precompiled {
             unsafe { Module::from_trusted_file(engine, path) }
         } else {