Merge pull request #1003 from cloudflare/avery/dont-reinstall

dont reinstall same binary versions

Cargo.toml

@@ -14,7 +14,7 @@ build = "build.rs"
 atty = "0.2.14"
 base64 = "0.10.1"
 billboard = "0.1.0"
-binary-install = "0.0.3-alpha"
+binary-install = "0.0.3-alpha.1"
 chrome-devtools-rs = { version = "0.0.0-alpha.0", features = ["color"] }
 chrono = "0.4.9"
 clap = "2.32.0"
@@ -44,6 +44,7 @@ prettytable-rs = "0.8.0"
 rand = "0.6.5"
 regex = "1"
 reqwest = { version = "0.10.4", features = ["blocking", "json"] }
+semver = "0.9.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0.39"
 serde_with = "1.3.1"
@@ -66,3 +67,6 @@ predicates = "1.0.2"
 
 [features]
 vendored-openssl = ['openssl/vendored']
+
+[profile.release]
+debug = true

src/build/mod.rs

@@ -14,7 +14,11 @@ pub fn build(target: &Target) -> Result<(), failure::Error> {
         }
         TargetType::Rust => {
             let tool_name = "wasm-pack";
-            let binary_path = install::install(tool_name, "rustwasm")?.binary(tool_name)?;
+            let tool_author = "rustwasm";
+            let is_binary = true;
+            let version = install::get_latest_version(tool_name)?;
+            let binary_path =
+                install::install(tool_name, tool_author, is_binary, version)?.binary(tool_name)?;
             let args = ["build", "--target", "no-modules"];
 
             let command = command(&args, &binary_path);

src/commands/generate.rs

@@ -55,7 +55,11 @@ pub fn generate(
 
 pub fn run_generate(name: &str, template: &str) -> Result<(), failure::Error> {
     let tool_name = "cargo-generate";
-    let binary_path = install::install(tool_name, "ashleygwilliams")?.binary(tool_name)?;
+    let tool_author = "ashleygwilliams";
+    let is_binary = true;
+    let version = install::get_latest_version(tool_name)?;
+    let binary_path =
+        install::install(tool_name, tool_author, is_binary, version)?.binary(tool_name)?;
 
     let args = ["generate", "--git", template, "--name", name, "--force"];
 

src/install/mod.rs

@@ -6,100 +6,90 @@ use crate::terminal::emoji;
 use binary_install::{Cache, Download};
 use krate::Krate;
 use log::info;
-use which::which;
+use semver::Version;
 
 use std::env;
-use std::path::Path;
-use std::process::Command;
+use std::fs;
+use std::path::{Path, PathBuf};
 
 use lazy_static::lazy_static;
 
 lazy_static! {
-    static ref CACHE: Cache = get_wrangler_cache().expect("creating binary dependency cache");
+    static ref CACHE: Cache = get_wrangler_cache().expect("Could not get Wrangler cache location");
 }
 
-pub fn install(tool_name: &str, owner: &str) -> Result<Download, failure::Error> {
-    if let Some(download) = tool_exists(tool_name)? {
-        return Ok(download);
-    }
-
-    let binaries = &[tool_name];
-    let latest_version = get_latest_version(tool_name)?;
-    let download = download_prebuilt(tool_name, owner, &latest_version, binaries);
-    match download {
-        Ok(download) => Ok(download),
-        Err(e) => {
-            failure::bail!("could not download pre-built `{}` ({}).", tool_name, e);
-        }
-    }
+enum ToolDownload {
+    NeedsInstall(Version),
+    InstalledAt(Download),
 }
 
-pub fn install_artifact(
+pub fn install(
     tool_name: &str,
     owner: &str,
-    version: &str,
+    is_binary: bool,
+    version: Version,
 ) -> Result<Download, failure::Error> {
-    if let Some(download) = tool_exists(tool_name)? {
-        return Ok(download);
-    }
-
-    let download = download_prebuilt(tool_name, owner, version, &[]);
-    match download {
-        Ok(download) => Ok(download),
-        Err(e) => {
-            failure::bail!("could not download pre-built `{}` ({}).", tool_name, e);
+    let download = match tool_needs_update(tool_name, version)? {
+        ToolDownload::NeedsInstall(version) => {
+            println!("{}  Installing {} v{}...", emoji::DOWN, tool_name, version);
+            let binaries: Vec<&str> = if is_binary { vec![tool_name] } else { vec![] };
+            let download =
+                download_prebuilt(tool_name, owner, &version.to_string(), binaries.as_ref());
+            match download {
+                Ok(download) => Ok(download),
+                Err(e) => Err(failure::format_err!(
+                    "could not download `{}`\n{}",
+                    tool_name,
+                    e
+                )),
+            }
         }
-    }
+        ToolDownload::InstalledAt(download) => Ok(download),
+    }?;
+    log::debug!("tool {} located at {:?}", tool_name, download);
+    Ok(download)
 }
 
-fn tool_exists(tool_name: &str) -> Result<Option<Download>, failure::Error> {
-    if let Ok(path) = which(tool_name) {
-        let no_parent_msg = format!("{} There is no path parent", emoji::WARN);
-        log::debug!("found global {} binary at: {}", tool_name, path.display());
-        if !tool_needs_update(tool_name, &path)? {
-            return Ok(Some(Download::at(path.parent().expect(&no_parent_msg))));
+fn tool_needs_update(
+    tool_name: &str,
+    target_version: Version,
+) -> Result<ToolDownload, failure::Error> {
+    let current_installation = get_installation(tool_name, &target_version);
+    // if something goes wrong checking the current installation
+    // we shouldn't fail, we should just re-install for them
+    if let Ok(current_installation) = current_installation {
+        if let Some((installed_version, installed_location)) = current_installation {
+            if installed_version == target_version {
+                return Ok(ToolDownload::InstalledAt(Download::at(&installed_location)));
+            }
         }
     }
-
-    Ok(None)
+    Ok(ToolDownload::NeedsInstall(target_version))
 }
 
-fn tool_needs_update(tool_name: &str, path: &Path) -> Result<bool, failure::Error> {
-    let no_version_msg = format!("failed to find version for {}", tool_name);
-
-    let tool_version_output = Command::new(path.as_os_str())
-        .arg("--version")
-        .output()
-        .expect(&no_version_msg);
-
-    if !tool_version_output.status.success() {
-        let error = String::from_utf8_lossy(&tool_version_output.stderr);
-        log::debug!("could not find version for {}\n{}", tool_name, error);
-        return Ok(true);
-    }
-
-    let installed_tool_version = String::from_utf8_lossy(&tool_version_output.stdout);
-    let installed_tool_version = match installed_tool_version.split_whitespace().last() {
-        None => return Ok(true),
-        Some(v) => v,
-    };
-    let latest_tool_version = get_latest_version(tool_name)?;
-    // TODO: If latest_tool_version is error, try getting latest version via a github api call.
-    if installed_tool_version == latest_tool_version {
-        log::debug!(
-            "installed {} version {} is up to date",
-            tool_name,
-            installed_tool_version
-        );
-        return Ok(false);
+fn get_installation(
+    tool_name: &str,
+    target_version: &Version,
+) -> Result<Option<(Version, PathBuf)>, failure::Error> {
+    for entry in fs::read_dir(&CACHE.destination)? {
+        let entry = entry?;
+        let filename = entry.file_name().into_string();
+        if let Ok(filename) = filename {
+            if filename.starts_with(tool_name) {
+                let installed_version = filename
+                    .split(&format!("{}-", tool_name))
+                    .collect::<Vec<&str>>()[1];
+                let installed_version = Version::parse(installed_version);
+                // if the installed version can't be parsed, ignore it
+                if let Ok(installed_version) = installed_version {
+                    if &installed_version == target_version {
+                        return Ok(Some((installed_version, entry.path())));
+                    }
+                }
+            }
+        }
     }
-    log::info!(
-        "installed {} version {} is out of date with latest version {}",
-        tool_name,
-        installed_tool_version,
-        latest_tool_version
-    );
-    Ok(true)
+    Ok(None)
 }
 
 fn download_prebuilt(
@@ -120,16 +110,13 @@ fn download_prebuilt(
 
     // no binaries are expected; downloading it as an artifact
     let res = if !binaries.is_empty() {
-        CACHE.download(true, tool_name, binaries, &url)?
+        CACHE.download_version(true, tool_name, binaries, &url, version)?
     } else {
-        CACHE.download_artifact(tool_name, &url)?
+        CACHE.download_artifact_version(tool_name, &url, version)?
     };
 
     match res {
-        Some(download) => {
-            println!("⬇️ Installing {}...", tool_name);
-            Ok(download)
-        }
+        Some(download) => Ok(download),
         None => failure::bail!("{} is not installed!", tool_name),
     }
 }
@@ -159,10 +146,11 @@ fn prebuilt_url(tool_name: &str, owner: &str, version: &str) -> Option<String> {
     }
 }
 
-fn get_latest_version(tool_name: &str) -> Result<String, failure::Error> {
+pub fn get_latest_version(tool_name: &str) -> Result<Version, failure::Error> {
     // TODO: return the latest version pulled from github api, not via Krate.
-    // TODO(argo tunnel team): make cloudflared binary available on github.
-    Ok(Krate::new(tool_name)?.max_version)
+    let latest_version = Krate::new(tool_name)?.max_version;
+    Version::parse(&latest_version)
+        .map_err(|e| failure::format_err!("could not parse latest version\n{}", e))
 }
 
 fn get_wrangler_cache() -> Result<Cache, failure::Error> {

src/terminal/emoji.rs

@@ -17,6 +17,7 @@ pub static BICEP: Emoji = Emoji("💪 ", "");
 pub static CRAB: Emoji = Emoji("🦀 ", "");
 pub static COMPUTER: Emoji = Emoji("💻 ", "");
 pub static DANCERS: Emoji = Emoji("👯 ", "");
+pub static DOWN: Emoji = Emoji("⬇️ ", "");
 pub static EAR: Emoji = Emoji("👂 ", "");
 pub static EYES: Emoji = Emoji("👀 ", "");
 pub static FILES: Emoji = Emoji("🗂️ ", "");

src/watch/mod.rs

@@ -53,7 +53,11 @@ pub fn watch_and_build(
         }
         TargetType::Rust => {
             let tool_name = "wasm-pack";
-            let binary_path = install::install(tool_name, "rustwasm")?.binary(tool_name)?;
+            let tool_author = "rustwasm";
+            let is_binary = true;
+            let version = install::get_latest_version(tool_name)?;
+            let binary_path =
+                install::install(tool_name, tool_author, is_binary, version)?.binary(tool_name)?;
             let args = ["build", "--target", "no-modules"];
 
             thread::spawn(move || {

src/wranglerjs/mod.rs

@@ -19,6 +19,7 @@ use notify::{self, RecursiveMode, Watcher};
 use output::WranglerjsOutput;
 use rand::distributions::Alphanumeric;
 use rand::{thread_rng, Rng};
+use semver::Version;
 
 use crate::install;
 use crate::settings::toml::Target;
@@ -168,7 +169,11 @@ fn setup_build(target: &Target) -> Result<(Command, PathBuf, Bundle), failure::E
 
     // export WASM_PACK_PATH for use by wasm-pack-plugin
     // https://github.com/wasm-tool/wasm-pack-plugin/blob/caca20df84782223f002735a8a2e99b2291f957c/plugin.js#L13
-    let wasm_pack_path = install::install("wasm-pack", "rustwasm")?.binary("wasm-pack")?;
+    let tool_name = "wasm-pack";
+    let tool_author = "rustwasm";
+    let version = install::get_latest_version(tool_name)?;
+    let wasm_pack_path =
+        install::install(tool_name, tool_author, true, version)?.binary("wasm-pack")?;
     command.env("WASM_PACK_PATH", wasm_pack_path);
 
     // create a temp file for IPC with the wranglerjs process
@@ -303,8 +308,10 @@ fn install() -> Result<PathBuf, failure::Error> {
         wranglerjs_path
     } else {
         let tool_name = "wranglerjs";
-        let version = env!("CARGO_PKG_VERSION");
-        let wranglerjs_path = install::install_artifact(tool_name, "cloudflare", version)?;
+        let tool_author = "cloudflare";
+        let is_binary = false;
+        let version = Version::parse(env!("CARGO_PKG_VERSION"))?;
+        let wranglerjs_path = install::install(tool_name, tool_author, is_binary, version)?;
         log::info!("wranglerjs downloaded at: {:?}", wranglerjs_path.path());
         wranglerjs_path.path()
     };