Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wasmer run not interpreting URLs correctly + display fixes #3370

Merged
merged 31 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
5d1d4f3
Fix #3366 - file paths could get interpreted as URLs in wasmer run
fschutt Nov 24, 2022
351b0a6
Add integration test, use different spinner library to fix #3346
fschutt Nov 24, 2022
3e4a08f
Use with_context with proper formatting
fschutt Nov 24, 2022
8674d8c
Adress review comments and fix test
fschutt Nov 24, 2022
4ec584c
Install cowsay properly
fschutt Nov 24, 2022
2145c2d
Disable test from running locally
fschutt Nov 24, 2022
cded3ac
Install wapm before running integration tests
fschutt Nov 24, 2022
75aeeda
Install wapm via cargo if not available (Windows)
fschutt Nov 24, 2022
b5c4952
Remove dependency on wapm
fschutt Nov 24, 2022
3d4c34b
Undo installing wapm automatically
fschutt Nov 24, 2022
b5be679
Remove dependency on wapm in unit tests
fschutt Nov 24, 2022
8676105
Merge branch 'master' into fix-3366
fschutt Nov 24, 2022
6eca5b9
Do not use home_dir
fschutt Nov 24, 2022
95d77d5
Update wrong error message
fschutt Nov 24, 2022
ddf1903
Fix unit test
fschutt Nov 24, 2022
429229f
Remove unnecessary fs::copy
fschutt Nov 24, 2022
4b4214d
Remove unnecessary line from gitignore
fschutt Nov 24, 2022
4040320
Make sure that the test is being run on Windows
fschutt Nov 24, 2022
3db9ac6
Fix "wasmer" -> get_wasmer_path()
fschutt Nov 25, 2022
854f9be
Fix Makefile on Windows
fschutt Nov 25, 2022
7df82ec
Merge branch 'master' into fix-3366
fschutt Nov 25, 2022
1969692
Windows: make path complex by canonicalizing
fschutt Nov 25, 2022
6d559c2
Fix wasmer_run_complex_url on Windows GitHub CI
fschutt Nov 25, 2022
c352e1c
Fix compilation error in complex_url test
fschutt Nov 25, 2022
9285031
Remove temporary out.tar.gz
fschutt Nov 25, 2022
808bcca
Fix last compile error on Windows
fschutt Nov 25, 2022
468c587
Windows: test that path actually conflicts with URL
fschutt Nov 25, 2022
fda1a3f
Replace \\ with / on Windows
fschutt Nov 25, 2022
1594c88
Debug why Windows can't find qjs.wasm
fschutt Nov 25, 2022
a0668f9
Get better at locating wasmer.exe for running integration tests
fschutt Nov 25, 2022
83a3854
Remove printlns in test_wasmer_run_complex_url
fschutt Nov 25, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 40 additions & 36 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion lib/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ wasmer-vfs = { version = "=3.0.1", path = "../vfs", default-features = false, f
atty = "0.2"
colored = "2.0"
anyhow = "1.0"
spinner = "0.5.0"
spinoff = "0.5.4"
clap = { version = "3.2.22", features = ["derive", "env"] }
# For the function names autosuggestion
distance = "0.4"
Expand Down Expand Up @@ -166,6 +166,9 @@ http = [
"serde",
]

[target.'cfg(target_os = "windows")'.dependencies]
colored = "2.0.0"

[package.metadata.binstall]
pkg-fmt = "tgz"

Expand Down
69 changes: 43 additions & 26 deletions lib/cli/src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,17 +645,20 @@ impl Run {
}
}

fn start_spinner(msg: String) -> Option<spinner::SpinnerHandle> {
fn start_spinner(msg: String) -> Option<spinoff::Spinner> {
if !isatty::stdout_isatty() {
return None;
}
Some(
spinner::SpinnerBuilder::new(msg)
.spinner(vec![
"⣾", "⣽", "⣻", "⢿", "⡿", "⣟", "⣯", "⣷", " ", "⠁", "⠂", "⠄", "⡀", "⢀", "⠠", "⠐", "⠈",
])
.start(),
)
#[cfg(target_os = "windows")]
{
use colored::control;
let _ = control::set_virtual_terminal(true);
}
Some(spinoff::Spinner::new(
spinoff::Spinners::Dots,
msg,
spinoff::Color::White,
))
}

/// Before looking up a command from the registry, try to see if we have
Expand Down Expand Up @@ -706,8 +709,7 @@ pub(crate) fn try_autoinstall_package(
force_install,
);
if let Some(sp) = sp.take() {
sp.close();
print!("\r");
sp.clear();
}
let _ = std::io::stdout().flush();
let (_, package_dir) = match result {
Expand Down Expand Up @@ -765,8 +767,8 @@ fn try_lookup_command(sv: &mut SplitVersion) -> Result<PackageDownloadInfo, anyh

for registry in wasmer_registry::get_all_available_registries().unwrap_or_default() {
let result = wasmer_registry::query_command_from_registry(&registry, &sv.package);
if sp.is_some() {
print!("\r");
if let Some(s) = sp.take() {
s.clear();
}
let _ = std::io::stdout().flush();
let command = sv.package.clone();
Expand All @@ -779,8 +781,7 @@ fn try_lookup_command(sv: &mut SplitVersion) -> Result<PackageDownloadInfo, anyh
}

if let Some(sp) = sp.take() {
sp.close();
print!("\r");
sp.clear();
}
let _ = std::io::stdout().flush();
Err(anyhow::anyhow!("command {sv} not found"))
Expand Down Expand Up @@ -828,11 +829,6 @@ pub(crate) fn try_run_package_or_file(
) -> Result<(), anyhow::Error> {
let debug_msgs_allowed = isatty::stdout_isatty();

if let Ok(url) = url::Url::parse(&format!("{}", r.path.display())) {
let result = try_run_url(&url, args, r, debug);
return result;
}

// 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() {
Expand All @@ -848,6 +844,18 @@ pub(crate) fn try_run_package_or_file(
return r.execute();
}

// c:// might be parsed as a URL on Windows
let url_string = format!("{}", r.path.display());
if let Ok(url) = url::Url::parse(&url_string) {
if url.scheme() == "http" || url.scheme() == "https" {
match try_run_url(&url, args, r, debug) {
Err(ExecuteLocalPackageError::BeforeExec(_)) => {}
Err(ExecuteLocalPackageError::DuringExec(e)) => return Err(e),
Ok(o) => return Ok(o),
}
}
}

let package = format!("{}", r.path.display());

let mut is_fake_sv = false;
Expand Down Expand Up @@ -915,9 +923,15 @@ pub(crate) fn try_run_package_or_file(
try_autoinstall_package(args, &sv, package_download_info, r.force_install)
}

fn try_run_url(url: &Url, _args: &[String], r: &Run, _debug: bool) -> Result<(), anyhow::Error> {
let checksum = wasmer_registry::get_remote_webc_checksum(url)
.map_err(|e| anyhow::anyhow!("error fetching {url}: {e}"))?;
fn try_run_url(
url: &Url,
_args: &[String],
r: &Run,
_debug: bool,
) -> Result<(), ExecuteLocalPackageError> {
let checksum = wasmer_registry::get_remote_webc_checksum(url).map_err(|e| {
ExecuteLocalPackageError::BeforeExec(anyhow::anyhow!("error fetching {url}: {e}"))
})?;

let packages = wasmer_registry::get_all_installed_webc_packages();

Expand All @@ -926,20 +940,23 @@ fn try_run_url(url: &Url, _args: &[String], r: &Run, _debug: bool) -> Result<(),

let result = wasmer_registry::install_webc_package(url, &checksum);

result.map_err(|e| anyhow::anyhow!("error fetching {url}: {e}"))?;
result.map_err(|e| {
ExecuteLocalPackageError::BeforeExec(anyhow::anyhow!("error fetching {url}: {e}"))
})?;

if let Some(sp) = sp {
sp.close();
sp.clear();
}
}

let webc_dir = wasmer_registry::get_webc_dir();

let webc_install_path = webc_dir
.context("Error installing package: no webc dir")?
.context("Error installing package: no webc dir")
.map_err(ExecuteLocalPackageError::BeforeExec)?
.join(checksum);

let mut r = r.clone();
r.path = webc_install_path;
r.execute()
r.execute().map_err(ExecuteLocalPackageError::DuringExec)
}
5 changes: 3 additions & 2 deletions lib/registry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ pub fn query_package_from_registry(

let v = response.package_version.as_ref().ok_or_else(|| {
QueryPackageError::ErrorSendingQuery(format!(
"Invalid response for crate {name:?}: no manifest"
"Invalid response for crate {name:?}: no package version: {response:#?}"
))
})?;

Expand Down Expand Up @@ -1136,7 +1136,8 @@ pub fn get_checksum_hash(bytes: &[u8]) -> String {
/// file is already installed before downloading it
pub fn get_remote_webc_checksum(url: &Url) -> Result<String, anyhow::Error> {
let request_max_bytes = webc::WebC::get_signature_offset_start() + 4 + 1024 + 8 + 8;
let data = get_webc_bytes(url, Some(0..request_max_bytes)).context("get_webc_bytes failed")?;
let data = get_webc_bytes(url, Some(0..request_max_bytes))
.with_context(|| format!("get_webc_bytes failed on {url}"))?;
let checksum = webc::WebC::get_checksum_bytes(&data)
.map_err(|e| anyhow::anyhow!("{e}"))
.context("get_checksum_bytes failed")?
Expand Down
50 changes: 50 additions & 0 deletions tests/integration/cli/tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,3 +546,53 @@ fn run_no_start_wasm_report_error() -> anyhow::Result<()> {
assert_eq!(result.contains("Can not find any export functions."), true);
Ok(())
}

// Test that changes to wapm run don't break wasmer run
#[test]
fn test_wapm_run_works() -> anyhow::Result<()> {
fschutt marked this conversation as resolved.
Show resolved Hide resolved
// only run this test on the CI, not locally
if std::env::var("GITHUB_TOKEN").is_err() {
return Ok(());
}

let temp_dir = tempfile::tempdir()?;
let path = temp_dir.path();
let output = Command::new("wapm")
.arg("install")
.arg("cowsay")
.current_dir(&path)
.output()?;

if !output.status.success() {
bail!(
"wapm install cowsay 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 output = Command::new("wapm")
.arg("run")
.arg("cowsay")
.arg("hello")
.current_dir(&path)
.env(
"WAPM_RUNTIME".to_string(),
format!("{}", get_wasmer_path().display()),
)
.output()?;

if !output.status.success() {
bail!(
"wapm install cowsay failed with: stdout: {}\n\nstderr: {}",
fschutt marked this conversation as resolved.
Show resolved Hide resolved
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")
);
}

Ok(())
}