Address review comments from #3299 (review)

wasmerio · Nov 18, 2022 · 3daec87 · 3daec87
1 parent 820b83e
commit 3daec87
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 30 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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())
 }