From 70fa262c5cb87df551fb806194f2c701400fe976 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 12:44:15 +0100 Subject: [PATCH 01/12] Remove unnecessary checks to test internet connection --- lib/registry/src/lib.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index 267755a4766..eb03472e4ff 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -678,9 +678,6 @@ pub fn get_if_package_has_new_version( version: Option, max_timeout: Duration, ) -> Result, String> { - if !test_if_registry_present(registry_url).unwrap_or(false) { - return Ok(None); - } let host = match url::Url::parse(registry_url) { Ok(o) => match o.host_str().map(|s| s.to_string()) { @@ -1018,10 +1015,6 @@ pub fn install_package( format!("Package {version_str} not found in registries {registries_searched:?}."); for r in registries.iter() { - let registry_test = test_if_registry_present(r); - if !registry_test.clone().unwrap_or(false) { - continue; - } if !force_install { let package_has_new_version = get_if_package_has_new_version( From 90f110f4b5867d13ce7c969597f5d14dac4e7cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 13:14:30 +0100 Subject: [PATCH 02/12] Disable println output and spinner if stdout is not a tty --- Cargo.lock | 29 +++++++++++++--- lib/cli/Cargo.toml | 1 + lib/cli/src/commands/run.rs | 66 ++++++++++++++++++++++++------------- lib/registry/src/lib.rs | 2 -- 4 files changed, 69 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ea0545bb901..7f7d22b5e6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1077,7 +1077,7 @@ checksum = "4b9663d381d07ae25dc88dbdf27df458faa83a9b25336bcac83d5e452b5fc9d3" dependencies = [ "cfg-if 1.0.0", "libc", - "redox_syscall", + "redox_syscall 0.2.16", "windows-sys 0.42.0", ] @@ -1662,6 +1662,18 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "879d54834c8c76457ef4293a689b2a8c59b076067ad77b15efafbb05f92a592b" +[[package]] +name = "isatty" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e31a8281fc93ec9693494da65fbf28c0c2aa60a2eaec25dc58e2f31952e95edc" +dependencies = [ + "cfg-if 0.1.10", + "libc", + "redox_syscall 0.1.57", + "winapi", +] + [[package]] name = "itertools" version = "0.10.5" @@ -2061,7 +2073,7 @@ dependencies = [ "cfg-if 1.0.0", "libc", "raw-window-handle 0.3.4", - "redox_syscall", + "redox_syscall 0.2.16", "sdl2", "sdl2-sys", "wasm-bindgen", @@ -2103,7 +2115,7 @@ dependencies = [ "cfg-if 1.0.0", "instant", "libc", - "redox_syscall", + "redox_syscall 0.2.16", "smallvec", "winapi", ] @@ -2457,6 +2469,12 @@ dependencies = [ "rand_core 0.3.1", ] +[[package]] +name = "redox_syscall" +version = "0.1.57" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41cc0f7e4d5d4544e8861606a285bb08d3e70712ccc7d2b84d7c0ccfaf4b05ce" + [[package]] name = "redox_syscall" version = "0.2.16" @@ -2473,7 +2491,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b033d837a7cf162d7993aded9304e30a83213c648b6e389db233191f891e5c2b" dependencies = [ "getrandom", - "redox_syscall", + "redox_syscall 0.2.16", "thiserror", ] @@ -3161,7 +3179,7 @@ dependencies = [ "cfg-if 1.0.0", "fastrand", "libc", - "redox_syscall", + "redox_syscall 0.2.16", "remove_dir_all", "winapi", ] @@ -3923,6 +3941,7 @@ dependencies = [ "distance", "fern", "http_req", + "isatty", "libc", "log", "nuke-dir", diff --git a/lib/cli/Cargo.toml b/lib/cli/Cargo.toml index 45987508bb9..4f18cf35864 100644 --- a/lib/cli/Cargo.toml +++ b/lib/cli/Cargo.toml @@ -70,6 +70,7 @@ url = "2.3.1" libc = { version = "^0.2", default-features = false } nuke-dir = { version = "0.1.0", optional = true } webc = { version = "3.0.1", optional = true } +isatty = "0.1.9" [build-dependencies] chrono = { version = "^0.4", default-features = false, features = [ "std", "clock" ] } diff --git a/lib/cli/src/commands/run.rs b/lib/cli/src/commands/run.rs index 4b06d9b7c81..4fd55a108e6 100644 --- a/lib/cli/src/commands/run.rs +++ b/lib/cli/src/commands/run.rs @@ -97,6 +97,7 @@ impl RunWithoutFile { mut self, package_root_dir: PathBuf, // <- package dir command: Option<&str>, + _debug_output_allowed: bool, ) -> Result { let (manifest, pathbuf) = wasmer_registry::get_executable_file_from_path(&package_root_dir, command)?; @@ -108,10 +109,12 @@ impl RunWithoutFile { for (alias, real_dir) in fs.iter() { let real_dir = package_root_dir.join(&real_dir); if !real_dir.exists() { - println!( - "warning: cannot map {alias:?} to {}: directory does not exist", - real_dir.display() - ); + if _debug_output_allowed { + println!( + "warning: cannot map {alias:?} to {}: directory does not exist", + real_dir.display() + ); + } continue; } @@ -641,12 +644,17 @@ impl Run { } } -fn start_spinner(msg: String) -> spinner::SpinnerHandle { - spinner::SpinnerBuilder::new(msg) - .spinner(vec![ - "⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷", " ", "⠁", "⠂", "⠄", "⡀", "⢀", "⠠", "⠐", "⠈", - ]) - .start() +fn start_spinner(msg: String) -> Option { + if !isatty::stdout_isatty() { + return None; + } + Some( + spinner::SpinnerBuilder::new(msg) + .spinner(vec![ + "⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷", " ", "⠁", "⠂", "⠄", "⡀", "⢀", "⠠", "⠐", "⠈", + ]) + .start(), + ) } pub(crate) fn try_autoinstall_package( @@ -656,7 +664,8 @@ pub(crate) fn try_autoinstall_package( force_install: bool, ) -> Result<(), anyhow::Error> { use std::io::Write; - let sp = start_spinner(format!("Installing package {} ...", sv.package)); + let mut sp = start_spinner(format!("Installing package {} ...", sv.package)); + let debug_msgs_allowed = sp.is_some(); let v = sv.version.as_deref(); let result = wasmer_registry::install_package( sv.registry.as_deref(), @@ -665,8 +674,10 @@ pub(crate) fn try_autoinstall_package( package, force_install, ); - sp.close(); - print!("\r"); + if let Some(sp) = sp.take() { + sp.close(); + print!("\r"); + } let _ = std::io::stdout().flush(); let (_, package_dir) = match result { Ok(o) => o, @@ -681,7 +692,7 @@ pub(crate) fn try_autoinstall_package( run_args.command_name = sv.command.clone(); run_args - .into_run_args(package_dir, sv.command.as_deref())? + .into_run_args(package_dir, sv.command.as_deref(), debug_msgs_allowed)? .execute() } @@ -695,6 +706,7 @@ enum ExecuteLocalPackageError { fn try_execute_local_package( args: &[String], sv: &SplitVersion, + debug_msgs_allowed: bool, ) -> Result<(), ExecuteLocalPackageError> { let package = wasmer_registry::get_local_package(None, &sv.package, sv.version.as_deref()) .ok_or_else(|| { @@ -710,7 +722,7 @@ fn try_execute_local_package( RunWithoutFile::try_parse_from(args_without_package.iter()) .map_err(|e| ExecuteLocalPackageError::DuringExec(e.into()))? - .into_run_args(package_dir, sv.command.as_deref()) + .into_run_args(package_dir, sv.command.as_deref(), debug_msgs_allowed) .map_err(ExecuteLocalPackageError::DuringExec)? .execute() .map_err(|e| ExecuteLocalPackageError::DuringExec(e.context(anyhow::anyhow!("{}", sv)))) @@ -718,11 +730,13 @@ fn try_execute_local_package( fn try_lookup_command(sv: &mut SplitVersion) -> Result { use std::io::Write; - let sp = start_spinner(format!("Looking up command {} ...", sv.package)); + let mut 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(®istry, &sv.package); - print!("\r"); + if sp.is_some() { + print!("\r"); + } let _ = std::io::stdout().flush(); let command = sv.package.clone(); if let Ok(o) = result { @@ -733,8 +747,10 @@ fn try_lookup_command(sv: &mut SplitVersion) -> Result Result<(), anyhow::Error> { + let debug_msgs_allowed = isatty::stdout_isatty(); + // 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 args_without_package = fixup_args(args, &format!("{}", r.path.display())); return RunWithoutFile::try_parse_from(args_without_package.iter())? - .into_run_args(r.path.clone(), r.command_name.as_deref())? + .into_run_args( + r.path.clone(), + r.command_name.as_deref(), + debug_msgs_allowed, + )? .execute(); } return r.execute(); @@ -838,13 +860,13 @@ pub(crate) fn try_run_package_or_file( } } - match try_execute_local_package(args, &sv) { + match try_execute_local_package(args, &sv, debug_msgs_allowed) { Ok(o) => return Ok(o), Err(ExecuteLocalPackageError::DuringExec(e)) => return Err(e), _ => {} } - if debug { + if debug && isatty::stdout_isatty() { eprintln!("finding local package {} failed", sv); } diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index eb03472e4ff..eab04ad6830 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -678,7 +678,6 @@ pub fn get_if_package_has_new_version( version: Option, max_timeout: Duration, ) -> Result, String> { - let host = match url::Url::parse(registry_url) { Ok(o) => match o.host_str().map(|s| s.to_string()) { Some(s) => s, @@ -1015,7 +1014,6 @@ pub fn install_package( format!("Package {version_str} not found in registries {registries_searched:?}."); for r in registries.iter() { - if !force_install { let package_has_new_version = get_if_package_has_new_version( r, From 156f0483b3c7868bcf3fb56f84b1c35cd8eb9531 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 14:24:32 +0100 Subject: [PATCH 03/12] Test that stdout doesn't contain extra messages when running without tty --- tests/integration/cli/tests/run.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration/cli/tests/run.rs b/tests/integration/cli/tests/run.rs index c98722789e0..a23b3231372 100644 --- a/tests/integration/cli/tests/run.rs +++ b/tests/integration/cli/tests/run.rs @@ -190,7 +190,7 @@ fn test_wasmer_create_exe_pirita_works() -> anyhow::Result<()> { let stdout = std::str::from_utf8(&output.stdout) .expect("stdout is not utf8! need to handle arbitrary bytes"); - if !stdout.ends_with("hello\n") { + if stdout != "hello\n" { bail!( "1 running python.wasmer failed with: stdout: {}\n\nstderr: {}", stdout, @@ -220,7 +220,7 @@ fn test_wasmer_run_pirita_works() -> anyhow::Result<()> { let stdout = std::str::from_utf8(&output.stdout) .expect("stdout is not utf8! need to handle arbitrary bytes"); - if !stdout.ends_with("hello\n") { + if stdout != "hello\n" { bail!( "1 running python.wasmer failed with: stdout: {}\n\nstderr: {}", stdout, @@ -303,7 +303,7 @@ fn test_wasmer_run_works() -> anyhow::Result<()> { let stdout = std::str::from_utf8(&output.stdout) .expect("stdout is not utf8! need to handle arbitrary bytes"); - if !stdout.ends_with("hello\n") { + if stdout != "hello\n" { bail!( "1 running python/python failed with: stdout: {}\n\nstderr: {}", stdout, @@ -323,7 +323,7 @@ fn test_wasmer_run_works() -> anyhow::Result<()> { let stdout = std::str::from_utf8(&output.stdout) .expect("stdout is not utf8! need to handle arbitrary bytes"); - if !stdout.ends_with("hello\n") { + if stdout != "hello\n" { bail!( "2 running python/python failed with: stdout: {}\n\nstderr: {}", stdout, @@ -343,7 +343,7 @@ fn test_wasmer_run_works() -> anyhow::Result<()> { let stdout = std::str::from_utf8(&output.stdout) .expect("stdout is not utf8! need to handle arbitrary bytes"); - if !stdout.ends_with("hello\n") { + if stdout != "hello\n" { bail!( "3 running python/python failed with: stdout: {}\n\nstderr: {}", stdout, From c0e33169979ad8411ff422fb1ca208216518389c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 16:09:55 +0100 Subject: [PATCH 04/12] Rework 5-minute timeout logic to query packages --- lib/registry/src/lib.rs | 204 ++++++++++++++++++++++++++++------------ 1 file changed, 142 insertions(+), 62 deletions(-) diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index eab04ad6830..d25aab32c91 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -669,6 +669,33 @@ impl QueryPackageError { } } +pub enum GetIfPackageHasNewVersionResult { + // if version = Some(...) and the ~/.wasmer/checkouts/.../{version} exists, the package is already installed + UseLocalAlreadyInstalled { + registry_host: String, + namespace: String, + name: String, + version: String, + path: PathBuf, + }, + // if version = None, check for the latest version + LocalVersionMayBeOutdated { + registry_host: String, + namespace: String, + name: String, + /// Versions that are already installed + whether they are + /// older (true) or younger (false) than the timeout + installed_versions: Vec<(String, bool)>, + }, + // registry / namespace / name / version doesn't exist yet + PackageNotInstalledYet { + registry_url: String, + namespace: String, + name: String, + version: Option, + }, +} + /// Returns true if a package has a newer version /// /// Also returns true if the package is not installed yet. @@ -677,7 +704,7 @@ pub fn get_if_package_has_new_version( name: &str, version: Option, max_timeout: Duration, -) -> Result, String> { +) -> Result { let host = match url::Url::parse(registry_url) { Ok(o) => match o.host_str().map(|s| s.to_string()) { Some(s) => s, @@ -694,65 +721,102 @@ pub fn get_if_package_has_new_version( let package_dir = match package_dir { Some(s) => s, - None => return Err(format!("package dir does not exist {name}/{namespace}")), - }; - - // all installed versions of this package - let mut all_installed_versions = Vec::new(); - - let may_have_newer_version = match version { - Some(ref o) => Some(()) - .map(|_| { - let modified = package_dir.join(&o).metadata().ok()?.modified().ok()?; - let version = semver::Version::parse(o).ok()?; - all_installed_versions.push(version); - if modified.elapsed().ok()? <= max_timeout { - Some(()) - } else { - None - } - }) - .is_some(), None => { - let read_dir = match std::fs::read_dir(&package_dir) { - Ok(o) => o, - Err(_) => return Err(format!("{}", package_dir.display())), - }; + return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + registry_url: registry_url.to_string(), + namespace: namespace.to_string(), + name: name.to_string(), + version: version.clone(), + }) + } + }; - // if there is any package younger than $max_timeout, return false - read_dir - .filter_map(|entry| { - let entry = entry.ok()?; - let version = semver::Version::parse(entry.file_name().to_str()?).ok()?; - all_installed_versions.push(version); - let modified = entry.metadata().ok()?.modified().ok()?; - if modified.elapsed().ok()? <= max_timeout { - Some(()) - } else { - None - } - }) - .next() - .is_some() + // if version is specified: look if that specific version exists + if let Some(s) = version.as_ref() { + let installed_path = package_dir.join(s).join("wapm.toml"); + if installed_path.exists() { + return Ok(GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { + registry_host: host.to_string(), + namespace: namespace.to_string(), + name: name.to_string(), + version: s.clone(), + path: installed_path, + }); + } else { + return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + registry_url: registry_url.to_string(), + namespace: namespace.to_string(), + name: name.to_string(), + version: Some(s.clone()), + }); } + } + + // version has not been explicitly specified: check if any package < duration exists + let read_dir = match std::fs::read_dir(&package_dir) { + Ok(o) => o, + Err(_) => return Err(format!("{}", package_dir.display())), }; - // If the version is specified, don't check for updates - // (since the package returned from the server would be the same) - if all_installed_versions.is_empty() || (may_have_newer_version && version.is_none()) { - let available_packages = - query_available_packages_from_registry(registry_url, name).unwrap_or_default(); + // all installed versions of this package + let all_installed_versions = read_dir + .filter_map(|entry| { + let entry = entry.ok()?; + let version = semver::Version::parse(entry.file_name().to_str()?).ok()?; + let modified = entry.metadata().ok()?.modified().ok()?; + let older_than_timeout = modified.elapsed().ok()? > max_timeout; + Some((version, older_than_timeout)) + }) + .collect::>(); - for p in available_packages { - if p.is_latest_version { - return Ok(Some(p)); - } + if all_installed_versions.is_empty() { + // package not installed yet + return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + registry_url: registry_url.to_string(), + namespace: namespace.to_string(), + name: name.to_string(), + version: version.clone(), + }); + } else if all_installed_versions + .iter() + .all(|(_, older_than_timeout)| *older_than_timeout) + { + // all packages are older than the timeout: there might be a new package available + return Ok(GetIfPackageHasNewVersionResult::LocalVersionMayBeOutdated { + registry_host: registry_url.to_string(), + namespace: namespace.to_string(), + name: name.to_string(), + installed_versions: all_installed_versions + .iter() + .map(|(key, old)| (format!("{key}"), *old)) + .collect::>(), + }); + } else { + // return the package that was younger than timeout + let younger_than_timeout_version = all_installed_versions + .iter() + .filter(|(_, older_than_timeout)| !older_than_timeout) + .next() + .unwrap(); + let version = format!("{}", younger_than_timeout_version.0); + let installed_path = package_dir.join(&version).join("wapm.toml"); + if installed_path.exists() { + return Ok(GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { + registry_host: host.to_string(), + namespace: namespace.to_string(), + name: name.to_string(), + version: version, + path: installed_path, + }); + } else { + return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + registry_url: registry_url.to_string(), + namespace: namespace.to_string(), + name: name.to_string(), + version: None, + }); } } - - Err(format!( - "Cannot find package {name:?} either locally or in registry" - )) } pub fn query_available_packages_from_registry( @@ -997,7 +1061,6 @@ pub fn install_package( None => get_all_available_registries()?, }; let mut url_of_package = None; - let mut error_packages = Vec::new(); let version_str = match version { None => name.to_string(), @@ -1010,8 +1073,7 @@ pub fn install_package( .filter_map(|s| Some(s.host_str()?.to_string())) .collect::>(); - let mut error_str = - format!("Package {version_str} not found in registries {registries_searched:?}."); + let mut errors = BTreeMap::new(); for r in registries.iter() { if !force_install { @@ -1020,10 +1082,25 @@ pub fn install_package( name, version.map(|s| s.to_string()), Duration::from_secs(60 * 5), - ); - if let Ok(Some(package)) = package_has_new_version { - url_of_package = Some((r, package)); - break; + )?; + match package_has_new_version { + GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { + registry_host, + namespace, + name, + version, + path, + } => { + return Ok(( + LocalPackage { + registry: registry_host, + name: format!("{namespace}/{name}"), + version, + }, + path, + )); + } + _ => {} } } @@ -1033,14 +1110,17 @@ pub fn install_package( break; } Err(e) => { - error_packages.push(e); + errors.insert(r.clone(), e); } } } - let mut did_you_mean = error_packages + let mut error_str = + format!("Package {version_str} not found in registries {registries_searched:?}."); + + let mut did_you_mean = errors .iter() - .flat_map(|error| { + .flat_map(|(registry, error)| { if let QueryPackageError::AmbigouusName { name, packages: _ } = error { error_str = format!("Ambigouus package name {name:?}. Please specify the package in the namespace/name format."); } From 6b41c2d0d95777e33c920dc1044aaf93f165e166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 16:26:44 +0100 Subject: [PATCH 05/12] Add and unit-test get_if_package_has_new_version --- lib/registry/src/lib.rs | 56 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index d25aab32c91..fd29ec143e3 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -669,6 +669,7 @@ impl QueryPackageError { } } +#[derive(Debug, Clone, PartialEq, PartialOrd)] pub enum GetIfPackageHasNewVersionResult { // if version = Some(...) and the ~/.wasmer/checkouts/.../{version} exists, the package is already installed UseLocalAlreadyInstalled { @@ -696,6 +697,55 @@ pub enum GetIfPackageHasNewVersionResult { }, } +#[test] +fn test_get_if_package_has_new_version() { + + let fake_registry = "https://h0.com"; + let fake_name = "namespace0/project1"; + let fake_version = "1.0.0"; + + let package_path = get_package_local_dir( + "h0.com", fake_name, fake_version + ).unwrap(); + let _ = std::fs::remove_file(&package_path.join("wapm.toml")); + let _ = std::fs::remove_file(&package_path.join("wapm.toml")); + + let r1 = get_if_package_has_new_version( + &fake_registry, + "namespace0/project1", + Some(fake_version.to_string()), + Duration::from_secs(5 * 60), + ); + + assert_eq!(r1.unwrap(), GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + registry_url: fake_registry.to_string(), + namespace: "namespace0".to_string(), + name: "project1".to_string(), + version: Some(fake_version.to_string()), + }); + + let package_path = get_package_local_dir( + "h0.com", fake_name, fake_version + ).unwrap(); + std::fs::create_dir_all(&package_path).unwrap(); + std::fs::write(&package_path.join("wapm.toml"), b"").unwrap(); + + let r1 = get_if_package_has_new_version( + &fake_registry, + "namespace0/project1", + Some(fake_version.to_string()), + Duration::from_secs(5 * 60), + ); + + assert_eq!(r1.unwrap(), GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { + registry_host: "h0.com".to_string(), + namespace: "namespace0".to_string(), + name: "project1".to_string(), + version: fake_version.to_string(), + path: package_path, + }); +} + /// Returns true if a package has a newer version /// /// Also returns true if the package is not installed yet. @@ -740,7 +790,7 @@ pub fn get_if_package_has_new_version( namespace: namespace.to_string(), name: name.to_string(), version: s.clone(), - path: installed_path, + path: package_dir.join(s), }); } else { return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { @@ -805,8 +855,8 @@ pub fn get_if_package_has_new_version( registry_host: host.to_string(), namespace: namespace.to_string(), name: name.to_string(), - version: version, - path: installed_path, + version: version.clone(), + path: package_dir.join(&version), }); } else { return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { From 10ad7521cab39c977211f66675856dc9cbbb0d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 16:27:31 +0100 Subject: [PATCH 06/12] cargo fmt --- lib/registry/src/lib.rs | 45 +++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index fd29ec143e3..9e98c122578 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -699,14 +699,11 @@ pub enum GetIfPackageHasNewVersionResult { #[test] fn test_get_if_package_has_new_version() { - let fake_registry = "https://h0.com"; let fake_name = "namespace0/project1"; let fake_version = "1.0.0"; - - let package_path = get_package_local_dir( - "h0.com", fake_name, fake_version - ).unwrap(); + + let package_path = get_package_local_dir("h0.com", fake_name, fake_version).unwrap(); let _ = std::fs::remove_file(&package_path.join("wapm.toml")); let _ = std::fs::remove_file(&package_path.join("wapm.toml")); @@ -717,16 +714,17 @@ fn test_get_if_package_has_new_version() { Duration::from_secs(5 * 60), ); - assert_eq!(r1.unwrap(), GetIfPackageHasNewVersionResult::PackageNotInstalledYet { - registry_url: fake_registry.to_string(), - namespace: "namespace0".to_string(), - name: "project1".to_string(), - version: Some(fake_version.to_string()), - }); - - let package_path = get_package_local_dir( - "h0.com", fake_name, fake_version - ).unwrap(); + assert_eq!( + r1.unwrap(), + GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + registry_url: fake_registry.to_string(), + namespace: "namespace0".to_string(), + name: "project1".to_string(), + version: Some(fake_version.to_string()), + } + ); + + let package_path = get_package_local_dir("h0.com", fake_name, fake_version).unwrap(); std::fs::create_dir_all(&package_path).unwrap(); std::fs::write(&package_path.join("wapm.toml"), b"").unwrap(); @@ -737,13 +735,16 @@ fn test_get_if_package_has_new_version() { Duration::from_secs(5 * 60), ); - assert_eq!(r1.unwrap(), GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { - registry_host: "h0.com".to_string(), - namespace: "namespace0".to_string(), - name: "project1".to_string(), - version: fake_version.to_string(), - path: package_path, - }); + assert_eq!( + r1.unwrap(), + GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { + registry_host: "h0.com".to_string(), + namespace: "namespace0".to_string(), + name: "project1".to_string(), + version: fake_version.to_string(), + path: package_path, + } + ); } /// Returns true if a package has a newer version From 508c1f06a1850c882ce6f6473f8283771fa3ddcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 16:59:32 +0100 Subject: [PATCH 07/12] Fix "make lint" --- lib/registry/src/lib.rs | 61 ++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index 9e98c122578..98af914b9ee 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -708,7 +708,7 @@ fn test_get_if_package_has_new_version() { let _ = std::fs::remove_file(&package_path.join("wapm.toml")); let r1 = get_if_package_has_new_version( - &fake_registry, + fake_registry, "namespace0/project1", Some(fake_version.to_string()), Duration::from_secs(5 * 60), @@ -729,7 +729,7 @@ fn test_get_if_package_has_new_version() { std::fs::write(&package_path.join("wapm.toml"), b"").unwrap(); let r1 = get_if_package_has_new_version( - &fake_registry, + fake_registry, "namespace0/project1", Some(fake_version.to_string()), Duration::from_secs(5 * 60), @@ -777,7 +777,7 @@ pub fn get_if_package_has_new_version( registry_url: registry_url.to_string(), namespace: namespace.to_string(), name: name.to_string(), - version: version.clone(), + version, }) } }; @@ -787,7 +787,7 @@ pub fn get_if_package_has_new_version( let installed_path = package_dir.join(s).join("wapm.toml"); if installed_path.exists() { return Ok(GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { - registry_host: host.to_string(), + registry_host: host, namespace: namespace.to_string(), name: name.to_string(), version: s.clone(), @@ -822,12 +822,12 @@ pub fn get_if_package_has_new_version( if all_installed_versions.is_empty() { // package not installed yet - return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { registry_url: registry_url.to_string(), namespace: namespace.to_string(), name: name.to_string(), - version: version.clone(), - }); + version, + }) } else if all_installed_versions .iter() .all(|(_, older_than_timeout)| *older_than_timeout) @@ -846,26 +846,25 @@ pub fn get_if_package_has_new_version( // return the package that was younger than timeout let younger_than_timeout_version = all_installed_versions .iter() - .filter(|(_, older_than_timeout)| !older_than_timeout) - .next() + .find(|(_, older_than_timeout)| !older_than_timeout) .unwrap(); let version = format!("{}", younger_than_timeout_version.0); let installed_path = package_dir.join(&version).join("wapm.toml"); if installed_path.exists() { - return Ok(GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { - registry_host: host.to_string(), + Ok(GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { + registry_host: host, namespace: namespace.to_string(), name: name.to_string(), version: version.clone(), path: package_dir.join(&version), - }); + }) } else { - return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { registry_url: registry_url.to_string(), namespace: namespace.to_string(), name: name.to_string(), version: None, - }); + }) } } } @@ -1134,24 +1133,22 @@ pub fn install_package( version.map(|s| s.to_string()), Duration::from_secs(60 * 5), )?; - match package_has_new_version { - GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { - registry_host, - namespace, - name, - version, + if let GetIfPackageHasNewVersionResult::UseLocalAlreadyInstalled { + registry_host, + namespace, + name, + version, + path, + } = package_has_new_version + { + return Ok(( + LocalPackage { + registry: registry_host, + name: format!("{namespace}/{name}"), + version, + }, path, - } => { - return Ok(( - LocalPackage { - registry: registry_host, - name: format!("{namespace}/{name}"), - version, - }, - path, - )); - } - _ => {} + )); } } @@ -1171,7 +1168,7 @@ pub fn install_package( let mut did_you_mean = errors .iter() - .flat_map(|(registry, error)| { + .flat_map(|(_registry, error)| { if let QueryPackageError::AmbigouusName { name, packages: _ } = error { error_str = format!("Ambigouus package name {name:?}. Please specify the package in the namespace/name format."); } From 8104a96cf085e8e9c07a3ed83ecba4d18ddf9485 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 18:25:47 +0100 Subject: [PATCH 08/12] Fix bug in installation of python/python if directory doesn't exist --- lib/registry/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index 98af914b9ee..bc93f491ca4 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -806,7 +806,14 @@ pub fn get_if_package_has_new_version( // version has not been explicitly specified: check if any package < duration exists let read_dir = match std::fs::read_dir(&package_dir) { Ok(o) => o, - Err(_) => return Err(format!("{}", package_dir.display())), + Err(_) => { + return Ok(GetIfPackageHasNewVersionResult::PackageNotInstalledYet { + registry_url: registry_url.to_string(), + namespace: namespace.to_string(), + name: name.to_string(), + version, + }); + } }; // all installed versions of this package From 017762d5f5b709ec540a903c01206ce2765282a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 18:42:30 +0100 Subject: [PATCH 09/12] Fix issue with wasmer [command] args handling --- lib/cli/src/commands/run.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cli/src/commands/run.rs b/lib/cli/src/commands/run.rs index 4fd55a108e6..313253a4188 100644 --- a/lib/cli/src/commands/run.rs +++ b/lib/cli/src/commands/run.rs @@ -828,7 +828,7 @@ pub(crate) fn try_run_package_or_file( is_fake_sv = true; match try_lookup_command(&mut fake_sv) { Ok(o) => SplitVersion { - original: format!("{}@{}", o.package, o.version), + original: package.to_string(), registry: None, package: o.package, version: Some(o.version), From 15aeda5f8edfab1ad475cf28e32af400651a1d81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 19:23:25 +0100 Subject: [PATCH 10/12] Fix "make lint" --- lib/cli/src/commands/run.rs | 35 +++++++++++++++++++++++++++++++++++ lib/registry/src/lib.rs | 30 ++++++++++++++++++++++++++---- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/lib/cli/src/commands/run.rs b/lib/cli/src/commands/run.rs index 313253a4188..2b5dad57bb9 100644 --- a/lib/cli/src/commands/run.rs +++ b/lib/cli/src/commands/run.rs @@ -657,6 +657,36 @@ fn start_spinner(msg: String) -> Option { ) } +/// Before looking up a command from the registry, try to see if we have +/// the command already installed +fn try_run_local_command( + args: &[String], + sv: &SplitVersion, + debug_msgs_allowed: bool, +) -> Result<(), ExecuteLocalPackageError> { + let result = wasmer_registry::try_finding_local_command(&sv.original).ok_or_else(|| { + ExecuteLocalPackageError::BeforeExec(anyhow::anyhow!( + "could not find command {} locally", + sv.original + )) + })?; + let package_dir = result + .get_path() + .map_err(|e| ExecuteLocalPackageError::BeforeExec(anyhow::anyhow!("{e}")))?; + + // Try auto-installing the remote package + let args_without_package = fixup_args(args, &sv.original); + let mut run_args = RunWithoutFile::try_parse_from(args_without_package.iter()) + .map_err(|e| ExecuteLocalPackageError::DuringExec(e.into()))?; + run_args.command_name = sv.command.clone(); + + run_args + .into_run_args(package_dir, sv.command.as_deref(), debug_msgs_allowed) + .map_err(ExecuteLocalPackageError::DuringExec)? + .execute() + .map_err(ExecuteLocalPackageError::DuringExec) +} + pub(crate) fn try_autoinstall_package( args: &[String], sv: &SplitVersion, @@ -826,6 +856,11 @@ pub(crate) fn try_run_package_or_file( command: None, }; is_fake_sv = true; + match try_run_local_command(args, &fake_sv, debug) { + Ok(()) => return Ok(()), + Err(ExecuteLocalPackageError::DuringExec(e)) => return Err(e), + _ => {} + } match try_lookup_command(&mut fake_sv) { Ok(o) => SplitVersion { original: package.to_string(), diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index bc93f491ca4..9c82dd3f734 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -431,10 +431,6 @@ pub struct PackageDownloadInfo { pub url: String, } -pub fn get_command_local(_name: &str) -> Result { - Err("unimplemented".to_string()) -} - pub fn get_package_local_dir( registry_host: &str, name: &str, @@ -453,6 +449,19 @@ pub fn get_package_local_dir( Ok(install_dir.join(namespace).join(name).join(version)) } +pub fn try_finding_local_command(cmd: &str) -> Option { + for p in get_all_local_packages(None) { + if p.get_commands() + .unwrap_or_default() + .iter() + .any(|c| c == cmd) + { + return Some(p); + } + } + None +} + #[derive(Debug, Clone)] pub struct LocalPackage { pub registry: String, @@ -469,6 +478,19 @@ impl LocalPackage { get_package_local_dir(&host, &self.name, &self.version) } + pub fn get_commands(&self) -> Result, String> { + let toml_path = self.get_path()?.join("wapm.toml"); + let toml = std::fs::read_to_string(&toml_path) + .map_err(|e| format!("error reading {}: {e}", toml_path.display()))?; + let toml_parsed = toml::from_str::(&toml) + .map_err(|e| format!("error parsing {}: {e}", toml_path.display()))?; + Ok(toml_parsed + .command + .unwrap_or_default() + .iter() + .map(|c| c.get_name()) + .collect()) + } } /// Returns the (manifest, .wasm file name), given a package dir From 86c664067adda44f803f7c0f9de72a2d58b68a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 19:27:15 +0100 Subject: [PATCH 11/12] Add unit test to check that commands work --- tests/integration/cli/tests/run.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/integration/cli/tests/run.rs b/tests/integration/cli/tests/run.rs index a23b3231372..dafb0d3f960 100644 --- a/tests/integration/cli/tests/run.rs +++ b/tests/integration/cli/tests/run.rs @@ -352,6 +352,26 @@ fn test_wasmer_run_works() -> anyhow::Result<()> { ); } + // same test again, but this time with only the command "python" (should be looked up locally) + let output = Command::new(get_wasmer_path()) + .arg("run") + .arg("python") + .arg(format!("--mapdir=.:{}", ASSET_PATH)) + .arg("test.py") + .output()?; + + let stdout = std::str::from_utf8(&output.stdout) + .expect("stdout is not utf8! need to handle arbitrary bytes"); + + if stdout != "hello\n" { + bail!( + "3 running python/python failed with: stdout: {}\n\nstderr: {}", + stdout, + std::str::from_utf8(&output.stderr) + .expect("stderr is not utf8! need to handle arbitrary bytes") + ); + } + Ok(()) } From 2ef0bb6b6bd84283d6d5924f23460fa60ddebc3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Thu, 3 Nov 2022 22:01:43 +0100 Subject: [PATCH 12/12] Remove GetPackages query --- .../graphql/queries/get_packages.graphql | 14 -- lib/registry/src/lib.rs | 202 +++++------------- 2 files changed, 49 insertions(+), 167 deletions(-) delete mode 100644 lib/registry/graphql/queries/get_packages.graphql diff --git a/lib/registry/graphql/queries/get_packages.graphql b/lib/registry/graphql/queries/get_packages.graphql deleted file mode 100644 index 9bbd6a0e227..00000000000 --- a/lib/registry/graphql/queries/get_packages.graphql +++ /dev/null @@ -1,14 +0,0 @@ -query GetPackagesQuery ($names: [String!]!) { - package: getPackages(names:$names) { - name - private - versions { - version - isLastVersion - distribution { - downloadUrl - } - manifest - } - } -} diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index 9c82dd3f734..4d034d5815d 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; use std::env; +use std::fmt; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -104,14 +105,6 @@ pub mod graphql { )] pub(crate) struct GetPackageByCommandQuery; - #[derive(GraphQLQuery)] - #[graphql( - schema_path = "graphql/schema.graphql", - query_path = "graphql/queries/get_packages.graphql", - response_derives = "Debug" - )] - pub(crate) struct GetPackagesQuery; - #[derive(GraphQLQuery)] #[graphql( schema_path = "graphql/schema.graphql", @@ -665,28 +658,20 @@ pub fn query_command_from_registry( #[derive(Debug, Clone, PartialEq, PartialOrd)] pub enum QueryPackageError { - AmbigouusName { - name: String, - packages: Vec, - }, ErrorSendingQuery(String), NoPackageFound { name: String, version: Option, - packages: Vec, }, } -impl QueryPackageError { - pub fn get_packages(&self) -> Vec { +impl fmt::Display for QueryPackageError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - QueryPackageError::AmbigouusName { name: _, packages } - | QueryPackageError::NoPackageFound { - name: _, - version: _, - packages, - } => packages.clone(), - _ => Vec::new(), + QueryPackageError::ErrorSendingQuery(q) => write!(f, "error sending query: {q}"), + QueryPackageError::NoPackageFound { name, version } => { + write!(f, "no package found for {name:?} (version = {version:?})") + } } } } @@ -898,123 +883,54 @@ pub fn get_if_package_has_new_version( } } -pub fn query_available_packages_from_registry( +/// Returns the download info of the packages, on error returns all the available packages +/// i.e. (("foo/python", "wapm.io"), ("bar/python" "wapm.io"))) +pub fn query_package_from_registry( registry_url: &str, name: &str, -) -> Result, QueryPackageError> { - use crate::graphql::{execute_query, get_packages_query, GetPackagesQuery}; + version: Option<&str>, +) -> Result { + use crate::graphql::{execute_query, get_package_version_query, GetPackageVersionQuery}; use graphql_client::GraphQLQuery; - let q = GetPackagesQuery::build_query(get_packages_query::Variables { - names: vec![name.to_string()], + let q = GetPackageVersionQuery::build_query(get_package_version_query::Variables { + name: name.to_string(), + version: version.map(|s| s.to_string()), }); - let response: get_packages_query::ResponseData = - execute_query(registry_url, "", &q).map_err(|e| { + let response: get_package_version_query::ResponseData = execute_query(registry_url, "", &q) + .map_err(|e| { QueryPackageError::ErrorSendingQuery(format!("Error sending GetPackagesQuery: {e}")) })?; - let available_packages = response - .package - .iter() - .filter_map(|p| { - let p = p.as_ref()?; - let mut versions = Vec::new(); - - for v in p.versions.iter() { - for v in v.iter() { - let v = match v.as_ref() { - Some(s) => s, - None => continue, - }; - - let manifest = toml::from_str::(&v.manifest).ok()?; - - versions.push(PackageDownloadInfo { - registry: registry_url.to_string(), - package: p.name.clone(), - - version: v.version.clone(), - is_latest_version: v.is_last_version, - manifest: v.manifest.clone(), + let v = response.package_version.as_ref().ok_or_else(|| { + QueryPackageError::ErrorSendingQuery(format!( + "Invalid response for crate {name:?}: no manifest" + )) + })?; - commands: manifest - .command - .unwrap_or_default() - .iter() - .map(|s| s.get_name()) - .collect::>() - .join(", "), + let manifest = toml::from_str::(&v.manifest).map_err(|e| { + QueryPackageError::ErrorSendingQuery(format!("Invalid manifest for crate {name:?}: {e}")) + })?; - url: v.distribution.download_url.clone(), - }); - } - } - - Some(versions) - }) - .collect::>() - .into_iter() - .flat_map(|v| v.into_iter()) - .collect::>(); - - Ok(available_packages) -} - -/// Returns the download info of the packages, on error returns all the available packages -/// i.e. (("foo/python", "wapm.io"), ("bar/python" "wapm.io"))) -pub fn query_package_from_registry( - registry_url: &str, - name: &str, - version: Option<&str>, -) -> Result { - let available_packages = query_available_packages_from_registry(registry_url, name)?; - - if !name.contains('/') { - return Err(QueryPackageError::AmbigouusName { - name: name.to_string(), - packages: available_packages, - }); - } - - let mut queried_packages = available_packages - .iter() - .filter(|v| { - if name.contains('/') && v.package != name { - return false; - } - - if version.is_some() && version != Some(&v.version) { - return false; - } + Ok(PackageDownloadInfo { + registry: registry_url.to_string(), + package: v.package.name.clone(), - true - }) - .collect::>(); + version: v.version.clone(), + is_latest_version: v.is_last_version, + manifest: v.manifest.clone(), - let selected_package = match version { - Some(s) => queried_packages.iter().find(|p| p.version == s), - None => { - if let Some(latest) = queried_packages.iter().find(|s| s.is_latest_version) { - Some(latest) - } else { - // sort package by version, select highest - queried_packages.sort_by_key(|k| semver::Version::parse(&k.version).ok()); - queried_packages.first() - } - } - }; + commands: manifest + .command + .unwrap_or_default() + .iter() + .map(|s| s.get_name()) + .collect::>() + .join(", "), - match selected_package { - None => { - return Err(QueryPackageError::NoPackageFound { - name: name.to_string(), - version: version.as_ref().map(|s| s.to_string()), - packages: available_packages, - }); - } - Some(s) => Ok((*s).clone()), - } + url: v.distribution.download_url.clone(), + }) } pub fn get_wasmer_root_dir() -> Option { @@ -1192,35 +1108,15 @@ pub fn install_package( } } - let mut error_str = - format!("Package {version_str} not found in registries {registries_searched:?}."); - - let mut did_you_mean = errors - .iter() - .flat_map(|(_registry, error)| { - if let QueryPackageError::AmbigouusName { name, packages: _ } = error { - error_str = format!("Ambigouus package name {name:?}. Please specify the package in the namespace/name format."); - } - let packages = error.get_packages(); - packages.iter().filter_map(|f| { - let from = url::Url::parse(&f.registry).ok()?.host_str()?.to_string(); - Some(format!(" {}@{} (from {from})", f.package, f.version)) - }) - .collect::>() - .into_iter() - }) - .collect::>(); - - let did_you_mean = if did_you_mean.is_empty() { - String::new() - } else { - did_you_mean.sort(); - did_you_mean.dedup(); - format!("\r\n\r\nDid you mean:\r\n{}\r\n", did_you_mean.join("\r\n")) - }; + let errors = errors + .into_iter() + .map(|(registry, e)| format!(" {registry}: {e}")) + .collect::>() + .join("\r\n"); - let (_, package_info) = - url_of_package.ok_or_else(|| format!("{error_str}{did_you_mean}"))?; + let (_, package_info) = url_of_package.ok_or_else(|| { + format!("Package {version_str} not found in registries {registries_searched:?}.\r\n\r\nErrors:\r\n\r\n{errors}") + })?; package_info }