diff options
author | David Sherret <dsherret@users.noreply.github.com> | 2023-09-14 13:51:28 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-14 17:51:28 +0000 |
commit | e66d3c2c2e287879a757e12943a6d240981cb9e8 (patch) | |
tree | 827f518778324a091fa3fbbe8f533fc10b74200a /cli/npm | |
parent | 54890ee98b9068af41214b86fb693135f0998a0a (diff) |
refactor: remove `DENO_UNSTABLE_NPM_SYNC_DOWNLOAD` and custom sync functionality (#20504)
https://github.com/denoland/deno/pull/20488 enables us to remove this
functionality. This is better because our test suite is now not testing
a separate code path.
Diffstat (limited to 'cli/npm')
-rw-r--r-- | cli/npm/cache.rs | 12 | ||||
-rw-r--r-- | cli/npm/mod.rs | 1 | ||||
-rw-r--r-- | cli/npm/registry.rs | 15 | ||||
-rw-r--r-- | cli/npm/resolvers/common.rs | 16 | ||||
-rw-r--r-- | cli/npm/resolvers/local.rs | 15 |
5 files changed, 5 insertions, 54 deletions
diff --git a/cli/npm/cache.rs b/cli/npm/cache.rs index 587ce6fd0..f76bf6821 100644 --- a/cli/npm/cache.rs +++ b/cli/npm/cache.rs @@ -18,7 +18,6 @@ use deno_npm::NpmPackageCacheFolderId; use deno_runtime::deno_fs; use deno_semver::package::PackageNv; use deno_semver::Version; -use once_cell::sync::Lazy; use crate::args::CacheSetting; use crate::http_util::HttpClient; @@ -29,17 +28,6 @@ use crate::util::progress_bar::ProgressBar; use super::tarball::verify_and_extract_tarball; -static SHOULD_SYNC_DOWNLOAD: Lazy<bool> = - Lazy::new(|| std::env::var("DENO_UNSTABLE_NPM_SYNC_DOWNLOAD").is_ok()); - -/// For some of the tests, we want downloading of packages -/// to be deterministic so that the output is always the same -pub fn should_sync_download() -> bool { - // this gets called a lot when doing npm resolution and was taking - // a significant amount of time, so cache it in a lazy - *SHOULD_SYNC_DOWNLOAD -} - const NPM_PACKAGE_SYNC_LOCK_FILENAME: &str = ".deno_sync_lock"; pub fn with_folder_sync_lock( diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 0191d8cd7..41eb09a57 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -7,7 +7,6 @@ mod resolution; mod resolvers; mod tarball; -pub use cache::should_sync_download; pub use cache::NpmCache; pub use cache::NpmCacheDir; pub use installer::PackageJsonDepsInstaller; diff --git a/cli/npm/registry.rs b/cli/npm/registry.rs index ec0647023..e960d926f 100644 --- a/cli/npm/registry.rs +++ b/cli/npm/registry.rs @@ -29,9 +29,7 @@ use crate::http_util::HttpClient; use crate::util::fs::atomic_write_file; use crate::util::progress_bar::ProgressBar; use crate::util::sync::AtomicFlag; -use crate::util::sync::TaskQueue; -use super::cache::should_sync_download; use super::cache::NpmCache; static NPM_REGISTRY_DEFAULT_URL: Lazy<Url> = Lazy::new(|| { @@ -106,24 +104,13 @@ impl CliNpmRegistryApi { } } -static SYNC_DOWNLOAD_TASK_QUEUE: Lazy<TaskQueue> = - Lazy::new(TaskQueue::default); - #[async_trait] impl NpmRegistryApi for CliNpmRegistryApi { async fn package_info( &self, name: &str, ) -> Result<Arc<NpmPackageInfo>, NpmRegistryPackageInfoLoadError> { - let result = if should_sync_download() { - let inner = self.inner().clone(); - SYNC_DOWNLOAD_TASK_QUEUE - .run(async move { inner.maybe_package_info(name).await }) - .await - } else { - self.inner().maybe_package_info(name).await - }; - match result { + match self.inner().maybe_package_info(name).await { Ok(Some(info)) => Ok(info), Ok(None) => Err(NpmRegistryPackageInfoLoadError::PackageNotExists { package_name: name.to_string(), diff --git a/cli/npm/resolvers/common.rs b/cli/npm/resolvers/common.rs index fec96738e..1991b2c72 100644 --- a/cli/npm/resolvers/common.rs +++ b/cli/npm/resolvers/common.rs @@ -20,7 +20,6 @@ use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolutionMode; -use crate::npm::cache::should_sync_download; use crate::npm::NpmCache; /// Part of the resolution that interacts with the file system. @@ -127,17 +126,10 @@ impl RegistryReadPermissionChecker { /// Caches all the packages in parallel. pub async fn cache_packages( - mut packages: Vec<NpmResolutionPackage>, + packages: Vec<NpmResolutionPackage>, cache: &Arc<NpmCache>, registry_url: &Url, ) -> Result<(), AnyError> { - let sync_download = should_sync_download(); - if sync_download { - // we're running the tests not with --quiet - // and we want the output to be deterministic - packages.sort_by(|a, b| a.id.cmp(&b.id)); - } - let mut handles = Vec::with_capacity(packages.len()); for package in packages { let cache = cache.clone(); @@ -147,11 +139,7 @@ pub async fn cache_packages( .ensure_package(&package.id.nv, &package.dist, ®istry_url) .await }); - if sync_download { - handle.await??; - } else { - handles.push(handle); - } + handles.push(handle); } let results = futures::future::join_all(handles).await; for result in results { diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index f8d7c2848..afa95e756 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -42,7 +42,6 @@ use serde::Deserialize; use serde::Serialize; use crate::npm::cache::mixed_case_package_name_encode; -use crate::npm::cache::should_sync_download; use crate::npm::resolution::NpmResolution; use crate::npm::NpmCache; use crate::util::fs::copy_dir_recursive; @@ -300,14 +299,8 @@ async fn sync_resolution_with_fs( // // Copy (hardlink in future) <global_registry_cache>/<package_id>/ to // node_modules/.deno/<package_folder_id_folder_name>/node_modules/<package_name> - let sync_download = should_sync_download(); - let mut package_partitions = + let package_partitions = snapshot.all_system_packages_partitioned(system_info); - if sync_download { - // we're running the tests not with --quiet - // and we want the output to be deterministic - package_partitions.packages.sort_by(|a, b| a.id.cmp(&b.id)); - } let mut handles: Vec<JoinHandle<Result<(), AnyError>>> = Vec::with_capacity(package_partitions.packages.len()); let mut newest_packages_by_name: HashMap<&String, &NpmResolutionPackage> = @@ -363,11 +356,7 @@ async fn sync_resolution_with_fs( drop(pb_guard); // explicit for clarity Ok(()) }); - if sync_download { - handle.await??; - } else { - handles.push(handle); - } + handles.push(handle); } } |