Revert "perf(upgrade): cache downloaded binaries in DENO_DIR" (#26799)

Reverts denoland/deno#26108

Tests are flaky on main
01de331742

cli/download_deno_binary.rs

@@ -1,97 +0,0 @@
-// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
-
-use std::path::Path;
-use std::path::PathBuf;
-
-use crate::http_util::HttpClient;
-use crate::http_util::HttpClientProvider;
-use crate::util::progress_bar::ProgressBar;
-use crate::util::progress_bar::ProgressBarStyle;
-use deno_core::anyhow::bail;
-use deno_core::error::AnyError;
-
-use crate::cache::DenoDir;
-use crate::shared::ReleaseChannel;
-
-#[derive(Clone, Copy, Debug)]
-pub enum BinaryKind {
-  Deno,
-  Denort,
-}
-
-impl BinaryKind {
-  pub fn name(&self) -> &str {
-    match self {
-      BinaryKind::Deno => "deno",
-      BinaryKind::Denort => "denort",
-    }
-  }
-}
-
-pub async fn download_deno_binary(
-  http_client_provider: &HttpClientProvider,
-  deno_dir: &DenoDir,
-  binary_kind: BinaryKind,
-  target: &str,
-  version_or_git_hash: &str,
-  release_channel: ReleaseChannel,
-) -> Result<PathBuf, AnyError> {
-  let binary_name = archive_name(binary_kind, target);
-  let binary_path_suffix = match release_channel {
-    ReleaseChannel::Canary => {
-      format!("canary/{}/{}", version_or_git_hash, binary_name,)
-    }
-    _ => {
-      format!("release/v{}/{}", version_or_git_hash, binary_name)
-    }
-  };
-
-  let download_directory = deno_dir.dl_folder_path();
-  let binary_path = download_directory.join(&binary_path_suffix);
-
-  if !binary_path.exists() {
-    let http_client = http_client_provider.get_or_create()?;
-    download_base_binary(
-      &http_client,
-      &download_directory,
-      &binary_path_suffix,
-    )
-    .await?;
-  }
-
-  Ok(binary_path)
-}
-
-pub fn archive_name(binary_kind: BinaryKind, target: &str) -> String {
-  format!("{}-{}.zip", binary_kind.name(), target)
-}
-
-async fn download_base_binary(
-  http_client: &HttpClient,
-  output_directory: &Path,
-  binary_path_suffix: &str,
-) -> Result<(), AnyError> {
-  let download_url = format!("https://dl.deno.land/{binary_path_suffix}");
-  let maybe_bytes = {
-    let progress_bars = ProgressBar::new(ProgressBarStyle::DownloadBars);
-    // provide an empty string here in order to prefer the downloading
-    // text above which will stay alive after the progress bars are complete
-    let progress = progress_bars.update("");
-    http_client
-      .download_with_progress_and_retries(
-        download_url.parse()?,
-        None,
-        &progress,
-      )
-      .await?
-  };
-  let Some(bytes) = maybe_bytes else {
-    bail!("Failed downloading {download_url}. The version you requested may not have been built for the current architecture.");
-  };
-
-  std::fs::create_dir_all(output_directory)?;
-  let output_path = output_directory.join(binary_path_suffix);
-  std::fs::create_dir_all(output_path.parent().unwrap())?;
-  tokio::fs::write(output_path, bytes).await?;
-  Ok(())
-}

cli/main.rs

@@ -4,7 +4,6 @@ mod args;
 mod auth_tokens;
 mod cache;
 mod cdp;
-mod download_deno_binary;
 mod emit;
 mod errors;
 mod factory;

cli/mainrt.rs

@@ -10,7 +10,6 @@ mod standalone;
 mod args;
 mod auth_tokens;
 mod cache;
-mod download_deno_binary;
 mod emit;
 mod errors;
 mod file_fetcher;

cli/standalone/binary.rs

@@ -63,9 +63,6 @@ use crate::args::NpmInstallDepsProvider;
 use crate::args::PermissionFlags;
 use crate::args::UnstableConfig;
 use crate::cache::DenoDir;
-use crate::download_deno_binary::archive_name;
-use crate::download_deno_binary::download_deno_binary;
-use crate::download_deno_binary::BinaryKind;
 use crate::emit::Emitter;
 use crate::file_fetcher::FileFetcher;
 use crate::http_util::HttpClientProvider;
@@ -455,24 +452,36 @@ impl<'a> DenoCompileBinaryWriter<'a> {
     }
 
     let target = compile_flags.resolve_target();
+    let binary_name = format!("denort-{target}.zip");
 
-    let archive_name = archive_name(BinaryKind::Denort, &target);
-
-    let binary_path = download_deno_binary(
-      self.http_client_provider,
-      self.deno_dir,
-      BinaryKind::Denort,
-      &target,
-      crate::version::DENO_VERSION_INFO.version_or_git_hash(),
-      crate::version::DENO_VERSION_INFO.release_channel,
+    let binary_path_suffix =
+      match crate::version::DENO_VERSION_INFO.release_channel {
+        ReleaseChannel::Canary => {
+          format!(
+            "canary/{}/{}",
+            crate::version::DENO_VERSION_INFO.git_hash,
+            binary_name
           )
+        }
+        _ => {
+          format!("release/v{}/{}", env!("CARGO_PKG_VERSION"), binary_name)
+        }
+      };
+
+    let download_directory = self.deno_dir.dl_folder_path();
+    let binary_path = download_directory.join(&binary_path_suffix);
+
+    if !binary_path.exists() {
+      self
+        .download_base_binary(&download_directory, &binary_path_suffix)
         .await?;
+    }
 
     let archive_data = std::fs::read(binary_path)?;
     let temp_dir = tempfile::TempDir::new()?;
     let base_binary_path = archive::unpack_into_dir(archive::UnpackArgs {
-      exe_name: BinaryKind::Denort.name(),
-      archive_name: &archive_name,
+      exe_name: "denort",
+      archive_name: &binary_name,
       archive_data: &archive_data,
       is_windows: target.contains("windows"),
       dest_path: temp_dir.path(),
@@ -482,6 +491,41 @@ impl<'a> DenoCompileBinaryWriter<'a> {
     Ok(base_binary)
   }
 
+  async fn download_base_binary(
+    &self,
+    output_directory: &Path,
+    binary_path_suffix: &str,
+  ) -> Result<(), AnyError> {
+    let download_url = format!("https://dl.deno.land/{binary_path_suffix}");
+    let maybe_bytes = {
+      let progress_bars = ProgressBar::new(ProgressBarStyle::DownloadBars);
+      let progress = progress_bars.update(&download_url);
+
+      self
+        .http_client_provider
+        .get_or_create()?
+        .download_with_progress_and_retries(
+          download_url.parse()?,
+          None,
+          &progress,
+        )
+        .await?
+    };
+    let bytes = match maybe_bytes {
+      Some(bytes) => bytes,
+      None => {
+        log::info!("Download could not be found, aborting");
+        std::process::exit(1)
+      }
+    };
+
+    std::fs::create_dir_all(output_directory)?;
+    let output_path = output_directory.join(binary_path_suffix);
+    std::fs::create_dir_all(output_path.parent().unwrap())?;
+    tokio::fs::write(output_path, bytes).await?;
+    Ok(())
+  }
+
   /// This functions creates a standalone deno binary by appending a bundle
   /// and magic trailer to the currently executing binary.
   #[allow(clippy::too_many_arguments)]

cli/tools/upgrade.rs

@@ -6,14 +6,13 @@ use crate::args::Flags;
 use crate::args::UpgradeFlags;
 use crate::args::UPGRADE_USAGE;
 use crate::colors;
-use crate::download_deno_binary::archive_name;
-use crate::download_deno_binary::download_deno_binary;
-use crate::download_deno_binary::BinaryKind;
 use crate::factory::CliFactory;
 use crate::http_util::HttpClient;
 use crate::http_util::HttpClientProvider;
 use crate::shared::ReleaseChannel;
 use crate::util::archive;
+use crate::util::progress_bar::ProgressBar;
+use crate::util::progress_bar::ProgressBarStyle;
 use crate::version;
 
 use async_trait::async_trait;
@@ -35,8 +34,12 @@ use std::process::Command;
 use std::sync::Arc;
 use std::time::Duration;
 
-static ARCHIVE_NAME: Lazy<String> =
-  Lazy::new(|| archive_name(BinaryKind::Deno, env!("TARGET")));
+const RELEASE_URL: &str = "https://github.com/denoland/deno/releases";
+const CANARY_URL: &str = "https://dl.deno.land/canary";
+const DL_RELEASE_URL: &str = "https://dl.deno.land/release";
+
+pub static ARCHIVE_NAME: Lazy<String> =
+  Lazy::new(|| format!("deno-{}.zip", env!("TARGET")));
 
 // How often query server for new version. In hours.
 const UPGRADE_CHECK_INTERVAL: i64 = 24;
@@ -529,17 +532,13 @@ pub async fn upgrade(
     return Ok(());
   };
 
-  let binary_path = download_deno_binary(
-    http_client_provider,
-    factory.deno_dir()?,
-    BinaryKind::Deno,
-    env!("TARGET"),
+  let download_url = get_download_url(
     &selected_version_to_upgrade.version_or_hash,
     requested_version.release_channel(),
-  )
-  .await?;
-
-  let Ok(archive_data) = tokio::fs::read(&binary_path).await else {
+  )?;
+  log::info!("{}", colors::gray(format!("Downloading {}", &download_url)));
+  let Some(archive_data) = download_package(&client, download_url).await?
+  else {
     log::error!("Download could not be found, aborting");
     std::process::exit(1)
   };
@@ -882,6 +881,48 @@ fn base_upgrade_url() -> Cow<'static, str> {
   }
 }
 
+fn get_download_url(
+  version: &str,
+  release_channel: ReleaseChannel,
+) -> Result<Url, AnyError> {
+  let download_url = match release_channel {
+    ReleaseChannel::Stable => {
+      format!("{}/download/v{}/{}", RELEASE_URL, version, *ARCHIVE_NAME)
+    }
+    ReleaseChannel::Rc => {
+      format!("{}/v{}/{}", DL_RELEASE_URL, version, *ARCHIVE_NAME)
+    }
+    ReleaseChannel::Canary => {
+      format!("{}/{}/{}", CANARY_URL, version, *ARCHIVE_NAME)
+    }
+    ReleaseChannel::Lts => {
+      format!("{}/v{}/{}", DL_RELEASE_URL, version, *ARCHIVE_NAME)
+    }
+  };
+
+  Url::parse(&download_url).with_context(|| {
+    format!(
+      "Failed to parse URL to download new release: {}",
+      download_url
+    )
+  })
+}
+
+async fn download_package(
+  client: &HttpClient,
+  download_url: Url,
+) -> Result<Option<Vec<u8>>, AnyError> {
+  let progress_bar = ProgressBar::new(ProgressBarStyle::DownloadBars);
+  // provide an empty string here in order to prefer the downloading
+  // text above which will stay alive after the progress bars are complete
+  let progress = progress_bar.update("");
+  let maybe_bytes = client
+    .download_with_progress_and_retries(download_url.clone(), None, &progress)
+    .await
+    .with_context(|| format!("Failed downloading {download_url}. The version you requested may not have been built for the current architecture."))?;
+  Ok(maybe_bytes)
+}
+
 fn replace_exe(from: &Path, to: &Path) -> Result<(), std::io::Error> {
   if cfg!(windows) {
     // On windows you cannot replace the currently running executable.

tests/specs/upgrade/out/upgrade.out

@@ -1,4 +1,5 @@
 Current Deno version: [WILDCARD]
+Downloading https://github.com/denoland/deno/releases/download/v1.43.2/deno-[WILDCARD].zip
 Deno is upgrading to version 1.43.2
 
 Upgraded successfully to Deno v1.43.2 (stable)

tests/specs/upgrade/space_in_tmp/upgrade.out

@@ -1,4 +1,5 @@
 Current Deno version: [WILDCARD]
+Downloading https://github.com/denoland/deno/releases/download/v1.43.2/deno-[WILDCARD].zip
 Deno is upgrading to version 1.43.2
 
 Upgraded successfully to Deno v1.43.2 (stable)

tests/specs/upgrade/specific_stable/upgrade.out

@@ -1,4 +1,5 @@
 Current Deno version: [WILDCARD]
+Downloading https://github.com/denoland/deno/releases/download/v1.43.2/deno-[WILDCARD].zip
 Deno is upgrading to version 1.43.2
 
 Upgraded successfully to Deno v1.43.2 (stable)