From 3daec87b1510afb28049f06b66c03c7027ac8085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=BCtt?= Date: Fri, 18 Nov 2022 20:17:58 +0100 Subject: [PATCH] Address review comments from https://github.com/wasmerio/wasmer/pull/3299#pullrequestreview-1186599785 --- Cargo.lock | 1 - lib/cli/Cargo.toml | 3 +-- lib/cli/src/commands/create_exe.rs | 38 ++++++++++++++---------------- lib/registry/src/lib.rs | 8 +------ 4 files changed, 20 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a26d2199541..618b65882a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3993,7 +3993,6 @@ dependencies = [ "serde", "serde_json", "spinner", - "tar", "target-lexicon 0.12.4", "tempdir", "tempfile", diff --git a/lib/cli/Cargo.toml b/lib/cli/Cargo.toml index a20693bfe6b..a7196ed8ee5 100644 --- a/lib/cli/Cargo.toml +++ b/lib/cli/Cargo.toml @@ -56,7 +56,7 @@ log = { version = "0.4", optional = true } tempfile = "3" tempdir = "0.3.7" http_req = { version="^0.8", default-features = false, features = ["rust-tls"], optional = true } -reqwest = { version = "^0.11", default-features = false, features = ["rustls-tls", "json"], optional = true } +reqwest = { version = "^0.11", default-features = false, features = ["rustls-tls", "json", "multipart"], optional = true } serde = { version = "1.0.147", features = ["derive"], optional = true } dirs = { version = "4.0", optional = true } serde_json = { version = "1.0", optional = true } @@ -71,7 +71,6 @@ 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" -tar = "0.4" dialoguer = "0.10.2" [build-dependencies] diff --git a/lib/cli/src/commands/create_exe.rs b/lib/cli/src/commands/create_exe.rs index bc69b5c0be2..77a7f9be22d 100644 --- a/lib/cli/src/commands/create_exe.rs +++ b/lib/cli/src/commands/create_exe.rs @@ -548,8 +548,6 @@ impl CreateExe { header_code_path = std::env::current_dir()?; } - println!("Output result to: {}", output_path.display()); - /* Compile main function */ let compilation = { let mut include_dir = libwasmer_path.clone(); @@ -591,7 +589,8 @@ impl CreateExe { cmd.arg(volume_obj.clone()); } - println!("{:?}", cmd); + #[cfg(feature = "debug")] + log::debug!("{:?}", cmd); cmd.output().context("Could not execute `zig`")? }; if !compilation.status.success() { @@ -1387,7 +1386,8 @@ mod http_fetch { .context("Could not lookup wasmer repository on Github.")?; if response.status_code() != StatusCode::new(200) { - eprintln!( + #[cfg(feature = "debug")] + log::warn!( "Warning: Github API replied with non-200 status code: {}. Response: {}", response.status_code(), String::from_utf8_lossy(&writer), @@ -1565,32 +1565,30 @@ mod http_fetch { .to_string(); let download_tempdir = tempdir::TempDir::new("wasmer-download")?; - let download_path = download_tempdir.path().to_path_buf().join(&filename); + let download_path = download_tempdir.path().join(&filename); let mut file = std::fs::File::create(&download_path)?; - eprintln!( + #[cfg(feature = "debug")] + log::debug!( "Downloading {} to {}", browser_download_url, download_path.display() ); - let uri = Uri::try_from(browser_download_url.as_str())?; - let mut response = Request::new(&uri) - .header("User-Agent", "wasmer") - .send(&mut file) + let mut response = reqwest::blocking::Client::builder() + .redirect(reqwest::redirect::Policy::limited(10)) + .timeout(std::time::Duration::from_secs(10)) + .build() + .map_err(anyhow::Error::new) + .context("Could not lookup wasmer artifact on Github.")? + .get(browser_download_url.as_str()) + .send() .map_err(anyhow::Error::new) .context("Could not lookup wasmer artifact on Github.")?; - if response.status_code() == StatusCode::new(302) { - let redirect_uri = - Uri::try_from(response.headers().get("Location").unwrap().as_str()).unwrap(); - response = Request::new(&redirect_uri) - .header("User-Agent", "wasmer") - .send(&mut file) - .map_err(anyhow::Error::new) - .context("Could not lookup wasmer artifact on Github.")?; - } - let _ = response; + response + .copy_to(&mut file) + .map_err(|e| anyhow::anyhow!("{e}"))?; match super::get_libwasmer_cache_path() { Ok(mut cache_path) => { diff --git a/lib/registry/src/lib.rs b/lib/registry/src/lib.rs index 3e8349e3fb8..1ecda1bc7ce 100644 --- a/lib/registry/src/lib.rs +++ b/lib/registry/src/lib.rs @@ -710,12 +710,6 @@ pub fn download_and_unpack_targz( let mut resp = reqwest::blocking::get(url) .map_err(|e| anyhow::anyhow!("failed to download {url}: {e}"))?; - if !target_targz_path.exists() { - // create all the parent paths, only remove the created directory, not the parent dirs - let _ = std::fs::create_dir_all(&target_targz_path); - let _ = std::fs::remove_dir(&target_targz_path); - } - { let mut file = std::fs::File::create(&target_targz_path).map_err(|e| { anyhow::anyhow!( @@ -729,7 +723,7 @@ pub fn download_and_unpack_targz( } try_unpack_targz(target_targz_path.as_path(), target_path, strip_toplevel) - .context(anyhow::anyhow!("Could not download {url}"))?; + .with_context(|| anyhow::anyhow!("Could not download {url}"))?; Ok(target_path.to_path_buf()) }