Adress review comments

lib/cli/src/cli.rs

@@ -10,12 +10,10 @@ use crate::commands::CreateExe;
 use crate::commands::CreateObj;
 #[cfg(feature = "wast")]
 use crate::commands::Wast;
-use crate::commands::{Cache, Config, Inspect, Run, RunWithoutFile, SelfUpdate, Validate};
+use crate::commands::{Cache, Config, Inspect, List, Run, SelfUpdate, Validate};
 use crate::error::PrettyError;
 use clap::{CommandFactory, ErrorKind, Parser};
-use spinner::SpinnerHandle;
 use std::fmt;
-use wasmer_registry::{get_all_local_packages, PackageDownloadInfo};
 
 #[derive(Parser, Debug)]
 #[cfg_attr(
@@ -40,7 +38,7 @@ use wasmer_registry::{get_all_local_packages, PackageDownloadInfo};
 enum WasmerCLIOptions {
     /// List all locally installed packages
     #[clap(name = "list")]
-    List,
+    List(List),
 
     /// Run a WebAssembly file. Formats accepted: wasm, wat
     #[clap(name = "run")]
@@ -165,7 +163,7 @@ impl WasmerCLIOptions {
             Self::CreateObj(create_obj) => create_obj.execute(),
             Self::Config(config) => config.execute(),
             Self::Inspect(inspect) => inspect.execute(),
-            Self::List => print_packages(),
+            Self::List(list) => list.execute(),
             #[cfg(feature = "wast")]
             Self::Wast(wast) => wast.execute(),
             #[cfg(target_os = "linux")]
@@ -239,206 +237,19 @@ fn wasmer_main_inner() -> Result<(), anyhow::Error> {
 
     // Check if the file is a package name
     if let WasmerCLIOptions::Run(r) = &options {
-        return try_run_package_or_file(&args, r);
+        return crate::commands::try_run_package_or_file(&args, r);
     }
 
     options.execute()
 }
 
-fn try_run_package_or_file(args: &[String], r: &Run) -> Result<(), anyhow::Error> {
-    // Check "r.path" is a file or a package / command name
-    if r.path.exists() {
-        if r.path.is_dir() && r.path.join("wapm.toml").exists() {
-            let mut args_without_package = args.to_vec();
-            if args_without_package.get(1) == Some(&format!("{}", r.path.display())) {
-                let _ = args_without_package.remove(1);
-            } else if args_without_package.get(2) == Some(&format!("{}", r.path.display())) {
-                let _ = args_without_package.remove(1);
-                let _ = args_without_package.remove(1);
-            }
-            return RunWithoutFile::try_parse_from(args_without_package.iter())?
-                .into_run_args(r.path.clone(), r.command_name.as_deref())?
-                .execute();
-        }
-        return r.execute();
-    }
-
-    let package = format!("{}", r.path.display());
-
-    let mut is_fake_sv = false;
-    let mut sv = match split_version(&package) {
-        Ok(o) => o,
-        Err(_) => {
-            let mut fake_sv = SplitVersion {
-                original: package.to_string(),
-                registry: None,
-                package: package.to_string(),
-                version: None,
-                command: None,
-            };
-            is_fake_sv = true;
-            match try_lookup_command(&mut fake_sv) {
-                Ok(o) => SplitVersion {
-                    original: format!("{}@{}", o.package, o.version),
-                    registry: None,
-                    package: o.package,
-                    version: Some(o.version),
-                    command: r.command_name.clone(),
-                },
-                Err(e) => {
-                    return Err(
-                        anyhow::anyhow!("No package for command {package:?} found, file {package:?} not found either")
-                        .context(e)
-                        .context(anyhow::anyhow!("{}", r.path.display()))
-                    );
-                }
-            }
-        }
-    };
-
-    if sv.command.is_none() {
-        sv.command = r.command_name.clone();
-    }
-
-    if sv.command.is_none() && is_fake_sv {
-        sv.command = Some(package);
-    }
-
-    let mut package_download_info = None;
-    if !sv.package.contains('/') {
-        if let Ok(o) = try_lookup_command(&mut sv) {
-            package_download_info = Some(o);
-        }
-    }
-
-    match try_execute_local_package(args, &sv) {
-        Ok(o) => return Ok(o),
-        Err(ExecuteLocalPackageError::DuringExec(e)) => return Err(e),
-        _ => {}
-    }
-
-    println!("finding local package {} failed", sv);
-    // else: local package not found - try to download and install package
-    try_autoinstall_package(args, &sv, package_download_info, r.force_install)
-}
-
-fn try_lookup_command(sv: &mut SplitVersion) -> Result<PackageDownloadInfo, anyhow::Error> {
-    use std::io::Write;
-    let sp = start_spinner(format!("Looking up command {} ...", sv.package));
-
-    for registry in wasmer_registry::get_all_available_registries().unwrap_or_default() {
-        let result = wasmer_registry::query_command_from_registry(&registry, &sv.package);
-        print!("\r");
-        let _ = std::io::stdout().flush();
-        let command = sv.package.clone();
-        if let Ok(o) = result {
-            sv.package = o.package.clone();
-            sv.version = Some(o.version.clone());
-            sv.command = Some(command);
-            return Ok(o);
-        }
-    }
-
-    sp.close();
-    print!("\r");
-    let _ = std::io::stdout().flush();
-    Err(anyhow::anyhow!("command {sv} not found"))
-}
-
-// We need to distinguish between errors that happen
-// before vs. during execution
-enum ExecuteLocalPackageError {
-    BeforeExec(anyhow::Error),
-    DuringExec(anyhow::Error),
-}
-
-fn try_execute_local_package(
-    args: &[String],
-    sv: &SplitVersion,
-) -> Result<(), ExecuteLocalPackageError> {
-    let package = wasmer_registry::get_local_package(None, &sv.package, sv.version.as_deref())
-        .ok_or_else(|| {
-            ExecuteLocalPackageError::BeforeExec(anyhow::anyhow!("no local package {sv:?} found"))
-        })?;
-
-    let package_dir = package
-        .get_path()
-        .map_err(|e| ExecuteLocalPackageError::BeforeExec(anyhow::anyhow!("{e}")))?;
-
-    // Try finding the local package
-    let mut args_without_package = args.to_vec();
-
-    // remove either "run" or $package
-    args_without_package.remove(1);
-
-    // "wasmer package arg1 arg2" => "wasmer arg1 arg2"
-    if (args_without_package.get(1).is_some() && args_without_package[1].starts_with(&sv.original))
-        || (sv.command.is_some() && args_without_package[1].ends_with(sv.command.as_ref().unwrap()))
-    {
-        args_without_package.remove(1);
-    }
-
-    RunWithoutFile::try_parse_from(args_without_package.iter())
-        .map_err(|e| ExecuteLocalPackageError::DuringExec(e.into()))?
-        .into_run_args(package_dir, sv.command.as_deref())
-        .map_err(ExecuteLocalPackageError::DuringExec)?
-        .execute()
-        .map_err(|e| ExecuteLocalPackageError::DuringExec(e.context(anyhow::anyhow!("{}", sv))))
-}
-
-fn try_autoinstall_package(
-    args: &[String],
-    sv: &SplitVersion,
-    package: Option<PackageDownloadInfo>,
-    force_install: bool,
-) -> Result<(), anyhow::Error> {
-    use std::io::Write;
-    let sp = start_spinner(format!("Installing package {} ...", sv.package));
-    let v = sv.version.as_deref();
-    let result = wasmer_registry::install_package(
-        sv.registry.as_deref(),
-        &sv.package,
-        v,
-        package,
-        force_install,
-    );
-    sp.close();
-    print!("\r");
-    let _ = std::io::stdout().flush();
-    let (_, package_dir) = match result {
-        Ok(o) => o,
-        Err(e) => {
-            return Err(anyhow::anyhow!("{e}"));
-        }
-    };
-
-    // Try auto-installing the remote package
-    let mut args_without_package = args.to_vec();
-    args_without_package.remove(1);
-
-    let mut run_args = RunWithoutFile::try_parse_from(args_without_package.iter())?;
-    run_args.command_name = sv.command.clone();
-
-    run_args
-        .into_run_args(package_dir, sv.command.as_deref())?
-        .execute()
-}
-
-fn start_spinner(msg: String) -> SpinnerHandle {
-    spinner::SpinnerBuilder::new(msg)
-        .spinner(vec![
-            "⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷", " ", "⠁", "⠂", "⠄", "⡀", "⢀", "⠠", "⠐", "⠈",
-        ])
-        .start()
-}
-
 #[derive(Debug, Clone, PartialEq, Default)]
-struct SplitVersion {
+pub(crate) struct SplitVersion {
-    original: String,
+    pub(crate) original: String,
-    registry: Option<String>,
+    pub(crate) registry: Option<String>,
-    package: String,
+    pub(crate) package: String,
-    version: Option<String>,
+    pub(crate) version: Option<String>,
-    command: Option<String>,
+    pub(crate) command: Option<String>,
 }
 
 impl fmt::Display for SplitVersion {
@@ -521,7 +332,8 @@ fn test_split_version() {
     );
 }
 
-fn split_version(s: &str) -> Result<SplitVersion, anyhow::Error> {
+impl SplitVersion {
+    pub fn new(s: &str) -> Result<SplitVersion, anyhow::Error> {
         let command = WasmerCLIOptions::command();
         let mut prohibited_package_names = command.get_subcommands().map(|s| s.get_name());
 
@@ -621,39 +433,6 @@ fn split_version(s: &str) -> Result<SplitVersion, anyhow::Error> {
 
         Ok(sv)
     }
-
-fn print_packages() -> Result<(), anyhow::Error> {
-    use prettytable::{format, row, Table};
-
-    let rows = get_all_local_packages(None)
-        .into_iter()
-        .filter_map(|pkg| {
-            let package_root_path = pkg.get_path().ok()?;
-            let (manifest, _) =
-                wasmer_registry::get_executable_file_from_path(&package_root_path, None).ok()?;
-            let commands = manifest
-                .command
-                .unwrap_or_default()
-                .iter()
-                .map(|c| c.get_name())
-                .collect::<Vec<_>>()
-                .join(" \r\n");
-
-            Some(row![pkg.registry, pkg.name, pkg.version, commands])
-        })
-        .collect::<Vec<_>>();
-
-    let empty_table = rows.is_empty();
-    let mut table = Table::init(rows);
-    table.set_titles(row!["Registry", "Package", "Version", "Commands"]);
-    table.set_format(*format::consts::FORMAT_NO_LINESEP_WITH_TITLE);
-    table.set_format(*format::consts::FORMAT_NO_COLSEP);
-    if empty_table {
-        table.add_empty_row();
-    }
-    let _ = table.printstd();
-
-    Ok(())
 }
 
 fn print_help(verbose: bool) -> Result<(), anyhow::Error> {

lib/cli/src/commands.rs

@@ -10,6 +10,7 @@ mod create_exe;
 #[cfg(feature = "static-artifact-create")]
 mod create_obj;
 mod inspect;
+mod list;
 mod run;
 mod self_update;
 mod validate;
@@ -26,7 +27,7 @@ pub use create_exe::*;
 pub use create_obj::*;
 #[cfg(feature = "wast")]
 pub use wast::*;
-pub use {cache::*, config::*, inspect::*, run::*, self_update::*, validate::*};
+pub use {cache::*, config::*, inspect::*, list::*, run::*, self_update::*, validate::*};
 
 /// The kind of object format to emit.
 #[derive(Debug, Copy, Clone, clap::Parser)]

lib/cli/src/commands/list.rs

@@ -0,0 +1,43 @@
+use clap::Parser;
+
+/// Subcommand for listing packages
+#[derive(Debug, Copy, Clone, Parser)]
+pub struct List {}
+
+impl List {
+    /// execute [List]
+    pub fn execute(&self) -> Result<(), anyhow::Error> {
+        use prettytable::{format, row, Table};
+
+        let rows = wasmer_registry::get_all_local_packages(None)
+            .into_iter()
+            .filter_map(|pkg| {
+                let package_root_path = pkg.get_path().ok()?;
+                let (manifest, _) =
+                    wasmer_registry::get_executable_file_from_path(&package_root_path, None)
+                        .ok()?;
+                let commands = manifest
+                    .command
+                    .unwrap_or_default()
+                    .iter()
+                    .map(|c| c.get_name())
+                    .collect::<Vec<_>>()
+                    .join(" \r\n");
+
+                Some(row![pkg.registry, pkg.name, pkg.version, commands])
+            })
+            .collect::<Vec<_>>();
+
+        let empty_table = rows.is_empty();
+        let mut table = Table::init(rows);
+        table.set_titles(row!["Registry", "Package", "Version", "Commands"]);
+        table.set_format(*format::consts::FORMAT_NO_LINESEP_WITH_TITLE);
+        table.set_format(*format::consts::FORMAT_NO_COLSEP);
+        if empty_table {
+            table.add_empty_row();
+        }
+        let _ = table.printstd();
+
+        Ok(())
+    }
+}

lib/cli/src/commands/run.rs

@@ -15,7 +15,9 @@ use wasmer::*;
 use wasmer_cache::{Cache, FileSystemCache, Hash};
 use wasmer_types::Type as ValueType;
 
+use crate::cli::SplitVersion;
 use clap::Parser;
+use wasmer_registry::PackageDownloadInfo;
 
 #[cfg(feature = "wasi")]
 mod wasi;
@@ -589,3 +591,190 @@ impl Run {
         bail!("binfmt_misc is only available on linux.")
     }
 }
+
+fn start_spinner(msg: String) -> spinner::SpinnerHandle {
+    spinner::SpinnerBuilder::new(msg)
+        .spinner(vec![
+            "⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷", " ", "⠁", "⠂", "⠄", "⡀", "⢀", "⠠", "⠐", "⠈",
+        ])
+        .start()
+}
+
+pub(crate) fn try_autoinstall_package(
+    args: &[String],
+    sv: &SplitVersion,
+    package: Option<PackageDownloadInfo>,
+    force_install: bool,
+) -> Result<(), anyhow::Error> {
+    use std::io::Write;
+    let sp = start_spinner(format!("Installing package {} ...", sv.package));
+    let v = sv.version.as_deref();
+    let result = wasmer_registry::install_package(
+        sv.registry.as_deref(),
+        &sv.package,
+        v,
+        package,
+        force_install,
+    );
+    sp.close();
+    print!("\r");
+    let _ = std::io::stdout().flush();
+    let (_, package_dir) = match result {
+        Ok(o) => o,
+        Err(e) => {
+            return Err(anyhow::anyhow!("{e}"));
+        }
+    };
+
+    // Try auto-installing the remote package
+    let mut args_without_package = args.to_vec();
+    args_without_package.remove(1);
+
+    let mut run_args = RunWithoutFile::try_parse_from(args_without_package.iter())?;
+    run_args.command_name = sv.command.clone();
+
+    run_args
+        .into_run_args(package_dir, sv.command.as_deref())?
+        .execute()
+}
+
+// We need to distinguish between errors that happen
+// before vs. during execution
+enum ExecuteLocalPackageError {
+    BeforeExec(anyhow::Error),
+    DuringExec(anyhow::Error),
+}
+
+fn try_execute_local_package(
+    args: &[String],
+    sv: &SplitVersion,
+) -> Result<(), ExecuteLocalPackageError> {
+    let package = wasmer_registry::get_local_package(None, &sv.package, sv.version.as_deref())
+        .ok_or_else(|| {
+            ExecuteLocalPackageError::BeforeExec(anyhow::anyhow!("no local package {sv:?} found"))
+        })?;
+
+    let package_dir = package
+        .get_path()
+        .map_err(|e| ExecuteLocalPackageError::BeforeExec(anyhow::anyhow!("{e}")))?;
+
+    // Try finding the local package
+    let mut args_without_package = args.to_vec();
+
+    // remove either "run" or $package
+    args_without_package.remove(1);
+
+    // "wasmer package arg1 arg2" => "wasmer arg1 arg2"
+    if (args_without_package.get(1).is_some() && args_without_package[1].starts_with(&sv.original))
+        || (sv.command.is_some() && args_without_package[1].ends_with(sv.command.as_ref().unwrap()))
+    {
+        args_without_package.remove(1);
+    }
+
+    RunWithoutFile::try_parse_from(args_without_package.iter())
+        .map_err(|e| ExecuteLocalPackageError::DuringExec(e.into()))?
+        .into_run_args(package_dir, sv.command.as_deref())
+        .map_err(ExecuteLocalPackageError::DuringExec)?
+        .execute()
+        .map_err(|e| ExecuteLocalPackageError::DuringExec(e.context(anyhow::anyhow!("{}", sv))))
+}
+
+fn try_lookup_command(sv: &mut SplitVersion) -> Result<PackageDownloadInfo, anyhow::Error> {
+    use std::io::Write;
+    let sp = start_spinner(format!("Looking up command {} ...", sv.package));
+
+    for registry in wasmer_registry::get_all_available_registries().unwrap_or_default() {
+        let result = wasmer_registry::query_command_from_registry(&registry, &sv.package);
+        print!("\r");
+        let _ = std::io::stdout().flush();
+        let command = sv.package.clone();
+        if let Ok(o) = result {
+            sv.package = o.package.clone();
+            sv.version = Some(o.version.clone());
+            sv.command = Some(command);
+            return Ok(o);
+        }
+    }
+
+    sp.close();
+    print!("\r");
+    let _ = std::io::stdout().flush();
+    Err(anyhow::anyhow!("command {sv} not found"))
+}
+
+pub(crate) fn try_run_package_or_file(args: &[String], r: &Run) -> Result<(), anyhow::Error> {
+    // Check "r.path" is a file or a package / command name
+    if r.path.exists() {
+        if r.path.is_dir() && r.path.join("wapm.toml").exists() {
+            let mut args_without_package = args.to_vec();
+            if args_without_package.get(1) == Some(&format!("{}", r.path.display())) {
+                let _ = args_without_package.remove(1);
+            } else if args_without_package.get(2) == Some(&format!("{}", r.path.display())) {
+                let _ = args_without_package.remove(1);
+                let _ = args_without_package.remove(1);
+            }
+            return RunWithoutFile::try_parse_from(args_without_package.iter())?
+                .into_run_args(r.path.clone(), r.command_name.as_deref())?
+                .execute();
+        }
+        return r.execute();
+    }
+
+    let package = format!("{}", r.path.display());
+
+    let mut is_fake_sv = false;
+    let mut sv = match SplitVersion::new(&package) {
+        Ok(o) => o,
+        Err(_) => {
+            let mut fake_sv = SplitVersion {
+                original: package.to_string(),
+                registry: None,
+                package: package.to_string(),
+                version: None,
+                command: None,
+            };
+            is_fake_sv = true;
+            match try_lookup_command(&mut fake_sv) {
+                Ok(o) => SplitVersion {
+                    original: format!("{}@{}", o.package, o.version),
+                    registry: None,
+                    package: o.package,
+                    version: Some(o.version),
+                    command: r.command_name.clone(),
+                },
+                Err(e) => {
+                    return Err(
+                        anyhow::anyhow!("No package for command {package:?} found, file {package:?} not found either")
+                        .context(e)
+                        .context(anyhow::anyhow!("{}", r.path.display()))
+                    );
+                }
+            }
+        }
+    };
+
+    if sv.command.is_none() {
+        sv.command = r.command_name.clone();
+    }
+
+    if sv.command.is_none() && is_fake_sv {
+        sv.command = Some(package);
+    }
+
+    let mut package_download_info = None;
+    if !sv.package.contains('/') {
+        if let Ok(o) = try_lookup_command(&mut sv) {
+            package_download_info = Some(o);
+        }
+    }
+
+    match try_execute_local_package(args, &sv) {
+        Ok(o) => return Ok(o),
+        Err(ExecuteLocalPackageError::DuringExec(e)) => return Err(e),
+        _ => {}
+    }
+
+    println!("finding local package {} failed", sv);
+    // else: local package not found - try to download and install package
+    try_autoinstall_package(args, &sv, package_download_info, r.force_install)
+}