summaryrefslogtreecommitdiff
path: root/cli
diff options
context:
space:
mode:
Diffstat (limited to 'cli')
-rw-r--r--cli/graph_util.rs8
-rw-r--r--cli/npm/installer.rs23
-rw-r--r--cli/npm/registry.rs11
-rw-r--r--cli/proc_state.rs2
-rw-r--r--cli/resolver.rs33
-rw-r--r--cli/tests/integration/cache_tests.rs20
-rw-r--r--cli/util/mod.rs1
-rw-r--r--cli/util/sync.rs35
8 files changed, 92 insertions, 41 deletions
diff --git a/cli/graph_util.rs b/cli/graph_util.rs
index 8ea48702a..df4e5132d 100644
--- a/cli/graph_util.rs
+++ b/cli/graph_util.rs
@@ -187,6 +187,7 @@ pub async fn create_graph_and_maybe_check(
let mut graph = ModuleGraph::default();
build_graph_with_npm_resolution(
&mut graph,
+ &cli_resolver,
&ps.npm_resolver,
roots,
&mut cache,
@@ -249,6 +250,7 @@ pub async fn create_graph_and_maybe_check(
pub async fn build_graph_with_npm_resolution<'a>(
graph: &mut ModuleGraph,
+ cli_graph_resolver: &CliGraphResolver,
npm_resolver: &NpmPackageResolver,
roots: Vec<ModuleSpecifier>,
loader: &mut dyn deno_graph::source::Loader,
@@ -256,6 +258,12 @@ pub async fn build_graph_with_npm_resolution<'a>(
) -> Result<(), AnyError> {
graph.build(roots, loader, options).await;
+ // ensure that the top level package.json is installed if a
+ // specifier was matched in the package.json
+ cli_graph_resolver
+ .top_level_package_json_install_if_necessary()
+ .await?;
+
// resolve the dependencies of any pending dependencies
// that were inserted by building the graph
npm_resolver.resolve_pending().await?;
diff --git a/cli/npm/installer.rs b/cli/npm/installer.rs
index 6d048f7ca..41d85a9b9 100644
--- a/cli/npm/installer.rs
+++ b/cli/npm/installer.rs
@@ -1,6 +1,5 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
-use std::sync::atomic::AtomicBool;
use std::sync::Arc;
use deno_core::error::AnyError;
@@ -9,13 +8,14 @@ use deno_core::futures::StreamExt;
use deno_npm::registry::NpmRegistryApi;
use crate::args::package_json::PackageJsonDeps;
+use crate::util::sync::AtomicFlag;
use super::NpmRegistry;
use super::NpmResolution;
#[derive(Debug)]
struct PackageJsonDepsInstallerInner {
- has_installed: AtomicBool,
+ has_installed_flag: AtomicFlag,
npm_registry_api: NpmRegistry,
npm_resolution: NpmResolution,
package_deps: PackageJsonDeps,
@@ -33,7 +33,7 @@ impl PackageJsonDepsInstaller {
) -> Self {
Self(deps.map(|package_deps| {
Arc::new(PackageJsonDepsInstallerInner {
- has_installed: AtomicBool::new(false),
+ has_installed_flag: Default::default(),
npm_registry_api,
npm_resolution,
package_deps,
@@ -45,30 +45,15 @@ impl PackageJsonDepsInstaller {
self.0.as_ref().map(|inner| &inner.package_deps)
}
- /// Gets if the package.json has the specified package name.
- pub fn has_package_name(&self, name: &str) -> bool {
- if let Some(package_deps) = self.package_deps() {
- // ensure this looks at the package name and not the
- // bare specifiers (do not look at the keys!)
- package_deps
- .values()
- .filter_map(|r| r.as_ref().ok())
- .any(|v| v.name == name)
- } else {
- false
- }
- }
-
/// Installs the top level dependencies in the package.json file
/// without going through and resolving the descendant dependencies yet.
pub async fn ensure_top_level_install(&self) -> Result<(), AnyError> {
- use std::sync::atomic::Ordering;
let inner = match &self.0 {
Some(inner) => inner,
None => return Ok(()),
};
- if inner.has_installed.swap(true, Ordering::SeqCst) {
+ if !inner.has_installed_flag.raise() {
return Ok(()); // already installed by something else
}
diff --git a/cli/npm/registry.rs b/cli/npm/registry.rs
index b9bff09e6..a87365c9c 100644
--- a/cli/npm/registry.rs
+++ b/cli/npm/registry.rs
@@ -5,8 +5,6 @@ use std::collections::HashSet;
use std::fs;
use std::io::ErrorKind;
use std::path::PathBuf;
-use std::sync::atomic::AtomicBool;
-use std::sync::atomic::Ordering;
use std::sync::Arc;
use async_trait::async_trait;
@@ -31,6 +29,7 @@ use crate::cache::CACHE_PERM;
use crate::http_util::HttpClient;
use crate::util::fs::atomic_write_file;
use crate::util::progress_bar::ProgressBar;
+use crate::util::sync::AtomicFlag;
use super::cache::should_sync_download;
use super::cache::NpmCache;
@@ -70,7 +69,7 @@ impl NpmRegistry {
Self(Some(Arc::new(NpmRegistryApiInner {
base_url,
cache,
- force_reload: AtomicBool::new(false),
+ force_reload_flag: Default::default(),
mem_cache: Default::default(),
previously_reloaded_packages: Default::default(),
http_client,
@@ -109,7 +108,7 @@ impl NpmRegistry {
if matches!(self.inner().cache.cache_setting(), CacheSetting::Only) {
return false;
}
- !self.inner().force_reload.swap(true, Ordering::SeqCst)
+ self.inner().force_reload_flag.raise()
}
fn inner(&self) -> &Arc<NpmRegistryApiInner> {
@@ -160,7 +159,7 @@ enum CacheItem {
struct NpmRegistryApiInner {
base_url: Url,
cache: NpmCache,
- force_reload: AtomicBool,
+ force_reload_flag: AtomicFlag,
mem_cache: Mutex<HashMap<String, CacheItem>>,
previously_reloaded_packages: Mutex<HashSet<String>>,
http_client: HttpClient,
@@ -230,7 +229,7 @@ impl NpmRegistryApiInner {
}
fn force_reload(&self) -> bool {
- self.force_reload.load(Ordering::SeqCst)
+ self.force_reload_flag.is_raised()
}
fn load_file_cached_package_info(
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index 188f57289..d3a0618ef 100644
--- a/cli/proc_state.rs
+++ b/cli/proc_state.rs
@@ -393,6 +393,7 @@ impl ProcState {
build_graph_with_npm_resolution(
graph,
+ &self.resolver,
&self.npm_resolver,
roots.clone(),
&mut cache,
@@ -695,6 +696,7 @@ impl ProcState {
let mut graph = ModuleGraph::default();
build_graph_with_npm_resolution(
&mut graph,
+ &self.resolver,
&self.npm_resolver,
roots,
loader,
diff --git a/cli/resolver.rs b/cli/resolver.rs
index 46badfe8a..56bae25c1 100644
--- a/cli/resolver.rs
+++ b/cli/resolver.rs
@@ -24,6 +24,7 @@ use crate::args::JsxImportSourceConfig;
use crate::npm::NpmRegistry;
use crate::npm::NpmResolution;
use crate::npm::PackageJsonDepsInstaller;
+use crate::util::sync::AtomicFlag;
/// A resolver that takes care of resolution, taking into account loaded
/// import map, JSX settings.
@@ -36,6 +37,7 @@ pub struct CliGraphResolver {
npm_registry_api: NpmRegistry,
npm_resolution: NpmResolution,
package_json_deps_installer: PackageJsonDepsInstaller,
+ found_package_json_dep_flag: Arc<AtomicFlag>,
sync_download_queue: Option<Arc<TaskQueue>>,
}
@@ -54,6 +56,7 @@ impl Default for CliGraphResolver {
npm_registry_api,
npm_resolution,
package_json_deps_installer: Default::default(),
+ found_package_json_dep_flag: Default::default(),
sync_download_queue: Self::create_sync_download_queue(),
}
}
@@ -79,6 +82,7 @@ impl CliGraphResolver {
npm_registry_api,
npm_resolution,
package_json_deps_installer,
+ found_package_json_dep_flag: Default::default(),
sync_download_queue: Self::create_sync_download_queue(),
}
}
@@ -98,6 +102,18 @@ impl CliGraphResolver {
pub fn as_graph_npm_resolver(&self) -> &dyn NpmResolver {
self
}
+
+ pub async fn top_level_package_json_install_if_necessary(
+ &self,
+ ) -> Result<(), AnyError> {
+ if self.found_package_json_dep_flag.is_raised() {
+ self
+ .package_json_deps_installer
+ .ensure_top_level_install()
+ .await?;
+ }
+ Ok(())
+ }
}
impl Resolver for CliGraphResolver {
@@ -132,6 +148,7 @@ impl Resolver for CliGraphResolver {
if let Some(deps) = self.package_json_deps_installer.package_deps().as_ref()
{
if let Some(specifier) = resolve_package_json_dep(specifier, deps)? {
+ self.found_package_json_dep_flag.raise();
return Ok(specifier);
}
}
@@ -195,7 +212,6 @@ impl NpmResolver for CliGraphResolver {
// this will internally cache the package information
let package_name = package_name.to_string();
let api = self.npm_registry_api.clone();
- let deps_installer = self.package_json_deps_installer.clone();
let maybe_sync_download_queue = self.sync_download_queue.clone();
async move {
let permit = if let Some(task_queue) = &maybe_sync_download_queue {
@@ -204,21 +220,6 @@ impl NpmResolver for CliGraphResolver {
None
};
- // trigger an npm install if the package name matches
- // a package in the package.json
- //
- // todo(dsherret): ideally this would only download if a bare
- // specifiy matched in the package.json, but deno_graph only
- // calls this once per package name and we might resolve an
- // npm specifier first which calls this, then a bare specifier
- // second and that would cause this not to occur.
- if deps_installer.has_package_name(&package_name) {
- deps_installer
- .ensure_top_level_install()
- .await
- .map_err(|err| format!("{err:#}"))?;
- }
-
let result = api
.package_info(&package_name)
.await
diff --git a/cli/tests/integration/cache_tests.rs b/cli/tests/integration/cache_tests.rs
index c80f7f5c8..32508a95a 100644
--- a/cli/tests/integration/cache_tests.rs
+++ b/cli/tests/integration/cache_tests.rs
@@ -1,6 +1,7 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
use test_util::env_vars_for_npm_tests;
+use test_util::TestContextBuilder;
itest!(_036_import_map_fetch {
args:
@@ -107,3 +108,22 @@ itest!(package_json_basic {
copy_temp_dir: Some("package_json/basic"),
exit_code: 0,
});
+
+#[test]
+fn cache_matching_package_json_dep_should_not_install_all() {
+ let context = TestContextBuilder::for_npm().use_temp_cwd().build();
+ let temp_dir = context.temp_dir();
+ temp_dir.write(
+ "package.json",
+ r#"{ "dependencies": { "@types/node": "18.8.2", "@denotest/esm-basic": "*" } }"#,
+ );
+ let output = context
+ .new_command()
+ .args("cache npm:@types/node@18.8.2")
+ .run();
+ output.assert_matches_text(concat!(
+ "Download http://localhost:4545/npm/registry/@types/node\n",
+ "Download http://localhost:4545/npm/registry/@types/node/node-18.8.2.tgz\n",
+ "Initialize @types/node@18.8.2\n",
+ ));
+}
diff --git a/cli/util/mod.rs b/cli/util/mod.rs
index c3133cc10..61511679f 100644
--- a/cli/util/mod.rs
+++ b/cli/util/mod.rs
@@ -11,6 +11,7 @@ pub mod fs;
pub mod logger;
pub mod path;
pub mod progress_bar;
+pub mod sync;
pub mod text_encoding;
pub mod time;
pub mod unix;
diff --git a/cli/util/sync.rs b/cli/util/sync.rs
new file mode 100644
index 000000000..6d136abc1
--- /dev/null
+++ b/cli/util/sync.rs
@@ -0,0 +1,35 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+use std::sync::atomic::AtomicBool;
+use std::sync::atomic::Ordering;
+
+/// Simplifies the use of an atomic boolean as a flag.
+#[derive(Debug, Default)]
+pub struct AtomicFlag(AtomicBool);
+
+impl AtomicFlag {
+ /// Raises the flag returning if the raise was successful.
+ pub fn raise(&self) -> bool {
+ !self.0.swap(true, Ordering::SeqCst)
+ }
+
+ /// Gets if the flag is raised.
+ pub fn is_raised(&self) -> bool {
+ self.0.load(Ordering::SeqCst)
+ }
+}
+
+#[cfg(test)]
+mod test {
+ use super::AtomicFlag;
+
+ #[test]
+ fn atomic_flag_raises() {
+ let flag = AtomicFlag::default();
+ assert!(!flag.is_raised()); // false by default
+ assert!(flag.raise());
+ assert!(flag.is_raised());
+ assert!(!flag.raise());
+ assert!(flag.is_raised());
+ }
+}
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-05 02:39:18 +0000