From f2e30a6f7999fa6ce4ab789266927e3abbb48e3f Mon Sep 17 00:00:00 2001 From: Yusuke Tanaka Date: Tue, 8 Aug 2023 10:07:29 -0700 Subject: [PATCH] refactor(cli): move `snapshot_from_lockfile` function to `deno_npm` (#20024) This commit moves `snapshot_from_lockfile` function to [deno_npm crate](https://github.com/denoland/deno_npm). This allows this function to be called outside Deno CLI (in particular, Deno Deploy). --- Cargo.lock | 13 ++-- Cargo.toml | 4 +- cli/Cargo.toml | 2 +- cli/args/lockfile.rs | 109 +++-------------------------- cli/args/mod.rs | 21 +++--- cli/lsp/language_server.rs | 32 +++++---- cli/npm/registry.rs | 40 +++++------ cli/npm/resolution.rs | 1 + cli/tests/integration/npm_tests.rs | 3 +- 9 files changed, 70 insertions(+), 155 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 81a07018ed..0e38867c01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1248,9 +1248,9 @@ dependencies = [ [[package]] name = "deno_lockfile" -version = "0.14.1" +version = "0.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa35b176a415501662244f99846b3d0dd1a7792b2135bf7f80007f8eca8b24ac" +checksum = "3e1fcc91fa4e18c3e0574965d7133709e76eda665cb589de703219f0819dfaec" dependencies = [ "ring", "serde", @@ -1353,12 +1353,13 @@ dependencies = [ [[package]] name = "deno_npm" -version = "0.10.1" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa5d1097de53e8ce3316d3e44095e253719ae367cf7478263f83082f44dddabf" +checksum = "341f2c3935bee51c15203c587213c42d120f0dc56f0aca912a4bebbf038d1030" dependencies = [ "anyhow", "async-trait", + "deno_lockfile", "deno_semver", "futures", "log", @@ -1965,9 +1966,9 @@ dependencies = [ [[package]] name = "eszip" -version = "0.48.0" +version = "0.49.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "58d2382a70396ccffd260adb879ed9f954c45457e8075830c7d4ec30c6dc8ad2" +checksum = "0b32387a359104b88abe2583679e6dcb817aff5534204ade9e4c4528db8079f3" dependencies = [ "anyhow", "base64 0.21.0", diff --git a/Cargo.toml b/Cargo.toml index 4a0daec2cd..952aad1fa5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,9 +47,9 @@ deno_runtime = { version = "0.122.0", path = "./runtime" } napi_sym = { version = "0.44.0", path = "./cli/napi/sym" } deno_bench_util = { version = "0.108.0", path = "./bench_util" } test_util = { path = "./test_util" } -deno_lockfile = "0.14.1" +deno_lockfile = "0.15.0" deno_media_type = { version = "0.1.1", features = ["module_specifier"] } -deno_npm = "0.10.1" +deno_npm = "0.11.0" deno_semver = "0.3.0" # exts diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 4896f240c2..97cfef0a69 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -57,7 +57,7 @@ deno_npm.workspace = true deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] } deno_semver.workspace = true deno_task_shell = "=0.13.1" -eszip = "=0.48.0" +eszip = "=0.49.0" napi_sym.workspace = true async-trait.workspace = true diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs index 9903ba6421..dd976862e4 100644 --- a/cli/args/lockfile.rs +++ b/cli/args/lockfile.rs @@ -1,25 +1,14 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; -use deno_core::anyhow::bail; -use deno_core::anyhow::Context; use deno_core::error::AnyError; -use deno_core::futures::stream::FuturesOrdered; -use deno_core::futures::StreamExt; use deno_core::parking_lot::Mutex; use deno_npm::registry::NpmRegistryApi; -use deno_npm::resolution::SerializedNpmResolutionSnapshot; -use deno_npm::resolution::SerializedNpmResolutionSnapshotPackage; use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot; -use deno_npm::NpmPackageId; -use deno_npm::NpmResolutionPackageSystemInfo; -use deno_semver::npm::NpmPackageReq; use crate::args::ConfigFile; -use crate::npm::CliNpmRegistryApi; use crate::Flags; use super::DenoSubcommand; @@ -63,96 +52,14 @@ pub fn discover( pub async fn snapshot_from_lockfile( lockfile: Arc>, - api: &CliNpmRegistryApi, + api: &dyn NpmRegistryApi, ) -> Result { - let (root_packages, mut packages) = { - let lockfile = lockfile.lock(); - - let mut root_packages = - HashMap::::with_capacity( - lockfile.content.npm.specifiers.len(), - ); - // collect the specifiers to version mappings - for (key, value) in &lockfile.content.npm.specifiers { - let package_req = NpmPackageReq::from_str(key) - .with_context(|| format!("Unable to parse npm specifier: {key}"))?; - let package_id = NpmPackageId::from_serialized(value)?; - root_packages.insert(package_req, package_id.clone()); - } - - // now fill the packages except for the dist information - let mut packages = Vec::with_capacity(lockfile.content.npm.packages.len()); - for (key, package) in &lockfile.content.npm.packages { - let id = NpmPackageId::from_serialized(key)?; - - // collect the dependencies - let mut dependencies = HashMap::with_capacity(package.dependencies.len()); - for (name, specifier) in &package.dependencies { - let dep_id = NpmPackageId::from_serialized(specifier)?; - dependencies.insert(name.clone(), dep_id); - } - - packages.push(SerializedNpmResolutionSnapshotPackage { - id, - dependencies, - // temporarily empty - system: Default::default(), - dist: Default::default(), - optional_dependencies: Default::default(), - }); - } - (root_packages, packages) + let incomplete_snapshot = { + let lock = lockfile.lock(); + deno_npm::resolution::incomplete_snapshot_from_lockfile(&lock)? }; - - // now that the lockfile is dropped, fetch the package version information - let pkg_nvs = packages.iter().map(|p| p.id.nv.clone()).collect::>(); - let get_version_infos = || { - FuturesOrdered::from_iter(pkg_nvs.iter().map(|nv| async move { - let package_info = api.package_info(&nv.name).await?; - match package_info.version_info(nv) { - Ok(version_info) => Ok(version_info), - Err(err) => { - bail!("Could not find '{}' specified in the lockfile.", err.0); - } - } - })) - }; - let mut version_infos = get_version_infos(); - let mut i = 0; - while let Some(result) = version_infos.next().await { - match result { - Ok(version_info) => { - let package = &mut packages[i]; - package.dist = version_info.dist; - package.system = NpmResolutionPackageSystemInfo { - cpu: version_info.cpu, - os: version_info.os, - }; - package.optional_dependencies = - version_info.optional_dependencies.into_keys().collect(); - } - Err(err) => { - if api.mark_force_reload() { - // reset and try again - version_infos = get_version_infos(); - i = 0; - continue; - } else { - return Err(err); - } - } - } - - i += 1; - } - - // clear the memory cache to reduce memory usage - api.clear_memory_cache(); - - SerializedNpmResolutionSnapshot { - packages, - root_packages, - } - .into_valid() - .context("The lockfile is corrupt. You can recreate it with --lock-write") + let snapshot = + deno_npm::resolution::snapshot_from_lockfile(incomplete_snapshot, api) + .await?; + Ok(snapshot) } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 31a09216f3..f7f73fd4ae 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -818,16 +818,17 @@ impl CliOptions { if let Some(lockfile) = self.maybe_lockfile() { if !lockfile.lock().overwrite { - return Ok(Some( - snapshot_from_lockfile(lockfile.clone(), api) - .await - .with_context(|| { - format!( - "failed reading lockfile '{}'", - lockfile.lock().filename.display() - ) - })?, - )); + let snapshot = snapshot_from_lockfile(lockfile.clone(), api) + .await + .with_context(|| { + format!( + "failed reading lockfile '{}'", + lockfile.lock().filename.display() + ) + })?; + // clear the memory cache to reduce memory usage + api.clear_memory_cache(); + return Ok(Some(snapshot)); } } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 5b9ba94f1f..f0da80bab8 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -927,6 +927,25 @@ impl Inner { Ok(()) } + async fn get_npm_snapshot( + &self, + ) -> Option { + let lockfile = self.config.maybe_lockfile()?; + let snapshot = + match snapshot_from_lockfile(lockfile.clone(), &*self.npm.api).await { + Ok(snapshot) => snapshot, + Err(err) => { + lsp_warn!("Failed getting npm snapshot from lockfile: {}", err); + return None; + } + }; + + // clear the memory cache to reduce memory usage + self.npm.api.clear_memory_cache(); + + Some(snapshot) + } + async fn recreate_npm_services_if_necessary(&mut self) { let deno_dir = match DenoDir::new(self.maybe_global_cache_path.clone()) { Ok(deno_dir) => deno_dir, @@ -948,18 +967,7 @@ impl Inner { registry_url, &progress_bar, ); - let maybe_snapshot = match self.config.maybe_lockfile() { - Some(lockfile) => { - match snapshot_from_lockfile(lockfile.clone(), &self.npm.api).await { - Ok(snapshot) => Some(snapshot), - Err(err) => { - lsp_warn!("Failed getting npm snapshot from lockfile: {}", err); - None - } - } - } - None => None, - }; + let maybe_snapshot = self.get_npm_snapshot().await; (self.npm.resolver, self.npm.resolution) = create_npm_resolver_and_resolution( registry_url, diff --git a/cli/npm/registry.rs b/cli/npm/registry.rs index 585cbe1631..907258d3b7 100644 --- a/cli/npm/registry.rs +++ b/cli/npm/registry.rs @@ -99,27 +99,6 @@ impl CliNpmRegistryApi { &self.inner().base_url } - /// Marks that new requests for package information should retrieve it - /// from the npm registry - /// - /// Returns true if it was successfully set for the first time. - pub fn mark_force_reload(&self) -> bool { - // never force reload the registry information if reloading - // is disabled or if we're already reloading - if matches!( - self.inner().cache.cache_setting(), - CacheSetting::Only | CacheSetting::ReloadAll - ) { - return false; - } - if self.inner().force_reload_flag.raise() { - self.clear_memory_cache(); // clear the memory cache to force reloading - true - } else { - false - } - } - fn inner(&self) -> &Arc { // this panicking indicates a bug in the code where this // wasn't initialized @@ -154,6 +133,23 @@ impl NpmRegistryApi for CliNpmRegistryApi { } } } + + fn mark_force_reload(&self) -> bool { + // never force reload the registry information if reloading + // is disabled or if we're already reloading + if matches!( + self.inner().cache.cache_setting(), + CacheSetting::Only | CacheSetting::ReloadAll + ) { + return false; + } + if self.inner().force_reload_flag.raise() { + self.clear_memory_cache(); // clear the cache to force reloading + true + } else { + false + } + } } type CacheItemPendingResult = @@ -386,7 +382,7 @@ impl CliNpmRegistryApiInner { name_folder_path.join("registry.json") } - pub fn clear_memory_cache(&self) { + fn clear_memory_cache(&self) { self.mem_cache.lock().clear(); } diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs index 6992d875e0..6beb520900 100644 --- a/cli/npm/resolution.rs +++ b/cli/npm/resolution.rs @@ -11,6 +11,7 @@ use deno_core::TaskQueue; use deno_lockfile::NpmPackageDependencyLockfileInfo; use deno_lockfile::NpmPackageLockfileInfo; use deno_npm::registry::NpmPackageInfo; +use deno_npm::registry::NpmRegistryApi; use deno_npm::resolution::NpmPackageVersionResolutionError; use deno_npm::resolution::NpmPackagesPartitioned; use deno_npm::resolution::NpmResolutionError; diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index c44335efc7..fb8d88dcac 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -1915,7 +1915,8 @@ fn reload_info_not_found_cache_but_exists_remote() { "error: failed reading lockfile '[WILDCARD]deno.lock'\n", "\n", "Caused by:\n", - " Could not find '@denotest/esm-basic@1.0.0' specified in the lockfile.\n" + " 0: Could not find '@denotest/esm-basic@1.0.0' specified in the lockfile.\n", + " 1: Could not find version '1.0.0' for npm package '@denotest/esm-basic'.\n", )); output.assert_exit_code(1);