From ce778a947e70616b435920fd6f1fc28c4b2a78d1 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Sun, 10 Nov 2024 09:00:44 +0530 Subject: [PATCH] Revert "perf(upgrade): cache downloaded binaries in DENO_DIR" (#26799) Reverts denoland/deno#26108 Tests are flaky on main https://github.com/denoland/deno/commit/01de3317424cc870913dbe85ff3b80eadaf8cc87 --- cli/download_deno_binary.rs | 97 ------------------- cli/main.rs | 1 - cli/mainrt.rs | 1 - cli/standalone/binary.rs | 74 +++++++++++--- cli/tools/upgrade.rs | 69 ++++++++++--- tests/specs/upgrade/out/upgrade.out | 1 + tests/specs/upgrade/space_in_tmp/upgrade.out | 1 + .../specs/upgrade/specific_stable/upgrade.out | 1 + 8 files changed, 117 insertions(+), 128 deletions(-) delete mode 100644 cli/download_deno_binary.rs diff --git a/cli/download_deno_binary.rs b/cli/download_deno_binary.rs deleted file mode 100644 index 4b44949d76..0000000000 --- a/cli/download_deno_binary.rs +++ /dev/null @@ -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 { - 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(()) -} diff --git a/cli/main.rs b/cli/main.rs index 59a1fb0cda..04daff6700 100644 --- a/cli/main.rs +++ b/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; diff --git a/cli/mainrt.rs b/cli/mainrt.rs index 9af61e1288..f5b798f817 100644 --- a/cli/mainrt.rs +++ b/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; diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index a81e40ad96..9e26512268 100644 --- a/cli/standalone/binary.rs +++ b/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_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 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, - ) - .await?; + 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)] diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs index ed1bc06cfb..77a9f72b80 100644 --- a/cli/tools/upgrade.rs +++ b/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 = - 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 = + 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 { + 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>, 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. diff --git a/tests/specs/upgrade/out/upgrade.out b/tests/specs/upgrade/out/upgrade.out index 5c5267c957..a2b47d0ec8 100644 --- a/tests/specs/upgrade/out/upgrade.out +++ b/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) diff --git a/tests/specs/upgrade/space_in_tmp/upgrade.out b/tests/specs/upgrade/space_in_tmp/upgrade.out index 5c5267c957..a2b47d0ec8 100644 --- a/tests/specs/upgrade/space_in_tmp/upgrade.out +++ b/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) diff --git a/tests/specs/upgrade/specific_stable/upgrade.out b/tests/specs/upgrade/specific_stable/upgrade.out index 5c5267c957..a2b47d0ec8 100644 --- a/tests/specs/upgrade/specific_stable/upgrade.out +++ b/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)