From b82cd0d0e39fdbd37fc258b998d54693d3e02674 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 8 Sep 2022 15:31:24 -0400 Subject: [PATCH 01/15] Initial work on a local node modules folder. --- cli/npm/local.rs | 277 ++++++++++++++++++++++++++++++++++++++++++ cli/npm/mod.rs | 1 + cli/npm/resolution.rs | 21 ++++ 3 files changed, 299 insertions(+) create mode 100644 cli/npm/local.rs diff --git a/cli/npm/local.rs b/cli/npm/local.rs new file mode 100644 index 00000000000000..941b63564202f6 --- /dev/null +++ b/cli/npm/local.rs @@ -0,0 +1,277 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + +use std::cell::RefCell; +use std::collections::HashMap; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::rc::Rc; + +use deno_core::error::AnyError; +use deno_core::futures::FutureExt; +use deno_core::url::Url; + +use super::cache::NpmCache; +use super::resolution::NpmResolutionSnapshot; +use super::NpmPackageId; +use super::NpmResolutionPackage; + +#[derive(Default)] +struct NodeModulesFolder { + parent: Option>>, + folders: HashMap>>, + packages: HashMap, +} + +struct NodeModulesPackage { + id: NpmPackageId, + parent: Rc>, + child_folder: Rc>, +} + +pub fn setup_node_modules( + snapshot: &NpmResolutionSnapshot, + cache: &NpmCache, + registry_url: &Url, + output_dir: PathBuf, +) -> Result<(), AnyError> { + // resolve everything to folder structure + let mut top_level_packages = snapshot + .top_level_packages() + .into_iter() + .map(|id| snapshot.package_from_id(&id).unwrap()) + .collect::>(); + top_level_packages.sort_by(|a, b| a.id.cmp(&b.id)); + + let folder = create_virtual_node_modules_folder(snapshot); + + // todo: + // - Need to ensure only one process enters this at a time. + sync_folder_with_fs(&folder.borrow(), cache, registry_url, output_dir)?; + Ok(()) +} + +fn create_virtual_node_modules_folder( + snapshot: &NpmResolutionSnapshot, +) -> Rc> { + // resolve everything to folder structure + let mut top_level_packages = snapshot + .top_level_packages() + .into_iter() + .map(|id| snapshot.package_from_id(&id).unwrap()) + .collect::>(); + top_level_packages.sort_by(|a, b| a.id.cmp(&b.id)); + + let folder = Rc::new(RefCell::new(NodeModulesFolder::default())); + + // go over all the top level packages to ensure they're + // kept in the top level folder + for package in &top_level_packages { + let folder_package = NodeModulesPackage { + parent: folder.clone(), + child_folder: Default::default(), + id: package.id.clone(), + }; + let mut folder = folder.borrow_mut(); + let folder_name = if folder.folders.contains_key(&folder_package.id.name) { + // This is when you say have two packages like so: + // import chalkv4 from "npm:chalk@4" + // import chalkv5 from "npm:chalk@5" + // In this scenario, we use the full resolved package id + // for the second package. + folder_package.id.to_string() + } else { + folder_package.id.name.to_string() + }; + folder + .packages + .insert(folder_package.id.clone(), folder_name.clone()); + let past_item = folder + .folders + .insert(folder_name, Rc::new(RefCell::new(folder_package))); + assert!(past_item.is_none()); + } + + // now go over each package and sub packages and populate them in the folder + for top_level_package in &top_level_packages { + let node_modules_package = { + let folder = RefCell::borrow(&folder); + let folder_name = folder.packages.get(&top_level_package.id).unwrap(); + folder.folders.get(folder_name).unwrap().clone() + }; + populate_folder_deps(top_level_package, &node_modules_package, snapshot); + } + folder +} + +fn populate_folder_deps( + package: &NpmResolutionPackage, + node_modules_package: &RefCell, + snapshot: &NpmResolutionSnapshot, +) { + // package_ref_name is what the package refers to the other package as + for (package_ref_name, id) in &package.dependencies { + if let Some(insert_folder) = get_insert_folder( + package_ref_name, + id, + node_modules_package.borrow().child_folder.clone(), + ) { + let node_modules_package = Rc::new(RefCell::new(NodeModulesPackage { + id: id.clone(), + parent: insert_folder.clone(), + child_folder: Default::default(), + })); + let mut insert_folder = insert_folder.borrow_mut(); + insert_folder + .packages + .insert(id.clone(), package_ref_name.clone()); + let past_item = insert_folder + .folders + .insert(package_ref_name.clone(), node_modules_package.clone()); + assert!(past_item.is_none()); + + // now go through all this module's dependencies + let package = snapshot.package_from_id(id).unwrap(); + populate_folder_deps(&package, &node_modules_package, snapshot); + } + } +} + +fn get_insert_folder( + package_ref_name: &String, + id: &NpmPackageId, + child_folder: Rc>, +) -> Option>> { + let mut current_folder = child_folder.clone(); + loop { + let parent = { + let folder = current_folder.borrow(); + if folder.folders.contains_key(package_ref_name) { + if folder.packages.get(id) == Some(package_ref_name) { + // same name and id exists in the tree, so ignore + return None; + } else { + // same name, but different id exists in the tree, so + // use the child folder + return Some(child_folder); + } + } + folder.parent.clone() + }; + match parent { + Some(parent) => { + // go up to the parent folder + current_folder = parent.borrow().parent.clone(); + } + None => { + // no name found, so insert in the root folder, which is the current folder + return Some(current_folder); + } + } + } +} + +fn sync_folder_with_fs( + folder: &NodeModulesFolder, + cache: &NpmCache, + registry_url: &Url, + output_dir: PathBuf, +) -> Result<(), AnyError> { + // create the folders + fs::create_dir_all(&output_dir)?; + for (folder_name, package) in &folder.folders { + let local_folder = output_dir.join(folder_name); + remove_dir_all(&local_folder)?; + let package = package.borrow(); + let child_folder = package.child_folder.borrow(); + let cache_folder = cache.package_folder(&package.id, registry_url); + if child_folder.folders.is_empty() { + // no sub packages, so create a symlink + symlink_dir(&cache_folder, &local_folder)?; + } else { + // there's sub packages, so symlink the children + symlink_dir_children(&cache_folder, &local_folder)?; + let sub_folder = local_folder.join("node_modules"); + sync_folder_with_fs(&child_folder, cache, registry_url, sub_folder)?; + } + } + Ok(()) +} + +fn remove_dir_all(path: &Path) -> Result<(), AnyError> { + match fs::remove_dir_all(path) { + Ok(_) => Ok(()), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(err) => Err(err.into()), + } +} + +fn symlink_dir_children( + oldpath: &Path, + newpath: &Path, +) -> Result<(), AnyError> { + debug_assert!(oldpath.is_dir()); + fs::create_dir(&newpath)?; + for entry in fs::read_dir(oldpath)? { + let entry = entry?; + if entry.file_type()?.is_dir() { + symlink_dir(&entry.path(), &newpath.join(entry.file_name()))?; + } else { + symlink_file(&entry.path(), &newpath.join(entry.file_name()))?; + } + } + Ok(()) +} + +// todo(dsherret): try to consolidate these symlink_dir and symlink_file functions +fn symlink_dir(oldpath: &Path, newpath: &Path) -> Result<(), AnyError> { + use std::io::Error; + let err_mapper = |err: Error| { + Error::new( + err.kind(), + format!( + "{}, symlink '{}' -> '{}'", + err, + oldpath.display(), + newpath.display() + ), + ) + }; + #[cfg(unix)] + { + use std::os::unix::fs::symlink; + symlink(&oldpath, &newpath).map_err(err_mapper)?; + } + #[cfg(not(unix))] + { + use std::os::windows::fs::symlink_dir; + symlink_dir(&oldpath, &newpath).map_err(err_mapper)?; + } + Ok(()) +} + +fn symlink_file(oldpath: &Path, newpath: &Path) -> Result<(), AnyError> { + use std::io::Error; + let err_mapper = |err: Error| { + Error::new( + err.kind(), + format!( + "{}, symlink '{}' -> '{}'", + err, + oldpath.display(), + newpath.display() + ), + ) + }; + #[cfg(unix)] + { + use std::os::unix::fs::symlink; + symlink(&oldpath, &newpath).map_err(err_mapper)?; + } + #[cfg(not(unix))] + { + use std::os::windows::fs::symlink_file; + symlink_file(&oldpath, &newpath).map_err(err_mapper)?; + } + Ok(()) +} diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index d0a57c5bc07a93..76636be6a7eee8 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -1,6 +1,7 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. mod cache; +mod local; mod registry; mod resolution; mod semver; diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs index 8fac92716efbc3..87fa9922f55b5c 100644 --- a/cli/npm/resolution.rs +++ b/cli/npm/resolution.rs @@ -2,6 +2,7 @@ use std::cmp::Ordering; use std::collections::HashMap; +use std::collections::HashSet; use std::collections::VecDeque; use deno_ast::ModuleSpecifier; @@ -185,6 +186,26 @@ impl NpmResolutionSnapshot { } } + pub fn top_level_packages(&self) -> Vec { + self + .package_reqs + .iter() + .map(|(req, version)| NpmPackageId { + name: req.name.clone(), + version: version.clone(), + }) + .collect::>() + .into_iter() + .collect::>() + } + + pub fn package_from_id( + &self, + id: &NpmPackageId, + ) -> Option<&NpmResolutionPackage> { + self.packages.get(id) + } + pub fn resolve_package_from_package( &self, name: &str, From 22d4a901296f9f6de78bf151beb3c8cdc25b0a63 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 19 Sep 2022 17:31:37 -0400 Subject: [PATCH 02/15] Add cli flags. Need to just implement resolving for the folder. --- cli/args/flags.rs | 32 ++++ cli/args/mod.rs | 17 +++ cli/npm/mod.rs | 1 - cli/npm/resolution.rs | 2 - cli/npm/resolvers/common.rs | 31 ++++ cli/npm/resolvers/global.rs | 33 +---- cli/npm/{ => resolvers}/local.rs | 243 +++++++++++++++++++++++++------ cli/npm/resolvers/mod.rs | 12 +- cli/proc_state.rs | 1 + cli/tools/standalone.rs | 1 + 10 files changed, 294 insertions(+), 79 deletions(-) rename cli/npm/{ => resolvers}/local.rs (52%) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 6924907cbf2d7f..1910585ed34514 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -301,6 +301,7 @@ pub struct Flags { pub cached_only: bool, pub type_check_mode: TypeCheckMode, pub config_flag: ConfigFlag, + pub local_npm: bool, pub coverage_dir: Option, pub enable_testing_features: bool, pub ignore: Vec, @@ -1734,6 +1735,7 @@ fn compile_args(app: Command) -> Command { .arg(import_map_arg()) .arg(no_remote_arg()) .arg(no_npm_arg()) + .arg(local_npm_arg()) .arg(no_config_arg()) .arg(config_arg()) .arg(no_check_arg()) @@ -1749,6 +1751,7 @@ fn compile_args_without_check_args(app: Command) -> Command { .arg(import_map_arg()) .arg(no_remote_arg()) .arg(no_npm_arg()) + .arg(local_npm_arg()) .arg(config_arg()) .arg(no_config_arg()) .arg(reload_arg()) @@ -2149,6 +2152,12 @@ fn no_npm_arg<'a>() -> Arg<'a> { .help("Do not resolve npm modules") } +fn local_npm_arg<'a>() -> Arg<'a> { + Arg::new("local-npm") + .long("local-npm") + .help("Creates a local node_modules folder") +} + fn unsafely_ignore_certificate_errors_arg<'a>() -> Arg<'a> { Arg::new("unsafely-ignore-certificate-errors") .long("unsafely-ignore-certificate-errors") @@ -2795,6 +2804,7 @@ fn compile_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) { import_map_arg_parse(flags, matches); no_remote_arg_parse(flags, matches); no_npm_arg_parse(flags, matches); + local_npm_args_parse(flags, matches); config_args_parse(flags, matches); no_check_arg_parse(flags, matches); check_arg_parse(flags, matches); @@ -2810,6 +2820,7 @@ fn compile_args_without_no_check_parse( import_map_arg_parse(flags, matches); no_remote_arg_parse(flags, matches); no_npm_arg_parse(flags, matches); + local_npm_args_parse(flags, matches); config_args_parse(flags, matches); reload_arg_parse(flags, matches); lock_args_parse(flags, matches); @@ -3057,6 +3068,12 @@ fn no_npm_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } } +fn local_npm_args_parse(flags: &mut Flags, matches: &ArgMatches) { + if matches.is_present("local-npm") { + flags.local_npm = true; + } +} + fn inspect_arg_validate(val: &str) -> Result<(), String> { match val.parse::() { Ok(_) => Ok(()), @@ -5030,6 +5047,21 @@ mod tests { ); } + #[test] + fn local_npm() { + let r = flags_from_vec(svec!["deno", "run", "--local-npm", "script.ts"]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Run(RunFlags { + script: "script.ts".to_string(), + }), + local_npm: true, + ..Flags::default() + } + ); + } + #[test] fn cached_only() { let r = flags_from_vec(svec!["deno", "run", "--cached-only", "script.ts"]); diff --git a/cli/args/mod.rs b/cli/args/mod.rs index e4c0d855604715..a28f7e45c54be2 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -146,6 +146,23 @@ impl CliOptions { self.overrides.import_map_specifier = Some(path); } + /// Resolves the folder to use for a local node_modules folder. + pub fn resolve_local_node_modules_folder( + &self, + ) -> Result, AnyError> { + if !self.flags.local_npm { + return Ok(None); + } else if let Some(config_path) = self + .maybe_config_file + .as_ref() + .and_then(|c| c.specifier.to_file_path().ok()) + { + Ok(Some(config_path.parent().unwrap().join("node_modules"))) + } else { + Ok(Some(std::env::current_dir()?.join("node_modules"))) + } + } + pub fn resolve_root_cert_store(&self) -> Result { get_root_cert_store( None, diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index a7607037104bb2..d60c06f144c9ec 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -1,7 +1,6 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. mod cache; -mod local; mod registry; mod resolution; mod resolvers; diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs index 196206de4e8146..87fa9922f55b5c 100644 --- a/cli/npm/resolution.rs +++ b/cli/npm/resolution.rs @@ -492,8 +492,6 @@ impl NpmResolution { !self.snapshot.read().packages.is_empty() } - // todo(dsherret): for use in the lsp - #[allow(dead_code)] pub fn snapshot(&self) -> NpmResolutionSnapshot { self.snapshot.read().clone() } diff --git a/cli/npm/resolvers/common.rs b/cli/npm/resolvers/common.rs index f0231859a0a17e..83316fc4f323ef 100644 --- a/cli/npm/resolvers/common.rs +++ b/cli/npm/resolvers/common.rs @@ -1,3 +1,4 @@ +use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; @@ -87,3 +88,33 @@ pub async fn cache_packages( } Ok(()) } + +pub fn ensure_registry_read_permission( + registry_path: &Path, + path: &Path, +) -> Result<(), AnyError> { + // allow reading if it's in the node_modules + if path.starts_with(®istry_path) + && path + .components() + .all(|c| !matches!(c, std::path::Component::ParentDir)) + { + // todo(dsherret): cache this? + if let Ok(registry_path) = std::fs::canonicalize(registry_path) { + match std::fs::canonicalize(path) { + Ok(path) if path.starts_with(registry_path) => { + return Ok(()); + } + Err(e) if e.kind() == ErrorKind::NotFound => { + return Ok(()); + } + _ => {} // ignore + } + } + } + + Err(deno_core::error::custom_error( + "PermissionDenied", + format!("Reading {} is not allowed", path.display()), + )) +} diff --git a/cli/npm/resolvers/global.rs b/cli/npm/resolvers/global.rs index 259d9b9a04ef18..a89a604d8ff2b4 100644 --- a/cli/npm/resolvers/global.rs +++ b/cli/npm/resolvers/global.rs @@ -17,6 +17,7 @@ use crate::npm::NpmPackageId; use crate::npm::NpmPackageReq; use crate::npm::NpmRegistryApi; +use super::common::ensure_registry_read_permission; use super::common::InnerNpmPackageResolver; use super::common::LocalNpmPackageInfo; @@ -103,36 +104,6 @@ impl InnerNpmPackageResolver for GlobalNpmPackageResolver { fn ensure_read_permission(&self, path: &Path) -> Result<(), AnyError> { let registry_path = self.cache.registry_folder(&self.registry_url); - ensure_read_permission(®istry_path, path) + ensure_registry_read_permission(®istry_path, path) } } - -fn ensure_read_permission( - registry_path: &Path, - path: &Path, -) -> Result<(), AnyError> { - // allow reading if it's in the deno_dir node modules - if path.starts_with(®istry_path) - && path - .components() - .all(|c| !matches!(c, std::path::Component::ParentDir)) - { - // todo(dsherret): cache this? - if let Ok(registry_path) = std::fs::canonicalize(registry_path) { - match std::fs::canonicalize(path) { - Ok(path) if path.starts_with(registry_path) => { - return Ok(()); - } - Err(e) if e.kind() == ErrorKind::NotFound => { - return Ok(()); - } - _ => {} // ignore - } - } - } - - Err(deno_core::error::custom_error( - "PermissionDenied", - format!("Reading {} is not allowed", path.display()), - )) -} diff --git a/cli/npm/local.rs b/cli/npm/resolvers/local.rs similarity index 52% rename from cli/npm/local.rs rename to cli/npm/resolvers/local.rs index 941b63564202f6..d1fe070ef26e75 100644 --- a/cli/npm/local.rs +++ b/cli/npm/resolvers/local.rs @@ -6,35 +6,126 @@ use std::fs; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; +use std::sync::Arc; +use deno_ast::ModuleSpecifier; use deno_core::error::AnyError; +use deno_core::futures::future::BoxFuture; use deno_core::futures::FutureExt; +use deno_core::parking_lot::RwLock; +use deno_core::serde_json; use deno_core::url::Url; +use serde::Deserialize; +use serde::Serialize; -use super::cache::NpmCache; -use super::resolution::NpmResolutionSnapshot; -use super::NpmPackageId; -use super::NpmResolutionPackage; +use crate::npm::resolution::NpmResolution; +use crate::npm::resolution::NpmResolutionSnapshot; +use crate::npm::NpmCache; +use crate::npm::NpmPackageId; +use crate::npm::NpmPackageReq; +use crate::npm::NpmRegistryApi; +use crate::npm::NpmResolutionPackage; -#[derive(Default)] -struct NodeModulesFolder { - parent: Option>>, - folders: HashMap>>, - packages: HashMap, +use super::common::cache_packages; +use super::common::ensure_registry_read_permission; +use super::common::InnerNpmPackageResolver; +use super::LocalNpmPackageInfo; + +#[derive(Debug, Clone)] +pub struct LocalNpmPackageResolver { + cache: NpmCache, + resolution: Arc, + folder: Arc>, + registry_url: Url, + node_modules_folder: PathBuf, } -struct NodeModulesPackage { - id: NpmPackageId, - parent: Rc>, - child_folder: Rc>, +impl LocalNpmPackageResolver { + pub fn new( + cache: NpmCache, + api: NpmRegistryApi, + node_modules_folder: PathBuf, + ) -> Self { + let registry_url = api.base_url().to_owned(); + let resolution = Arc::new(NpmResolution::new(api)); + + Self { + cache, + resolution, + folder: Default::default(), + registry_url, + node_modules_folder, + } + } } -pub fn setup_node_modules( +impl InnerNpmPackageResolver for LocalNpmPackageResolver { + fn resolve_package_from_deno_module( + &self, + pkg_req: &NpmPackageReq, + ) -> Result { + todo!() + } + + fn resolve_package_from_package( + &self, + name: &str, + referrer: &ModuleSpecifier, + ) -> Result { + todo!() + } + + fn resolve_package_from_specifier( + &self, + specifier: &ModuleSpecifier, + ) -> Result { + todo!() + } + + fn has_packages(&self) -> bool { + self.resolution.has_packages() + } + + fn add_package_reqs( + &self, + packages: Vec, + ) -> BoxFuture<'static, Result<(), AnyError>> { + let resolver = self.clone(); + async move { + resolver.resolution.add_package_reqs(packages).await?; + cache_packages( + resolver.resolution.all_packages(), + &resolver.cache, + &resolver.registry_url, + ) + .await?; + + let folder = setup_node_modules( + &resolver.resolution.snapshot(), + &resolver.cache, + &resolver.registry_url, + &resolver.node_modules_folder, + )?; + + // todo(dsherret): move instead of clone + *resolver.folder.write() = folder.read().clone(); + Ok(()) + } + .boxed() + } + + fn ensure_read_permission(&self, path: &Path) -> Result<(), AnyError> { + let registry_path = self.cache.registry_folder(&self.registry_url); + ensure_registry_read_permission(®istry_path, path) + } +} + +fn setup_node_modules( snapshot: &NpmResolutionSnapshot, cache: &NpmCache, registry_url: &Url, - output_dir: PathBuf, -) -> Result<(), AnyError> { + output_dir: &PathBuf, +) -> Result>, AnyError> { // resolve everything to folder structure let mut top_level_packages = snapshot .top_level_packages() @@ -45,15 +136,29 @@ pub fn setup_node_modules( let folder = create_virtual_node_modules_folder(snapshot); - // todo: - // - Need to ensure only one process enters this at a time. - sync_folder_with_fs(&folder.borrow(), cache, registry_url, output_dir)?; - Ok(()) + // todo(dsherret): ensure only one process enters this at a time. + sync_folder_with_fs(&folder.read(), cache, registry_url, &output_dir)?; + + Ok(folder) +} + +#[derive(Default, Debug, Clone)] +struct NodeModulesFolder { + parent: Option>>, + folders: HashMap>>, + packages: HashMap, +} + +#[derive(Debug)] +struct NodeModulesPackage { + id: NpmPackageId, + parent: Arc>, + child_folder: Arc>, } fn create_virtual_node_modules_folder( snapshot: &NpmResolutionSnapshot, -) -> Rc> { +) -> Arc> { // resolve everything to folder structure let mut top_level_packages = snapshot .top_level_packages() @@ -62,7 +167,7 @@ fn create_virtual_node_modules_folder( .collect::>(); top_level_packages.sort_by(|a, b| a.id.cmp(&b.id)); - let folder = Rc::new(RefCell::new(NodeModulesFolder::default())); + let folder = Arc::new(RwLock::new(NodeModulesFolder::default())); // go over all the top level packages to ensure they're // kept in the top level folder @@ -72,7 +177,7 @@ fn create_virtual_node_modules_folder( child_folder: Default::default(), id: package.id.clone(), }; - let mut folder = folder.borrow_mut(); + let mut folder = folder.write(); let folder_name = if folder.folders.contains_key(&folder_package.id.name) { // This is when you say have two packages like so: // import chalkv4 from "npm:chalk@4" @@ -88,14 +193,14 @@ fn create_virtual_node_modules_folder( .insert(folder_package.id.clone(), folder_name.clone()); let past_item = folder .folders - .insert(folder_name, Rc::new(RefCell::new(folder_package))); + .insert(folder_name, Arc::new(RwLock::new(folder_package))); assert!(past_item.is_none()); } // now go over each package and sub packages and populate them in the folder for top_level_package in &top_level_packages { let node_modules_package = { - let folder = RefCell::borrow(&folder); + let folder = folder.read(); let folder_name = folder.packages.get(&top_level_package.id).unwrap(); folder.folders.get(folder_name).unwrap().clone() }; @@ -106,7 +211,7 @@ fn create_virtual_node_modules_folder( fn populate_folder_deps( package: &NpmResolutionPackage, - node_modules_package: &RefCell, + node_modules_package: &RwLock, snapshot: &NpmResolutionSnapshot, ) { // package_ref_name is what the package refers to the other package as @@ -114,14 +219,14 @@ fn populate_folder_deps( if let Some(insert_folder) = get_insert_folder( package_ref_name, id, - node_modules_package.borrow().child_folder.clone(), + node_modules_package.read().child_folder.clone(), ) { - let node_modules_package = Rc::new(RefCell::new(NodeModulesPackage { + let node_modules_package = Arc::new(RwLock::new(NodeModulesPackage { id: id.clone(), parent: insert_folder.clone(), child_folder: Default::default(), })); - let mut insert_folder = insert_folder.borrow_mut(); + let mut insert_folder = insert_folder.write(); insert_folder .packages .insert(id.clone(), package_ref_name.clone()); @@ -140,12 +245,12 @@ fn populate_folder_deps( fn get_insert_folder( package_ref_name: &String, id: &NpmPackageId, - child_folder: Rc>, -) -> Option>> { + child_folder: Arc>, +) -> Option>> { let mut current_folder = child_folder.clone(); loop { let parent = { - let folder = current_folder.borrow(); + let folder = current_folder.read(); if folder.folders.contains_key(package_ref_name) { if folder.packages.get(id) == Some(package_ref_name) { // same name and id exists in the tree, so ignore @@ -161,7 +266,7 @@ fn get_insert_folder( match parent { Some(parent) => { // go up to the parent folder - current_folder = parent.borrow().parent.clone(); + current_folder = parent.read().parent.clone(); } None => { // no name found, so insert in the root folder, which is the current folder @@ -171,30 +276,80 @@ fn get_insert_folder( } } +#[derive(Default, Serialize, Deserialize)] +struct FolderData { + packages: HashMap, +} + +#[derive(Serialize, Deserialize, PartialEq, Eq)] +struct FolderDataPackage { + id: String, + kind: FolderKind, +} + +#[derive(Serialize, Deserialize, PartialEq, Eq, Clone, Copy)] +enum FolderKind { + Symlink, + SubDir, +} + fn sync_folder_with_fs( folder: &NodeModulesFolder, cache: &NpmCache, registry_url: &Url, - output_dir: PathBuf, + output_dir: &PathBuf, ) -> Result<(), AnyError> { // create the folders - fs::create_dir_all(&output_dir)?; + fs::create_dir_all(output_dir)?; + let resolution_file = output_dir.join(".deno_resolution"); + let mut folder_data: FolderData = fs::read_to_string(&resolution_file) + .ok() + .and_then(|text| serde_json::from_str(&text).ok()) + .unwrap_or_default(); for (folder_name, package) in &folder.folders { let local_folder = output_dir.join(folder_name); - remove_dir_all(&local_folder)?; - let package = package.borrow(); - let child_folder = package.child_folder.borrow(); + let package = package.read(); + let child_folder = package.child_folder.read(); let cache_folder = cache.package_folder(&package.id, registry_url); - if child_folder.folders.is_empty() { - // no sub packages, so create a symlink - symlink_dir(&cache_folder, &local_folder)?; + let folder_kind = if child_folder.folders.is_empty() { + FolderKind::Symlink } else { - // there's sub packages, so symlink the children - symlink_dir_children(&cache_folder, &local_folder)?; + FolderKind::SubDir + }; + let expected_folder_data_package = FolderDataPackage { + id: package.id.to_string(), + kind: folder_kind, + }; + let state_matches = folder_data.packages.get(&package.id.name) + == Some(&expected_folder_data_package); + + if !state_matches { + match folder_kind { + FolderKind::Symlink => { + remove_dir_all(&local_folder)?; + // no sub packages, so create a symlink + symlink_dir(&cache_folder, &local_folder)?; + } + FolderKind::SubDir => { + // there's sub packages, so symlink the children + symlink_dir_children(&cache_folder, &local_folder)?; + } + } + folder_data + .packages + .insert(package.id.name.to_string(), expected_folder_data_package); + } + + if folder_kind == FolderKind::SubDir { let sub_folder = local_folder.join("node_modules"); - sync_folder_with_fs(&child_folder, cache, registry_url, sub_folder)?; + sync_folder_with_fs(&child_folder, cache, registry_url, &sub_folder)?; } } + + if let Ok(text) = serde_json::to_string(&folder_data) { + let _ignore = fs::write(resolution_file, text); + } + Ok(()) } diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 02e5be983d87ac..517e917fbaedb7 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -2,6 +2,7 @@ mod common; mod global; +mod local; use deno_core::anyhow::bail; use deno_core::error::custom_error; @@ -16,6 +17,7 @@ use deno_ast::ModuleSpecifier; use deno_core::error::AnyError; use self::common::InnerNpmPackageResolver; +use self::local::LocalNpmPackageResolver; use super::NpmCache; use super::NpmPackageReq; use super::NpmRegistryApi; @@ -35,10 +37,18 @@ impl NpmPackageResolver { api: NpmRegistryApi, unstable: bool, no_npm: bool, + local_node_modules: Option, ) -> Self { // For now, always create a GlobalNpmPackageResolver, but in the future // this might be a local node_modules folder - let inner = Arc::new(GlobalNpmPackageResolver::new(cache, api)); + let inner: Arc = match local_node_modules { + Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new( + cache, + api, + node_modules_folder, + )), + None => Arc::new(GlobalNpmPackageResolver::new(cache, api)), + }; Self { unstable, no_npm, diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 2d2c3a76de6d85..285b5878686940 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -242,6 +242,7 @@ impl ProcState { // don't do the unstable error when in the lsp || matches!(cli_options.sub_command(), DenoSubcommand::Lsp), cli_options.no_npm(), + cli_options.resolve_local_node_modules_folder()?, ); let emit_options: deno_ast::EmitOptions = ts_config_result.ts_config.into(); diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index 3a2bed1053ce56..2c1bc9f7a4be8f 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -270,6 +270,7 @@ pub fn compile_to_runtime_flags( import_map_path: flags.import_map_path.clone(), inspect_brk: None, inspect: None, + local_npm: false, location: flags.location.clone(), lock_write: false, lock: None, From 2453479604ae4ed3054c1e59b9b5b0dff10a39c8 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 20 Sep 2022 12:49:39 -0400 Subject: [PATCH 03/15] Commit work for backup purposes. --- cli/node/mod.rs | 30 ++-- cli/npm/resolvers/common.rs | 21 +-- cli/npm/resolvers/global.rs | 28 ++-- cli/npm/resolvers/local.rs | 263 ++++++++++++++++++++++-------------- cli/npm/resolvers/mod.rs | 42 +++--- cli/proc_state.rs | 10 +- ext/node/package_json.rs | 5 + 7 files changed, 224 insertions(+), 175 deletions(-) diff --git a/cli/node/mod.rs b/cli/node/mod.rs index 2ec9295dd8b453..2bf2e90aa80693 100644 --- a/cli/node/mod.rs +++ b/cli/node/mod.rs @@ -433,9 +433,8 @@ pub fn node_resolve_npm_reference( reference: &NpmPackageReference, npm_resolver: &NpmPackageResolver, ) -> Result, AnyError> { - let package_folder = npm_resolver - .resolve_package_from_deno_module(&reference.req)? - .folder_path; + let package_folder = + npm_resolver.resolve_package_folder_from_deno_module(&reference.req)?; let resolved_path = package_config_resolve( &reference .sub_path @@ -462,15 +461,28 @@ pub fn node_resolve_binary_export( bin_name: Option<&str>, npm_resolver: &NpmPackageResolver, ) -> Result { - let pkg = npm_resolver.resolve_package_from_deno_module(pkg_req)?; - let package_folder = pkg.folder_path; + fn get_package_display_name(package_json: &PackageJson) -> String { + package_json + .name + .as_ref() + .and_then(|name| { + package_json + .version + .as_ref() + .map(|version| format!("{}@{}", name, version)) + }) + .unwrap_or_else(|| format!("{}", package_json.path.display())) + } + + let package_folder = + npm_resolver.resolve_package_folder_from_deno_module(pkg_req)?; let package_json_path = package_folder.join("package.json"); let package_json = PackageJson::load(npm_resolver, package_json_path)?; let bin = match &package_json.bin { Some(bin) => bin, None => bail!( "package {} did not have a 'bin' property in its package.json", - pkg.id + get_package_display_name(&package_json), ), }; let bin_entry = match bin { @@ -490,13 +502,13 @@ pub fn node_resolve_binary_export( o.get(&pkg_req.name) } }, - _ => bail!("package {} did not have a 'bin' property with a string or object value in its package.json", pkg.id), + _ => bail!("package {} did not have a 'bin' property with a string or object value in its package.json", get_package_display_name(&package_json)), }; let bin_entry = match bin_entry { Some(e) => e, None => bail!( "package {} did not have a 'bin' entry for {} in its package.json", - pkg.id, + get_package_display_name(&package_json), bin_name.unwrap_or(&pkg_req.name), ), }; @@ -504,7 +516,7 @@ pub fn node_resolve_binary_export( Value::String(s) => s, _ => bail!( "package {} had a non-string sub property of 'bin' in its package.json", - pkg.id + get_package_display_name(&package_json), ), }; diff --git a/cli/npm/resolvers/common.rs b/cli/npm/resolvers/common.rs index 83316fc4f323ef..c114101b758420 100644 --- a/cli/npm/resolvers/common.rs +++ b/cli/npm/resolvers/common.rs @@ -10,34 +10,25 @@ use deno_core::futures::future::BoxFuture; use deno_core::url::Url; use crate::npm::NpmCache; -use crate::npm::NpmPackageId; use crate::npm::NpmPackageReq; use crate::npm::NpmResolutionPackage; -/// Information about the local npm package. -pub struct LocalNpmPackageInfo { - /// Unique identifier. - pub id: NpmPackageId, - /// Local folder path of the npm package. - pub folder_path: PathBuf, -} - pub trait InnerNpmPackageResolver: Send + Sync { - fn resolve_package_from_deno_module( + fn resolve_package_folder_from_deno_module( &self, pkg_req: &NpmPackageReq, - ) -> Result; + ) -> Result; - fn resolve_package_from_package( + fn resolve_package_folder_from_package( &self, name: &str, referrer: &ModuleSpecifier, - ) -> Result; + ) -> Result; - fn resolve_package_from_specifier( + fn resolve_package_folder_from_specifier( &self, specifier: &ModuleSpecifier, - ) -> Result; + ) -> Result; fn has_packages(&self) -> bool; diff --git a/cli/npm/resolvers/global.rs b/cli/npm/resolvers/global.rs index a89a604d8ff2b4..6a5156e90068c7 100644 --- a/cli/npm/resolvers/global.rs +++ b/cli/npm/resolvers/global.rs @@ -1,7 +1,7 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. -use std::io::ErrorKind; use std::path::Path; +use std::path::PathBuf; use std::sync::Arc; use deno_ast::ModuleSpecifier; @@ -19,7 +19,6 @@ use crate::npm::NpmRegistryApi; use super::common::ensure_registry_read_permission; use super::common::InnerNpmPackageResolver; -use super::common::LocalNpmPackageInfo; #[derive(Debug, Clone)] pub struct GlobalNpmPackageResolver { @@ -40,45 +39,42 @@ impl GlobalNpmPackageResolver { } } - fn local_package_info(&self, id: &NpmPackageId) -> LocalNpmPackageInfo { - LocalNpmPackageInfo { - folder_path: self.cache.package_folder(id, &self.registry_url), - id: id.clone(), - } + fn package_folder(&self, id: &NpmPackageId) -> PathBuf { + self.cache.package_folder(id, &self.registry_url) } } impl InnerNpmPackageResolver for GlobalNpmPackageResolver { - fn resolve_package_from_deno_module( + fn resolve_package_folder_from_deno_module( &self, pkg_req: &NpmPackageReq, - ) -> Result { + ) -> Result { let pkg = self.resolution.resolve_package_from_deno_module(pkg_req)?; - Ok(self.local_package_info(&pkg.id)) + Ok(self.package_folder(&pkg.id)) } - fn resolve_package_from_package( + fn resolve_package_folder_from_package( &self, name: &str, referrer: &ModuleSpecifier, - ) -> Result { + ) -> Result { let referrer_pkg_id = self .cache .resolve_package_id_from_specifier(referrer, &self.registry_url)?; let pkg = self .resolution .resolve_package_from_package(name, &referrer_pkg_id)?; - Ok(self.local_package_info(&pkg.id)) + Ok(self.package_folder(&pkg.id)) } - fn resolve_package_from_specifier( + fn resolve_package_folder_from_specifier( &self, specifier: &ModuleSpecifier, - ) -> Result { + ) -> Result { let pkg_id = self .cache .resolve_package_id_from_specifier(specifier, &self.registry_url)?; - Ok(self.local_package_info(&pkg_id)) + Ok(self.package_folder(&pkg_id)) } fn has_packages(&self) -> bool { diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index d1fe070ef26e75..b04fec97e41968 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -2,6 +2,7 @@ use std::cell::RefCell; use std::collections::HashMap; +use std::collections::HashSet; use std::fs; use std::path::Path; use std::path::PathBuf; @@ -9,6 +10,8 @@ use std::rc::Rc; use std::sync::Arc; use deno_ast::ModuleSpecifier; +use deno_core::anyhow::anyhow; +use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::futures::future::BoxFuture; use deno_core::futures::FutureExt; @@ -29,15 +32,15 @@ use crate::npm::NpmResolutionPackage; use super::common::cache_packages; use super::common::ensure_registry_read_permission; use super::common::InnerNpmPackageResolver; -use super::LocalNpmPackageInfo; #[derive(Debug, Clone)] pub struct LocalNpmPackageResolver { cache: NpmCache, resolution: Arc, - folder: Arc>, + folder_index: Arc>, registry_url: Url, - node_modules_folder: PathBuf, + root_node_modules_path: PathBuf, + root_node_modules_specifier: ModuleSpecifier, } impl LocalNpmPackageResolver { @@ -52,34 +55,81 @@ impl LocalNpmPackageResolver { Self { cache, resolution, - folder: Default::default(), + folder_index: Default::default(), registry_url, - node_modules_folder, + root_node_modules_specifier: ModuleSpecifier::from_directory_path(&node_modules_folder).unwrap(), + root_node_modules_path: node_modules_folder, } } + + fn resolve_package_root(&self, path: &Path) -> PathBuf { + let mut last_found = path; + loop { + let parent = path.parent().unwrap(); + if parent.file_name().unwrap() == "node_modules" { + return last_found.to_path_buf(); + } else { + last_found = parent; + } + } + } + + fn resolve_folder_for_specifier(&self, specifier: &ModuleSpecifier) -> Result { + match self.maybe_resolve_folder_for_specifier(specifier) { + Some(path) => Ok(path), + None => bail!("could not find npm package for '{}'", specifier), + } + } + + fn maybe_resolve_folder_for_specifier(&self, specifier: &ModuleSpecifier) -> Option { + let relative_url = self.root_node_modules_specifier.make_relative(specifier)?; + if relative_url.starts_with("../") { + return None; + } + // it's within the directory, so use it + specifier.to_file_path().ok() + } } impl InnerNpmPackageResolver for LocalNpmPackageResolver { - fn resolve_package_from_deno_module( + fn resolve_package_folder_from_deno_module( &self, pkg_req: &NpmPackageReq, - ) -> Result { - todo!() + ) -> Result { + let resolved_package = + self.resolution.resolve_package_from_deno_module(pkg_req)?; + let folder_index = self.folder_index.read(); + let root_folder = folder_index.all_folders.get(&self.root_node_modules_path).unwrap(); + let sub_dir_name = if root_folder + .folder_names + .contains(&resolved_package.id.to_string()) + { + // it's the fully resolved package name + resolved_package.id.to_string() + } else { + resolved_package.id.name.clone() + }; + Ok(self.root_node_modules_path.join(sub_dir_name)) } - fn resolve_package_from_package( + fn resolve_package_folder_from_package( &self, name: &str, referrer: &ModuleSpecifier, - ) -> Result { + ) -> Result { + let local_path = self.resolve_folder_for_specifier(referrer)?; + let package_root_path = self.resolve_package_root(&local_path); + self.resolution.resolve_package_from_package(name, referrer) todo!() } - fn resolve_package_from_specifier( + fn resolve_package_folder_from_specifier( &self, specifier: &ModuleSpecifier, - ) -> Result { - todo!() + ) -> Result { + let local_path = self.resolve_folder_for_specifier(specifier)?; + let package_root_path = self.resolve_package_root(&local_path); + Ok(package_root_path) } fn has_packages(&self) -> bool { @@ -100,15 +150,14 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { ) .await?; - let folder = setup_node_modules( + let folder_index = setup_node_modules( &resolver.resolution.snapshot(), &resolver.cache, &resolver.registry_url, - &resolver.node_modules_folder, + &resolver.root_node_modules_path, )?; - // todo(dsherret): move instead of clone - *resolver.folder.write() = folder.read().clone(); + *resolver.folder_index.write() = folder_index; Ok(()) } .boxed() @@ -124,8 +173,8 @@ fn setup_node_modules( snapshot: &NpmResolutionSnapshot, cache: &NpmCache, registry_url: &Url, - output_dir: &PathBuf, -) -> Result>, AnyError> { + root_node_modules_dir_path: &PathBuf, +) -> Result { // resolve everything to folder structure let mut top_level_packages = snapshot .top_level_packages() @@ -134,31 +183,45 @@ fn setup_node_modules( .collect::>(); top_level_packages.sort_by(|a, b| a.id.cmp(&b.id)); - let folder = create_virtual_node_modules_folder(snapshot); + let folders_index = create_virtual_node_modules_folder(snapshot, root_node_modules_dir_path); // todo(dsherret): ensure only one process enters this at a time. - sync_folder_with_fs(&folder.read(), cache, registry_url, &output_dir)?; + sync_folder_with_fs(&folders_index, &root_node_modules_dir_path, cache, registry_url)?; + + Ok(folders_index) +} - Ok(folder) +#[derive(Default, Debug)] +struct NodeModulesFolderIndex { + // all folders based on their path + all_folders: HashMap, } #[derive(Default, Debug, Clone)] struct NodeModulesFolder { - parent: Option>>, - folders: HashMap>>, - packages: HashMap, + path: PathBuf, + folder_names: HashSet, + packages_to_folder_names: HashMap, } -#[derive(Debug)] -struct NodeModulesPackage { - id: NpmPackageId, - parent: Arc>, - child_folder: Arc>, +impl NodeModulesFolder { + pub fn new(path: PathBuf) -> Self { + Self { + path, + ..Default::default() + } + } + + pub fn add_folder(&mut self, folder_name: String, package_id: NpmPackageId) { + self.folder_names.insert(folder_name.clone()); + self.packages_to_folder_names.insert(package_id, folder_name); + } } fn create_virtual_node_modules_folder( snapshot: &NpmResolutionSnapshot, -) -> Arc> { + root_node_modules_path: &Path, +) -> NodeModulesFolderIndex { // resolve everything to folder structure let mut top_level_packages = snapshot .top_level_packages() @@ -167,51 +230,45 @@ fn create_virtual_node_modules_folder( .collect::>(); top_level_packages.sort_by(|a, b| a.id.cmp(&b.id)); - let folder = Arc::new(RwLock::new(NodeModulesFolder::default())); + let mut folders_index = NodeModulesFolderIndex { + all_folders: Default::default(), + }; + let mut root_folder = NodeModulesFolder::new(root_node_modules_path.to_path_buf()); // go over all the top level packages to ensure they're // kept in the top level folder for package in &top_level_packages { - let folder_package = NodeModulesPackage { - parent: folder.clone(), - child_folder: Default::default(), - id: package.id.clone(), - }; - let mut folder = folder.write(); - let folder_name = if folder.folders.contains_key(&folder_package.id.name) { + let folder_name = if root_folder.folder_names.contains(&package.id.name) { // This is when you say have two packages like so: // import chalkv4 from "npm:chalk@4" // import chalkv5 from "npm:chalk@5" // In this scenario, we use the full resolved package id // for the second package. - folder_package.id.to_string() + package.id.to_string() } else { - folder_package.id.name.to_string() + package.id.name.to_string() }; - folder - .packages - .insert(folder_package.id.clone(), folder_name.clone()); - let past_item = folder - .folders - .insert(folder_name, Arc::new(RwLock::new(folder_package))); - assert!(past_item.is_none()); + root_folder.add_folder(folder_name.clone(), package.id.clone()); } + folders_index.all_folders.insert(root_node_modules_path.to_path_buf(), root_folder); // now go over each package and sub packages and populate them in the folder for top_level_package in &top_level_packages { - let node_modules_package = { - let folder = folder.read(); - let folder_name = folder.packages.get(&top_level_package.id).unwrap(); - folder.folders.get(folder_name).unwrap().clone() + let sub_node_modules_path = { + let root_folder = folders_index.all_folders.get(root_node_modules_path).unwrap(); + let sub_dir = root_folder.packages_to_folder_names.get(&top_level_package.id).unwrap(); + root_folder.path.join(sub_dir).join("node_modules") }; - populate_folder_deps(top_level_package, &node_modules_package, snapshot); + populate_folder_deps(top_level_package, &sub_node_modules_path, root_node_modules_path, &mut folders_index, snapshot); } - folder + folders_index } fn populate_folder_deps( package: &NpmResolutionPackage, - node_modules_package: &RwLock, + sub_node_modules_path: &Path, + root_node_modules_path: &Path, + folders_index: &mut NodeModulesFolderIndex, snapshot: &NpmResolutionSnapshot, ) { // package_ref_name is what the package refers to the other package as @@ -219,25 +276,20 @@ fn populate_folder_deps( if let Some(insert_folder) = get_insert_folder( package_ref_name, id, - node_modules_package.read().child_folder.clone(), + sub_node_modules_path, + root_node_modules_path, + folders_index, ) { - let node_modules_package = Arc::new(RwLock::new(NodeModulesPackage { - id: id.clone(), - parent: insert_folder.clone(), - child_folder: Default::default(), - })); - let mut insert_folder = insert_folder.write(); - insert_folder - .packages - .insert(id.clone(), package_ref_name.clone()); - let past_item = insert_folder - .folders - .insert(package_ref_name.clone(), node_modules_package.clone()); - assert!(past_item.is_none()); + let sub_node_modules_path = { + let node_modules_folder = folders_index.all_folders.entry(insert_folder.clone()).or_insert_with(|| NodeModulesFolder::new(insert_folder)); + node_modules_folder.add_folder(package_ref_name.clone(), id.clone()); + node_modules_folder.path.join(package_ref_name).join("node_modules") + }; // now go through all this module's dependencies let package = snapshot.package_from_id(id).unwrap(); - populate_folder_deps(&package, &node_modules_package, snapshot); + + populate_folder_deps(&package, &sub_node_modules_path, root_node_modules_path, folders_index, snapshot); } } } @@ -245,34 +297,32 @@ fn populate_folder_deps( fn get_insert_folder( package_ref_name: &String, id: &NpmPackageId, - child_folder: Arc>, -) -> Option>> { - let mut current_folder = child_folder.clone(); + sub_node_modules_path: &Path, + root_node_modules_path: &Path, + folders_index: &mut NodeModulesFolderIndex, +) -> Option { + let mut current_folder_path = sub_node_modules_path.to_path_buf(); loop { - let parent = { - let folder = current_folder.read(); - if folder.folders.contains_key(package_ref_name) { - if folder.packages.get(id) == Some(package_ref_name) { + if let Some(folder) = folders_index.all_folders.get(¤t_folder_path) { + if folder.folder_names.contains(package_ref_name) { + if folder.packages_to_folder_names.get(id) == Some(package_ref_name) { // same name and id exists in the tree, so ignore return None; } else { // same name, but different id exists in the tree, so // use the child folder - return Some(child_folder); + return Some(sub_node_modules_path.to_path_buf()); } } - folder.parent.clone() - }; - match parent { - Some(parent) => { - // go up to the parent folder - current_folder = parent.read().parent.clone(); - } - None => { - // no name found, so insert in the root folder, which is the current folder - return Some(current_folder); - } } + // go up two folders to the next node_modules + let parent = sub_node_modules_path.parent().unwrap().parent().unwrap(); + if parent == root_node_modules_path { + // no name found, so insert in the root folder + return Some(root_node_modules_path.to_path_buf()); + } + debug_assert_eq!(parent.file_stem().unwrap(), "node_modules"); + current_folder_path = parent.to_path_buf(); } } @@ -294,11 +344,15 @@ enum FolderKind { } fn sync_folder_with_fs( - folder: &NodeModulesFolder, + folders_index: &NodeModulesFolderIndex, + output_dir: &PathBuf, cache: &NpmCache, registry_url: &Url, - output_dir: &PathBuf, ) -> Result<(), AnyError> { + let folder = match folders_index.all_folders.get(output_dir) { + Some(folder) => folder, + None => return Ok(()), // nothing to do + }; // create the folders fs::create_dir_all(output_dir)?; let resolution_file = output_dir.join(".deno_resolution"); @@ -306,43 +360,42 @@ fn sync_folder_with_fs( .ok() .and_then(|text| serde_json::from_str(&text).ok()) .unwrap_or_default(); - for (folder_name, package) in &folder.folders { - let local_folder = output_dir.join(folder_name); - let package = package.read(); - let child_folder = package.child_folder.read(); - let cache_folder = cache.package_folder(&package.id, registry_url); - let folder_kind = if child_folder.folders.is_empty() { + for (package_id, folder_name) in &folder.packages_to_folder_names { + let local_folder_path = output_dir.join(folder_name); + let sub_node_modules_path = local_folder_path.join("node_modules"); + let cache_folder = cache.package_folder(&package_id, registry_url); + let folder_kind = if folders_index.all_folders.get(&sub_node_modules_path).is_none() { FolderKind::Symlink } else { FolderKind::SubDir }; let expected_folder_data_package = FolderDataPackage { - id: package.id.to_string(), + id: package_id.to_string(), kind: folder_kind, }; - let state_matches = folder_data.packages.get(&package.id.name) + let state_matches = folder_data.packages.get(&package_id.name) == Some(&expected_folder_data_package); if !state_matches { match folder_kind { FolderKind::Symlink => { - remove_dir_all(&local_folder)?; + remove_dir_all(&local_folder_path)?; // no sub packages, so create a symlink - symlink_dir(&cache_folder, &local_folder)?; + symlink_dir(&cache_folder, &local_folder_path)?; } FolderKind::SubDir => { // there's sub packages, so symlink the children - symlink_dir_children(&cache_folder, &local_folder)?; + symlink_dir_children(&cache_folder, &local_folder_path)?; } } folder_data .packages - .insert(package.id.name.to_string(), expected_folder_data_package); + .insert(package_id.name.to_string(), expected_folder_data_package); } if folder_kind == FolderKind::SubDir { - let sub_folder = local_folder.join("node_modules"); - sync_folder_with_fs(&child_folder, cache, registry_url, &sub_folder)?; + let sub_folder = local_folder_path.join("node_modules"); + sync_folder_with_fs(folders_index, &sub_folder, cache, registry_url)?; } } diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 517e917fbaedb7..ecc8923429ebd8 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -22,8 +22,6 @@ use super::NpmCache; use super::NpmPackageReq; use super::NpmRegistryApi; -pub use self::common::LocalNpmPackageInfo; - #[derive(Clone)] pub struct NpmPackageResolver { unstable: bool, @@ -56,36 +54,40 @@ impl NpmPackageResolver { } } - /// Resolves an npm package from a Deno module. - pub fn resolve_package_from_deno_module( + /// Resolves an npm package folder path from a Deno module. + pub fn resolve_package_folder_from_deno_module( &self, pkg_req: &NpmPackageReq, - ) -> Result { - self.inner.resolve_package_from_deno_module(pkg_req) + ) -> Result { + self.inner.resolve_package_folder_from_deno_module(pkg_req) } - /// Resolves an npm package from an npm package referrer. - pub fn resolve_package_from_package( + /// Resolves an npm package folder path from an npm package referrer. + pub fn resolve_package_folder_from_package( &self, name: &str, referrer: &ModuleSpecifier, - ) -> Result { - self.inner.resolve_package_from_package(name, referrer) + ) -> Result { + self + .inner + .resolve_package_folder_from_package(name, referrer) } /// Resolve the root folder of the package the provided specifier is in. /// /// This will error when the provided specifier is not in an npm package. - pub fn resolve_package_from_specifier( + pub fn resolve_package_folder_from_specifier( &self, specifier: &ModuleSpecifier, - ) -> Result { - self.inner.resolve_package_from_specifier(specifier) + ) -> Result { + self.inner.resolve_package_folder_from_specifier(specifier) } /// Gets if the provided specifier is in an npm package. pub fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { - self.resolve_package_from_specifier(specifier).is_ok() + self + .resolve_package_folder_from_specifier(specifier) + .is_ok() } /// If the resolver has resolved any npm packages. @@ -132,9 +134,7 @@ impl RequireNpmResolver for NpmPackageResolver { referrer: &std::path::Path, ) -> Result { let referrer = specifier_to_path(referrer)?; - self - .resolve_package_from_package(specifier, &referrer) - .map(|p| p.folder_path) + self.resolve_package_folder_from_package(specifier, &referrer) } fn resolve_package_folder_from_path( @@ -142,9 +142,7 @@ impl RequireNpmResolver for NpmPackageResolver { path: &Path, ) -> Result { let specifier = specifier_to_path(path)?; - self - .resolve_package_from_specifier(&specifier) - .map(|p| p.folder_path) + self.resolve_package_folder_from_specifier(&specifier) } fn in_npm_package(&self, path: &Path) -> bool { @@ -152,7 +150,9 @@ impl RequireNpmResolver for NpmPackageResolver { Ok(p) => p, Err(_) => return false, }; - self.resolve_package_from_specifier(&specifier).is_ok() + self + .resolve_package_folder_from_specifier(&specifier) + .is_ok() } fn ensure_read_permission(&self, path: &Path) -> Result<(), AnyError> { diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 285b5878686940..0cbb67a0addbcb 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -515,15 +515,7 @@ impl ProcState { &self.npm_resolver, )) .with_context(|| { - format!( - "Could not resolve '{}' from '{}'.", - specifier, - self - .npm_resolver - .resolve_package_from_specifier(&referrer) - .unwrap() - .id - ) + format!("Could not resolve '{}' from '{}'.", specifier, referrer) }); } diff --git a/ext/node/package_json.rs b/ext/node/package_json.rs index 95d773aeb3ad4b..81daa7ca715e46 100644 --- a/ext/node/package_json.rs +++ b/ext/node/package_json.rs @@ -22,6 +22,7 @@ pub struct PackageJson { main: Option, // use .main(...) module: Option, // use .main(...) pub name: Option, + pub version: Option, pub path: PathBuf, pub typ: String, pub types: Option, @@ -37,6 +38,7 @@ impl PackageJson { main: None, module: None, name: None, + version: None, path, typ: "none".to_string(), types: None, @@ -71,6 +73,7 @@ impl PackageJson { let main_val = package_json.get("main"); let module_val = package_json.get("module"); let name_val = package_json.get("name"); + let version_val = package_json.get("version"); let type_val = package_json.get("type"); let bin = package_json.get("bin").map(ToOwned::to_owned); let exports = package_json.get("exports").map(|exports| { @@ -88,6 +91,7 @@ impl PackageJson { .map(|imp| imp.to_owned()); let main = main_val.and_then(|s| s.as_str()).map(|s| s.to_string()); let name = name_val.and_then(|s| s.as_str()).map(|s| s.to_string()); + let version = version_val.and_then(|s| s.as_str()).map(|s| s.to_string()); let module = module_val.and_then(|s| s.as_str()).map(|s| s.to_string()); // Ignore unknown types for forwards compatibility @@ -116,6 +120,7 @@ impl PackageJson { path, main, name, + version, module, typ, types, From 2ac8ef0c59f25c847f9255253dda283cb0420f10 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 20 Sep 2022 12:53:50 -0400 Subject: [PATCH 04/15] Compiling, but completely untested. --- cli/npm/resolvers/local.rs | 107 +++++++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 22 deletions(-) diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index b04fec97e41968..55197d11d8b224 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -1,16 +1,13 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. -use std::cell::RefCell; use std::collections::HashMap; use std::collections::HashSet; use std::fs; use std::path::Path; use std::path::PathBuf; -use std::rc::Rc; use std::sync::Arc; use deno_ast::ModuleSpecifier; -use deno_core::anyhow::anyhow; use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::futures::future::BoxFuture; @@ -57,7 +54,10 @@ impl LocalNpmPackageResolver { resolution, folder_index: Default::default(), registry_url, - root_node_modules_specifier: ModuleSpecifier::from_directory_path(&node_modules_folder).unwrap(), + root_node_modules_specifier: ModuleSpecifier::from_directory_path( + &node_modules_folder, + ) + .unwrap(), root_node_modules_path: node_modules_folder, } } @@ -74,15 +74,22 @@ impl LocalNpmPackageResolver { } } - fn resolve_folder_for_specifier(&self, specifier: &ModuleSpecifier) -> Result { + fn resolve_folder_for_specifier( + &self, + specifier: &ModuleSpecifier, + ) -> Result { match self.maybe_resolve_folder_for_specifier(specifier) { Some(path) => Ok(path), None => bail!("could not find npm package for '{}'", specifier), } } - fn maybe_resolve_folder_for_specifier(&self, specifier: &ModuleSpecifier) -> Option { - let relative_url = self.root_node_modules_specifier.make_relative(specifier)?; + fn maybe_resolve_folder_for_specifier( + &self, + specifier: &ModuleSpecifier, + ) -> Option { + let relative_url = + self.root_node_modules_specifier.make_relative(specifier)?; if relative_url.starts_with("../") { return None; } @@ -99,7 +106,10 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { let resolved_package = self.resolution.resolve_package_from_deno_module(pkg_req)?; let folder_index = self.folder_index.read(); - let root_folder = folder_index.all_folders.get(&self.root_node_modules_path).unwrap(); + let root_folder = folder_index + .all_folders + .get(&self.root_node_modules_path) + .unwrap(); let sub_dir_name = if root_folder .folder_names .contains(&resolved_package.id.to_string()) @@ -119,8 +129,22 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { ) -> Result { let local_path = self.resolve_folder_for_specifier(referrer)?; let package_root_path = self.resolve_package_root(&local_path); - self.resolution.resolve_package_from_package(name, referrer) - todo!() + let mut current_folder = package_root_path.as_path(); + loop { + let sub_dir = current_folder.join("node_modules").join(name); + if sub_dir.is_dir() { + return Ok(sub_dir); + } + if current_folder == self.root_node_modules_path { + bail!( + "could not find package '{}' from referrer '{}'.", + name, + referrer + ); + } + current_folder = current_folder.parent().unwrap().parent().unwrap(); + debug_assert_eq!(current_folder.file_stem().unwrap(), "node_modules"); + } } fn resolve_package_folder_from_specifier( @@ -183,10 +207,16 @@ fn setup_node_modules( .collect::>(); top_level_packages.sort_by(|a, b| a.id.cmp(&b.id)); - let folders_index = create_virtual_node_modules_folder(snapshot, root_node_modules_dir_path); + let folders_index = + create_virtual_node_modules_folder(snapshot, root_node_modules_dir_path); // todo(dsherret): ensure only one process enters this at a time. - sync_folder_with_fs(&folders_index, &root_node_modules_dir_path, cache, registry_url)?; + sync_folder_with_fs( + &folders_index, + &root_node_modules_dir_path, + cache, + registry_url, + )?; Ok(folders_index) } @@ -214,7 +244,9 @@ impl NodeModulesFolder { pub fn add_folder(&mut self, folder_name: String, package_id: NpmPackageId) { self.folder_names.insert(folder_name.clone()); - self.packages_to_folder_names.insert(package_id, folder_name); + self + .packages_to_folder_names + .insert(package_id, folder_name); } } @@ -233,7 +265,8 @@ fn create_virtual_node_modules_folder( let mut folders_index = NodeModulesFolderIndex { all_folders: Default::default(), }; - let mut root_folder = NodeModulesFolder::new(root_node_modules_path.to_path_buf()); + let mut root_folder = + NodeModulesFolder::new(root_node_modules_path.to_path_buf()); // go over all the top level packages to ensure they're // kept in the top level folder @@ -250,16 +283,30 @@ fn create_virtual_node_modules_folder( }; root_folder.add_folder(folder_name.clone(), package.id.clone()); } - folders_index.all_folders.insert(root_node_modules_path.to_path_buf(), root_folder); + folders_index + .all_folders + .insert(root_node_modules_path.to_path_buf(), root_folder); // now go over each package and sub packages and populate them in the folder for top_level_package in &top_level_packages { let sub_node_modules_path = { - let root_folder = folders_index.all_folders.get(root_node_modules_path).unwrap(); - let sub_dir = root_folder.packages_to_folder_names.get(&top_level_package.id).unwrap(); + let root_folder = folders_index + .all_folders + .get(root_node_modules_path) + .unwrap(); + let sub_dir = root_folder + .packages_to_folder_names + .get(&top_level_package.id) + .unwrap(); root_folder.path.join(sub_dir).join("node_modules") }; - populate_folder_deps(top_level_package, &sub_node_modules_path, root_node_modules_path, &mut folders_index, snapshot); + populate_folder_deps( + top_level_package, + &sub_node_modules_path, + root_node_modules_path, + &mut folders_index, + snapshot, + ); } folders_index } @@ -281,15 +328,27 @@ fn populate_folder_deps( folders_index, ) { let sub_node_modules_path = { - let node_modules_folder = folders_index.all_folders.entry(insert_folder.clone()).or_insert_with(|| NodeModulesFolder::new(insert_folder)); + let node_modules_folder = folders_index + .all_folders + .entry(insert_folder.clone()) + .or_insert_with(|| NodeModulesFolder::new(insert_folder)); node_modules_folder.add_folder(package_ref_name.clone(), id.clone()); - node_modules_folder.path.join(package_ref_name).join("node_modules") + node_modules_folder + .path + .join(package_ref_name) + .join("node_modules") }; // now go through all this module's dependencies let package = snapshot.package_from_id(id).unwrap(); - populate_folder_deps(&package, &sub_node_modules_path, root_node_modules_path, folders_index, snapshot); + populate_folder_deps( + &package, + &sub_node_modules_path, + root_node_modules_path, + folders_index, + snapshot, + ); } } } @@ -364,7 +423,11 @@ fn sync_folder_with_fs( let local_folder_path = output_dir.join(folder_name); let sub_node_modules_path = local_folder_path.join("node_modules"); let cache_folder = cache.package_folder(&package_id, registry_url); - let folder_kind = if folders_index.all_folders.get(&sub_node_modules_path).is_none() { + let folder_kind = if folders_index + .all_folders + .get(&sub_node_modules_path) + .is_none() + { FolderKind::Symlink } else { FolderKind::SubDir From 0d534fe1c1184bbb5bce423188456803c48025a4 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 20 Sep 2022 14:29:04 -0400 Subject: [PATCH 05/15] Committing current code which is all wrong. --- cli/npm/resolvers/local.rs | 53 ++++++++++++++++++++++++++++++-------- cli/npm/resolvers/mod.rs | 9 ++++--- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index 55197d11d8b224..333b13c6ba4ae0 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -65,7 +65,7 @@ impl LocalNpmPackageResolver { fn resolve_package_root(&self, path: &Path) -> PathBuf { let mut last_found = path; loop { - let parent = path.parent().unwrap(); + let parent = last_found.parent().unwrap(); if parent.file_name().unwrap() == "node_modules" { return last_found.to_path_buf(); } else { @@ -110,7 +110,7 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { .all_folders .get(&self.root_node_modules_path) .unwrap(); - let sub_dir_name = if root_folder + let package_name = if root_folder .folder_names .contains(&resolved_package.id.to_string()) { @@ -119,7 +119,10 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { } else { resolved_package.id.name.clone() }; - Ok(self.root_node_modules_path.join(sub_dir_name)) + Ok(join_package_name( + &self.root_node_modules_path, + &package_name, + )) } fn resolve_package_folder_from_package( @@ -131,7 +134,8 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { let package_root_path = self.resolve_package_root(&local_path); let mut current_folder = package_root_path.as_path(); loop { - let sub_dir = current_folder.join("node_modules").join(name); + let sub_dir = + join_package_name(¤t_folder.join("node_modules"), name); if sub_dir.is_dir() { return Ok(sub_dir); } @@ -142,8 +146,7 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { referrer ); } - current_folder = current_folder.parent().unwrap().parent().unwrap(); - debug_assert_eq!(current_folder.file_stem().unwrap(), "node_modules"); + current_folder = get_next_node_modules_ancestor(current_folder); } } @@ -188,8 +191,9 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { } fn ensure_read_permission(&self, path: &Path) -> Result<(), AnyError> { - let registry_path = self.cache.registry_folder(&self.registry_url); - ensure_registry_read_permission(®istry_path, path) + return Ok(()); + // let registry_path = self.cache.registry_folder(&self.registry_url); + // ensure_registry_read_permission(®istry_path, path) } } @@ -374,8 +378,8 @@ fn get_insert_folder( } } } - // go up two folders to the next node_modules - let parent = sub_node_modules_path.parent().unwrap().parent().unwrap(); + // go up to the next node_modules + let parent = get_next_node_modules_ancestor(sub_node_modules_path); if parent == root_node_modules_path { // no name found, so insert in the root folder return Some(root_node_modules_path.to_path_buf()); @@ -385,6 +389,25 @@ fn get_insert_folder( } } +fn join_package_name(path: &Path, package_name: &str) -> PathBuf { + let mut path = path.to_path_buf(); + // ensure backslashes are used on windows + for part in package_name.split('/') { + path = path.join(part); + } + path +} + +fn get_next_node_modules_ancestor(mut path: &Path) -> &Path { + loop { + path = path.parent().unwrap(); + let file_name = path.file_name().unwrap().to_string_lossy(); + if file_name == "node_modules" { + return path; + } + } +} + #[derive(Default, Serialize, Deserialize)] struct FolderData { packages: HashMap, @@ -420,7 +443,7 @@ fn sync_folder_with_fs( .and_then(|text| serde_json::from_str(&text).ok()) .unwrap_or_default(); for (package_id, folder_name) in &folder.packages_to_folder_names { - let local_folder_path = output_dir.join(folder_name); + let local_folder_path = join_package_name(output_dir, folder_name); let sub_node_modules_path = local_folder_path.join("node_modules"); let cache_folder = cache.package_folder(&package_id, registry_url); let folder_kind = if folders_index @@ -440,6 +463,14 @@ fn sync_folder_with_fs( == Some(&expected_folder_data_package); if !state_matches { + // some packages might contain a slash and thus be in a sub dir, so create this dir + if folder_name.contains('/') { + let parent = local_folder_path.parent().unwrap(); + if !parent.exists() { + fs::create_dir(parent)?; + } + } + match folder_kind { FolderKind::Symlink => { remove_dir_all(&local_folder_path)?; diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index ecc8923429ebd8..9d71770bd5f65f 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -59,6 +59,7 @@ impl NpmPackageResolver { &self, pkg_req: &NpmPackageReq, ) -> Result { + log::debug!("Resolving {}", pkg_req); self.inner.resolve_package_folder_from_deno_module(pkg_req) } @@ -68,6 +69,7 @@ impl NpmPackageResolver { name: &str, referrer: &ModuleSpecifier, ) -> Result { + log::debug!("Resolving {} from {}", name, referrer); self .inner .resolve_package_folder_from_package(name, referrer) @@ -80,6 +82,7 @@ impl NpmPackageResolver { &self, specifier: &ModuleSpecifier, ) -> Result { + log::debug!("Resolving {}", specifier); self.inner.resolve_package_folder_from_specifier(specifier) } @@ -133,7 +136,7 @@ impl RequireNpmResolver for NpmPackageResolver { specifier: &str, referrer: &std::path::Path, ) -> Result { - let referrer = specifier_to_path(referrer)?; + let referrer = path_to_specifier(referrer)?; self.resolve_package_folder_from_package(specifier, &referrer) } @@ -141,7 +144,7 @@ impl RequireNpmResolver for NpmPackageResolver { &self, path: &Path, ) -> Result { - let specifier = specifier_to_path(path)?; + let specifier = path_to_specifier(path)?; self.resolve_package_folder_from_specifier(&specifier) } @@ -160,7 +163,7 @@ impl RequireNpmResolver for NpmPackageResolver { } } -fn specifier_to_path(path: &Path) -> Result { +fn path_to_specifier(path: &Path) -> Result { match ModuleSpecifier::from_file_path(&path) { Ok(specifier) => Ok(specifier), Err(()) => bail!("Could not convert '{}' to url.", path.display()), From fd7d660fa323cc05de00e27703e6013429d8fbe4 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 20 Sep 2022 16:55:39 -0400 Subject: [PATCH 06/15] Use a pnpm style node_modules folder --- cli/fs_util.rs | 54 +++++ cli/npm/resolvers/local.rs | 485 +++++++++---------------------------- 2 files changed, 166 insertions(+), 373 deletions(-) diff --git a/cli/fs_util.rs b/cli/fs_util.rs index f3a4addc0381aa..4a26f94848e0a8 100644 --- a/cli/fs_util.rs +++ b/cli/fs_util.rs @@ -294,6 +294,60 @@ pub async fn remove_dir_all_if_exists(path: &Path) -> std::io::Result<()> { } } +/// Copies a directory to another directory. +/// +/// Note: Does not handle symlinks. +pub fn copy_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> { + std::fs::create_dir_all(&to) + .with_context(|| format!("Creating {}", to.display()))?; + let read_dir = std::fs::read_dir(&from) + .with_context(|| format!("Reading {}", from.display()))?; + + for entry in read_dir { + let entry = entry?; + let file_type = entry.file_type()?; + let new_from = from.join(entry.file_name()); + let new_to = to.join(entry.file_name()); + + if file_type.is_dir() { + copy_dir_recursive(&new_from, &new_to).with_context(|| { + format!("Dir {} to {}", new_from.display(), new_to.display()) + })?; + } else if file_type.is_file() { + std::fs::copy(&new_from, &new_to).with_context(|| { + format!("Copying {} to {}", new_from.display(), new_to.display()) + })?; + } + } + + Ok(()) +} + +pub fn symlink_dir(oldpath: &Path, newpath: &Path) -> Result<(), AnyError> { + let err_mapper = |err: Error| { + Error::new( + err.kind(), + format!( + "{}, symlink '{}' -> '{}'", + err, + oldpath.display(), + newpath.display() + ), + ) + }; + #[cfg(unix)] + { + use std::os::unix::fs::symlink; + symlink(&oldpath, &newpath).map_err(err_mapper)?; + } + #[cfg(not(unix))] + { + use std::os::windows::fs::symlink_dir; + symlink_dir(&oldpath, &newpath).map_err(err_mapper)?; + } + Ok(()) +} + /// Attempts to convert a specifier to a file path. By default, uses the Url /// crate's `to_file_path()` method, but falls back to try and resolve unix-style /// paths on Windows. diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index 333b13c6ba4ae0..21d3058273b29d 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -1,7 +1,7 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. -use std::collections::HashMap; use std::collections::HashSet; +use std::collections::VecDeque; use std::fs; use std::path::Path; use std::path::PathBuf; @@ -9,22 +9,19 @@ use std::sync::Arc; use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; +use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::futures::future::BoxFuture; use deno_core::futures::FutureExt; -use deno_core::parking_lot::RwLock; -use deno_core::serde_json; use deno_core::url::Url; -use serde::Deserialize; -use serde::Serialize; +use crate::fs_util; use crate::npm::resolution::NpmResolution; use crate::npm::resolution::NpmResolutionSnapshot; use crate::npm::NpmCache; use crate::npm::NpmPackageId; use crate::npm::NpmPackageReq; use crate::npm::NpmRegistryApi; -use crate::npm::NpmResolutionPackage; use super::common::cache_packages; use super::common::ensure_registry_read_permission; @@ -34,7 +31,6 @@ use super::common::InnerNpmPackageResolver; pub struct LocalNpmPackageResolver { cache: NpmCache, resolution: Arc, - folder_index: Arc>, registry_url: Url, root_node_modules_path: PathBuf, root_node_modules_specifier: ModuleSpecifier, @@ -52,7 +48,6 @@ impl LocalNpmPackageResolver { Self { cache, resolution, - folder_index: Default::default(), registry_url, root_node_modules_specifier: ModuleSpecifier::from_directory_path( &node_modules_folder, @@ -105,24 +100,17 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { ) -> Result { let resolved_package = self.resolution.resolve_package_from_deno_module(pkg_req)?; - let folder_index = self.folder_index.read(); - let root_folder = folder_index - .all_folders - .get(&self.root_node_modules_path) - .unwrap(); - let package_name = if root_folder - .folder_names - .contains(&resolved_package.id.to_string()) - { - // it's the fully resolved package name - resolved_package.id.to_string() - } else { - resolved_package.id.name.clone() - }; - Ok(join_package_name( + + // it might be at the full path if there are duplicate names + let fully_resolved_folder_path = join_package_name( &self.root_node_modules_path, - &package_name, - )) + &resolved_package.id.to_string(), + ); + Ok(if fully_resolved_folder_path.exists() { + fully_resolved_folder_path + } else { + join_package_name(&self.root_node_modules_path, &resolved_package.id.name) + }) } fn resolve_package_folder_from_package( @@ -177,216 +165,137 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { ) .await?; - let folder_index = setup_node_modules( + sync_resolution_with_fs( &resolver.resolution.snapshot(), &resolver.cache, &resolver.registry_url, &resolver.root_node_modules_path, )?; - *resolver.folder_index.write() = folder_index; Ok(()) } .boxed() } fn ensure_read_permission(&self, path: &Path) -> Result<(), AnyError> { - return Ok(()); - // let registry_path = self.cache.registry_folder(&self.registry_url); - // ensure_registry_read_permission(®istry_path, path) + ensure_registry_read_permission(&self.root_node_modules_path, path) } } -fn setup_node_modules( +/// Creates a pnpm style folder structure. +fn sync_resolution_with_fs( snapshot: &NpmResolutionSnapshot, cache: &NpmCache, registry_url: &Url, - root_node_modules_dir_path: &PathBuf, -) -> Result { - // resolve everything to folder structure - let mut top_level_packages = snapshot - .top_level_packages() - .into_iter() - .map(|id| snapshot.package_from_id(&id).unwrap()) - .collect::>(); - top_level_packages.sort_by(|a, b| a.id.cmp(&b.id)); - - let folders_index = - create_virtual_node_modules_folder(snapshot, root_node_modules_dir_path); - - // todo(dsherret): ensure only one process enters this at a time. - sync_folder_with_fs( - &folders_index, - &root_node_modules_dir_path, - cache, - registry_url, - )?; - - Ok(folders_index) -} - -#[derive(Default, Debug)] -struct NodeModulesFolderIndex { - // all folders based on their path - all_folders: HashMap, -} - -#[derive(Default, Debug, Clone)] -struct NodeModulesFolder { - path: PathBuf, - folder_names: HashSet, - packages_to_folder_names: HashMap, -} + root_node_modules_dir_path: &Path, +) -> Result<(), AnyError> { + fn get_package_folder_name(package_id: &NpmPackageId) -> String { + package_id.to_string().replace('/', "+") + } -impl NodeModulesFolder { - pub fn new(path: PathBuf) -> Self { - Self { - path, - ..Default::default() + let deno_local_registry_dir = root_node_modules_dir_path.join(".deno"); + fs::create_dir_all(&deno_local_registry_dir).with_context(|| { + format!("Creating '{}'", deno_local_registry_dir.display()) + })?; + + // 1. Write all the packages out the .deno directory. + // + // Copy (hardlink in future) // to + // node_modules/.deno//node_modules/ + let all_packages = snapshot.all_packages(); + for package in &all_packages { + let folder_name = get_package_folder_name(&package.id); + let folder_path = deno_local_registry_dir.join(&folder_name); + let initialized_file = folder_path.join("deno_initialized"); + if !initialized_file.exists() { + let sub_node_modules = folder_path.join("node_modules"); + let package_path = join_package_name(&sub_node_modules, &package.id.name); + fs::create_dir_all(&package_path) + .with_context(|| format!("Creating '{}'", folder_path.display()))?; + let cache_folder = cache.package_folder(&package.id, registry_url); + // for now copy, but in the future consider hard linking + fs_util::copy_dir_recursive(&cache_folder, &package_path)?; + // write out a file that indicates this folder has been initialized + fs::write(initialized_file, "")?; } } - pub fn add_folder(&mut self, folder_name: String, package_id: NpmPackageId) { - self.folder_names.insert(folder_name.clone()); - self - .packages_to_folder_names - .insert(package_id, folder_name); + // 2. Symlink all the dependencies into the .deno directory. + // + // Symlink node_modules/.deno//node_modules/ to + // node_modules/.deno//node_modules/ + for package in &all_packages { + let sub_node_modules = deno_local_registry_dir + .join(&get_package_folder_name(&package.id)) + .join("node_modules"); + for (name, dep_id) in &package.dependencies { + let dep_folder_name = get_package_folder_name(dep_id); + let dep_folder_path = join_package_name( + &deno_local_registry_dir + .join(dep_folder_name) + .join("node_modules"), + &dep_id.name, + ); + symlink_package_dir( + &dep_folder_path, + &join_package_name(&sub_node_modules, name), + )?; + } } -} - -fn create_virtual_node_modules_folder( - snapshot: &NpmResolutionSnapshot, - root_node_modules_path: &Path, -) -> NodeModulesFolderIndex { - // resolve everything to folder structure - let mut top_level_packages = snapshot - .top_level_packages() - .into_iter() - .map(|id| snapshot.package_from_id(&id).unwrap()) - .collect::>(); - top_level_packages.sort_by(|a, b| a.id.cmp(&b.id)); - let mut folders_index = NodeModulesFolderIndex { - all_folders: Default::default(), - }; - let mut root_folder = - NodeModulesFolder::new(root_node_modules_path.to_path_buf()); - - // go over all the top level packages to ensure they're - // kept in the top level folder - for package in &top_level_packages { - let folder_name = if root_folder.folder_names.contains(&package.id.name) { - // This is when you say have two packages like so: - // import chalkv4 from "npm:chalk@4" - // import chalkv5 from "npm:chalk@5" - // In this scenario, we use the full resolved package id - // for the second package. - package.id.to_string() + // 3. Create all the packages in the node_modules folder, which are symlinks. + // + // Symlink node_modules/ to + // node_modules/.deno//node_modules/ + let mut found_names = HashSet::new(); + let mut pending_packages = VecDeque::new(); + pending_packages.extend( + snapshot + .top_level_packages() + .into_iter() + .map(|id| (id, true)), + ); + while let Some((package_id, is_top_level)) = pending_packages.pop_front() { + let root_folder_name = if found_names.insert(package_id.name.clone()) { + package_id.name.clone() + } else if is_top_level { + package_id.to_string() } else { - package.id.name.to_string() + continue; // skip, already handled }; - root_folder.add_folder(folder_name.clone(), package.id.clone()); + let local_registry_package_path = deno_local_registry_dir + .join(&get_package_folder_name(&package_id)) + .join("node_modules") + .join(&package_id.name); + + symlink_package_dir( + &local_registry_package_path, + &join_package_name(root_node_modules_dir_path, &root_folder_name), + )?; + if let Some(package) = snapshot.package_from_id(&package_id) { + for id in package.dependencies.values() { + pending_packages.push_back((id.clone(), false)); + } + } } - folders_index - .all_folders - .insert(root_node_modules_path.to_path_buf(), root_folder); - // now go over each package and sub packages and populate them in the folder - for top_level_package in &top_level_packages { - let sub_node_modules_path = { - let root_folder = folders_index - .all_folders - .get(root_node_modules_path) - .unwrap(); - let sub_dir = root_folder - .packages_to_folder_names - .get(&top_level_package.id) - .unwrap(); - root_folder.path.join(sub_dir).join("node_modules") - }; - populate_folder_deps( - top_level_package, - &sub_node_modules_path, - root_node_modules_path, - &mut folders_index, - snapshot, - ); - } - folders_index + Ok(()) } -fn populate_folder_deps( - package: &NpmResolutionPackage, - sub_node_modules_path: &Path, - root_node_modules_path: &Path, - folders_index: &mut NodeModulesFolderIndex, - snapshot: &NpmResolutionSnapshot, -) { - // package_ref_name is what the package refers to the other package as - for (package_ref_name, id) in &package.dependencies { - if let Some(insert_folder) = get_insert_folder( - package_ref_name, - id, - sub_node_modules_path, - root_node_modules_path, - folders_index, - ) { - let sub_node_modules_path = { - let node_modules_folder = folders_index - .all_folders - .entry(insert_folder.clone()) - .or_insert_with(|| NodeModulesFolder::new(insert_folder)); - node_modules_folder.add_folder(package_ref_name.clone(), id.clone()); - node_modules_folder - .path - .join(package_ref_name) - .join("node_modules") - }; - - // now go through all this module's dependencies - let package = snapshot.package_from_id(id).unwrap(); - - populate_folder_deps( - &package, - &sub_node_modules_path, - root_node_modules_path, - folders_index, - snapshot, - ); - } +fn symlink_package_dir( + old_path: &Path, + new_path: &Path, +) -> Result<(), AnyError> { + let new_parent = new_path.parent().unwrap(); + if new_parent.file_name().unwrap() != "node_modules" { + // create the parent folder that will contain the symlink + fs::create_dir_all(new_parent) + .with_context(|| format!("Creating '{}'", new_parent.display()))?; } -} -fn get_insert_folder( - package_ref_name: &String, - id: &NpmPackageId, - sub_node_modules_path: &Path, - root_node_modules_path: &Path, - folders_index: &mut NodeModulesFolderIndex, -) -> Option { - let mut current_folder_path = sub_node_modules_path.to_path_buf(); - loop { - if let Some(folder) = folders_index.all_folders.get(¤t_folder_path) { - if folder.folder_names.contains(package_ref_name) { - if folder.packages_to_folder_names.get(id) == Some(package_ref_name) { - // same name and id exists in the tree, so ignore - return None; - } else { - // same name, but different id exists in the tree, so - // use the child folder - return Some(sub_node_modules_path.to_path_buf()); - } - } - } - // go up to the next node_modules - let parent = get_next_node_modules_ancestor(sub_node_modules_path); - if parent == root_node_modules_path { - // no name found, so insert in the root folder - return Some(root_node_modules_path.to_path_buf()); - } - debug_assert_eq!(parent.file_stem().unwrap(), "node_modules"); - current_folder_path = parent.to_path_buf(); - } + // need to delete the previous symlink before creating a new one + let _ignore = fs::remove_dir(new_path); + fs_util::symlink_dir(old_path, new_path) } fn join_package_name(path: &Path, package_name: &str) -> PathBuf { @@ -407,173 +316,3 @@ fn get_next_node_modules_ancestor(mut path: &Path) -> &Path { } } } - -#[derive(Default, Serialize, Deserialize)] -struct FolderData { - packages: HashMap, -} - -#[derive(Serialize, Deserialize, PartialEq, Eq)] -struct FolderDataPackage { - id: String, - kind: FolderKind, -} - -#[derive(Serialize, Deserialize, PartialEq, Eq, Clone, Copy)] -enum FolderKind { - Symlink, - SubDir, -} - -fn sync_folder_with_fs( - folders_index: &NodeModulesFolderIndex, - output_dir: &PathBuf, - cache: &NpmCache, - registry_url: &Url, -) -> Result<(), AnyError> { - let folder = match folders_index.all_folders.get(output_dir) { - Some(folder) => folder, - None => return Ok(()), // nothing to do - }; - // create the folders - fs::create_dir_all(output_dir)?; - let resolution_file = output_dir.join(".deno_resolution"); - let mut folder_data: FolderData = fs::read_to_string(&resolution_file) - .ok() - .and_then(|text| serde_json::from_str(&text).ok()) - .unwrap_or_default(); - for (package_id, folder_name) in &folder.packages_to_folder_names { - let local_folder_path = join_package_name(output_dir, folder_name); - let sub_node_modules_path = local_folder_path.join("node_modules"); - let cache_folder = cache.package_folder(&package_id, registry_url); - let folder_kind = if folders_index - .all_folders - .get(&sub_node_modules_path) - .is_none() - { - FolderKind::Symlink - } else { - FolderKind::SubDir - }; - let expected_folder_data_package = FolderDataPackage { - id: package_id.to_string(), - kind: folder_kind, - }; - let state_matches = folder_data.packages.get(&package_id.name) - == Some(&expected_folder_data_package); - - if !state_matches { - // some packages might contain a slash and thus be in a sub dir, so create this dir - if folder_name.contains('/') { - let parent = local_folder_path.parent().unwrap(); - if !parent.exists() { - fs::create_dir(parent)?; - } - } - - match folder_kind { - FolderKind::Symlink => { - remove_dir_all(&local_folder_path)?; - // no sub packages, so create a symlink - symlink_dir(&cache_folder, &local_folder_path)?; - } - FolderKind::SubDir => { - // there's sub packages, so symlink the children - symlink_dir_children(&cache_folder, &local_folder_path)?; - } - } - folder_data - .packages - .insert(package_id.name.to_string(), expected_folder_data_package); - } - - if folder_kind == FolderKind::SubDir { - let sub_folder = local_folder_path.join("node_modules"); - sync_folder_with_fs(folders_index, &sub_folder, cache, registry_url)?; - } - } - - if let Ok(text) = serde_json::to_string(&folder_data) { - let _ignore = fs::write(resolution_file, text); - } - - Ok(()) -} - -fn remove_dir_all(path: &Path) -> Result<(), AnyError> { - match fs::remove_dir_all(path) { - Ok(_) => Ok(()), - Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(()), - Err(err) => Err(err.into()), - } -} - -fn symlink_dir_children( - oldpath: &Path, - newpath: &Path, -) -> Result<(), AnyError> { - debug_assert!(oldpath.is_dir()); - fs::create_dir(&newpath)?; - for entry in fs::read_dir(oldpath)? { - let entry = entry?; - if entry.file_type()?.is_dir() { - symlink_dir(&entry.path(), &newpath.join(entry.file_name()))?; - } else { - symlink_file(&entry.path(), &newpath.join(entry.file_name()))?; - } - } - Ok(()) -} - -// todo(dsherret): try to consolidate these symlink_dir and symlink_file functions -fn symlink_dir(oldpath: &Path, newpath: &Path) -> Result<(), AnyError> { - use std::io::Error; - let err_mapper = |err: Error| { - Error::new( - err.kind(), - format!( - "{}, symlink '{}' -> '{}'", - err, - oldpath.display(), - newpath.display() - ), - ) - }; - #[cfg(unix)] - { - use std::os::unix::fs::symlink; - symlink(&oldpath, &newpath).map_err(err_mapper)?; - } - #[cfg(not(unix))] - { - use std::os::windows::fs::symlink_dir; - symlink_dir(&oldpath, &newpath).map_err(err_mapper)?; - } - Ok(()) -} - -fn symlink_file(oldpath: &Path, newpath: &Path) -> Result<(), AnyError> { - use std::io::Error; - let err_mapper = |err: Error| { - Error::new( - err.kind(), - format!( - "{}, symlink '{}' -> '{}'", - err, - oldpath.display(), - newpath.display() - ), - ) - }; - #[cfg(unix)] - { - use std::os::unix::fs::symlink; - symlink(&oldpath, &newpath).map_err(err_mapper)?; - } - #[cfg(not(unix))] - { - use std::os::windows::fs::symlink_file; - symlink_file(&oldpath, &newpath).map_err(err_mapper)?; - } - Ok(()) -} From 6a79fe37524dbc78aefb862d89d14664be48afda Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 20 Sep 2022 17:00:05 -0400 Subject: [PATCH 07/15] Updates. --- cli/args/mod.rs | 10 ++++++---- cli/npm/resolvers/mod.rs | 2 -- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/args/mod.rs b/cli/args/mod.rs index a28f7e45c54be2..53a0b77fdf48f9 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -42,6 +42,7 @@ use crate::emit::TsConfigWithIgnoredOptions; use crate::emit::TsTypeLib; use crate::file_fetcher::get_root_cert_store; use crate::file_fetcher::CacheSetting; +use crate::fs_util; use crate::lockfile::Lockfile; use crate::version; @@ -150,17 +151,18 @@ impl CliOptions { pub fn resolve_local_node_modules_folder( &self, ) -> Result, AnyError> { - if !self.flags.local_npm { + let path = if !self.flags.local_npm { return Ok(None); } else if let Some(config_path) = self .maybe_config_file .as_ref() .and_then(|c| c.specifier.to_file_path().ok()) { - Ok(Some(config_path.parent().unwrap().join("node_modules"))) + config_path.parent().unwrap().join("node_modules") } else { - Ok(Some(std::env::current_dir()?.join("node_modules"))) - } + std::env::current_dir()?.join("node_modules") + }; + Ok(Some(fs_util::canonicalize_path(&path)?)) } pub fn resolve_root_cert_store(&self) -> Result { diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 9d71770bd5f65f..c2c30256d05d29 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -37,8 +37,6 @@ impl NpmPackageResolver { no_npm: bool, local_node_modules: Option, ) -> Self { - // For now, always create a GlobalNpmPackageResolver, but in the future - // this might be a local node_modules folder let inner: Arc = match local_node_modules { Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new( cache, From 97fbb4742d5a427e7656180ad259248b1691a70f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 20 Sep 2022 20:19:57 -0400 Subject: [PATCH 08/15] Add some tests and fix issues. --- Cargo.lock | 1 - cli/Cargo.toml | 1 - cli/args/mod.rs | 3 +- cli/node/mod.rs | 2 +- cli/npm/resolvers/mod.rs | 17 ++++---- cli/proc_state.rs | 4 +- cli/tests/integration/npm_tests.rs | 18 ++++++++ .../require-added-nm-folder/1.0.0/index.js | 3 ++ .../1.0.0/package.json | 4 ++ .../npm/require_added_nm_folder/main.js | 10 +++++ .../npm/require_added_nm_folder/main.out | 1 + ext/node/lib.rs | 2 + ext/node/path.rs | 42 +++++++++++++++++++ ext/node/resolution.rs | 2 +- test_util/src/lib.rs | 20 ++++++--- 15 files changed, 110 insertions(+), 20 deletions(-) create mode 100644 cli/tests/testdata/npm/registry/@denotest/require-added-nm-folder/1.0.0/index.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/require-added-nm-folder/1.0.0/package.json create mode 100644 cli/tests/testdata/npm/require_added_nm_folder/main.js create mode 100644 cli/tests/testdata/npm/require_added_nm_folder/main.out create mode 100644 ext/node/path.rs diff --git a/Cargo.lock b/Cargo.lock index f602d3af2ff87c..9a8a2c49bb02a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -830,7 +830,6 @@ dependencies = [ "notify", "once_cell", "os_pipe", - "path-clean", "percent-encoding 2.2.0", "pin-project", "pretty_assertions", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index fa7284d54ca440..7fa0afa76e3964 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -85,7 +85,6 @@ monch = "=0.2.0" notify = "=5.0.0" once_cell = "=1.14.0" os_pipe = "=1.0.1" -path-clean = "=0.1.0" percent-encoding = "=2.2.0" pin-project = "1.0.11" # don't pin because they yank crates from cargo rand = { version = "=0.8.5", features = ["small_rng"] } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 53a0b77fdf48f9..7dede0bc15e839 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -42,7 +42,6 @@ use crate::emit::TsConfigWithIgnoredOptions; use crate::emit::TsTypeLib; use crate::file_fetcher::get_root_cert_store; use crate::file_fetcher::CacheSetting; -use crate::fs_util; use crate::lockfile::Lockfile; use crate::version; @@ -162,7 +161,7 @@ impl CliOptions { } else { std::env::current_dir()?.join("node_modules") }; - Ok(Some(fs_util::canonicalize_path(&path)?)) + Ok(Some(path)) } pub fn resolve_root_cert_store(&self) -> Result { diff --git a/cli/node/mod.rs b/cli/node/mod.rs index 2bf2e90aa80693..c265afdf6aa4cf 100644 --- a/cli/node/mod.rs +++ b/cli/node/mod.rs @@ -26,11 +26,11 @@ use deno_runtime::deno_node::package_imports_resolve; use deno_runtime::deno_node::package_resolve; use deno_runtime::deno_node::NodeModuleKind; use deno_runtime::deno_node::PackageJson; +use deno_runtime::deno_node::PathClean; use deno_runtime::deno_node::RequireNpmResolver; use deno_runtime::deno_node::DEFAULT_CONDITIONS; use deno_runtime::deno_node::NODE_GLOBAL_THIS_NAME; use once_cell::sync::Lazy; -use path_clean::PathClean; use regex::Regex; use crate::file_fetcher::FileFetcher; diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index c2c30256d05d29..31dc31e865d1f1 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -4,8 +4,11 @@ mod common; mod global; mod local; +use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; use deno_core::error::custom_error; +use deno_core::error::AnyError; +use deno_runtime::deno_node::PathClean; use deno_runtime::deno_node::RequireNpmResolver; use global::GlobalNpmPackageResolver; @@ -13,9 +16,6 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; -use deno_ast::ModuleSpecifier; -use deno_core::error::AnyError; - use self::common::InnerNpmPackageResolver; use self::local::LocalNpmPackageResolver; use super::NpmCache; @@ -147,10 +147,11 @@ impl RequireNpmResolver for NpmPackageResolver { } fn in_npm_package(&self, path: &Path) -> bool { - let specifier = match ModuleSpecifier::from_file_path(path) { - Ok(p) => p, - Err(_) => return false, - }; + let specifier = + match ModuleSpecifier::from_file_path(&path.to_path_buf().clean()) { + Ok(p) => p, + Err(_) => return false, + }; self .resolve_package_folder_from_specifier(&specifier) .is_ok() @@ -162,7 +163,7 @@ impl RequireNpmResolver for NpmPackageResolver { } fn path_to_specifier(path: &Path) -> Result { - match ModuleSpecifier::from_file_path(&path) { + match ModuleSpecifier::from_file_path(&path.to_path_buf().clean()) { Ok(specifier) => Ok(specifier), Err(()) => bail!("Could not convert '{}' to url.", path.display()), } diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 0cbb67a0addbcb..deb41ab05ae5b7 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -242,7 +242,9 @@ impl ProcState { // don't do the unstable error when in the lsp || matches!(cli_options.sub_command(), DenoSubcommand::Lsp), cli_options.no_npm(), - cli_options.resolve_local_node_modules_folder()?, + cli_options + .resolve_local_node_modules_folder() + .with_context(|| "Resolving local node_modules folder.")?, ); let emit_options: deno_ast::EmitOptions = ts_config_result.ts_config.into(); diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index 49bac64fb3eb5e..44ffdd9a7ff16f 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -423,6 +423,24 @@ itest!(builtin_module_module { http_server: true, }); +itest!(local_npm_require_added_node_modules_folder { + args: + "run --unstable --local-npm -A --quiet $TESTDATA/npm/require_added_nm_folder/main.js", + output: "npm/require_added_nm_folder/main.out", + envs: env_vars(), + http_server: true, + exit_code: 0, + temp_cwd: true, +}); + +itest!(local_npm_with_deps { + args: "run --allow-read --allow-env --unstable --local-npm $TESTDATA/npm/cjs_with_deps/main.js", + output: "npm/cjs_with_deps/main.out", + envs: env_vars(), + http_server: true, + temp_cwd: true, +}); + #[test] fn ensure_registry_files_local() { // ensures the registry files all point at local tarballs diff --git a/cli/tests/testdata/npm/registry/@denotest/require-added-nm-folder/1.0.0/index.js b/cli/tests/testdata/npm/registry/@denotest/require-added-nm-folder/1.0.0/index.js new file mode 100644 index 00000000000000..8c8c4a0fa09c7c --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/require-added-nm-folder/1.0.0/index.js @@ -0,0 +1,3 @@ +exports.getValue = () => { + return require(".other-package").get(); +}; diff --git a/cli/tests/testdata/npm/registry/@denotest/require-added-nm-folder/1.0.0/package.json b/cli/tests/testdata/npm/registry/@denotest/require-added-nm-folder/1.0.0/package.json new file mode 100644 index 00000000000000..718f1eb8cb5561 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/require-added-nm-folder/1.0.0/package.json @@ -0,0 +1,4 @@ +{ + "name": "@denotest/require-added-nm-folder", + "version": "1.0.0" +} diff --git a/cli/tests/testdata/npm/require_added_nm_folder/main.js b/cli/tests/testdata/npm/require_added_nm_folder/main.js new file mode 100644 index 00000000000000..723b2023aafb93 --- /dev/null +++ b/cli/tests/testdata/npm/require_added_nm_folder/main.js @@ -0,0 +1,10 @@ +import { getValue } from "npm:@denotest/require-added-nm-folder"; + +Deno.mkdirSync("./node_modules/.other-package"); +Deno.writeTextFileSync("./node_modules/.other-package/package.json", "{}"); +Deno.writeTextFileSync( + "./node_modules/.other-package/index.js", + "exports.get = () => 5;", +); + +console.log(getValue()); diff --git a/cli/tests/testdata/npm/require_added_nm_folder/main.out b/cli/tests/testdata/npm/require_added_nm_folder/main.out new file mode 100644 index 00000000000000..7ed6ff82de6bcc --- /dev/null +++ b/cli/tests/testdata/npm/require_added_nm_folder/main.out @@ -0,0 +1 @@ +5 diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 42348915ed437c..5178d81f7cf45e 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -15,9 +15,11 @@ use std::rc::Rc; pub mod errors; mod package_json; +mod path; mod resolution; pub use package_json::PackageJson; +pub use path::PathClean; pub use resolution::get_closest_package_json; pub use resolution::get_package_scope_config; pub use resolution::legacy_main_resolve; diff --git a/ext/node/path.rs b/ext/node/path.rs new file mode 100644 index 00000000000000..a413cec4464ca2 --- /dev/null +++ b/ext/node/path.rs @@ -0,0 +1,42 @@ +use std::path::Component; +use std::path::PathBuf; + +/// Extenion to path_clean::PathClean +pub trait PathClean { + fn clean(&self) -> T; +} + +impl PathClean for PathBuf { + fn clean(&self) -> PathBuf { + path_clean(self) + } +} + +fn path_clean(path: &PathBuf) -> PathBuf { + let path = path_clean::PathClean::clean(path); + if path.to_string_lossy().contains("..\\") { + // temporary workaround because path_clean::PathClean::clean is + // not good enough on windows + let mut components = Vec::new(); + + for component in path.components() { + match component { + Component::CurDir => { + // skip + } + Component::ParentDir => { + let poped_component = components.pop(); + if !matches!(poped_component, Some(Component::Normal(_))) { + panic!("Error normalizing: {}", path.display()); + } + } + Component::Normal(_) | Component::RootDir | Component::Prefix(_) => { + components.push(component); + } + } + } + components.into_iter().collect::() + } else { + path + } +} diff --git a/ext/node/resolution.rs b/ext/node/resolution.rs index 52ed06116e067c..1bde9970939820 100644 --- a/ext/node/resolution.rs +++ b/ext/node/resolution.rs @@ -10,11 +10,11 @@ use deno_core::serde_json::Map; use deno_core::serde_json::Value; use deno_core::url::Url; use deno_core::ModuleSpecifier; -use path_clean::PathClean; use regex::Regex; use crate::errors; use crate::package_json::PackageJson; +use crate::path::PathClean; use crate::RequireNpmResolver; pub static DEFAULT_CONDITIONS: &[&str] = &["deno", "node", "import"]; diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index 5a357ee7defc0d..73fe3ff9b0be33 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -1861,6 +1861,7 @@ pub struct CheckOutputIntegrationTest<'a> { pub http_server: bool, pub envs: Vec<(String, String)>, pub env_clear: bool, + pub temp_cwd: bool, } impl<'a> CheckOutputIntegrationTest<'a> { @@ -1874,6 +1875,11 @@ impl<'a> CheckOutputIntegrationTest<'a> { ); std::borrow::Cow::Borrowed(&self.args_vec) }; + let testdata_dir = testdata_path(); + let args = args + .iter() + .map(|arg| arg.replace("$TESTDATA", &testdata_dir.to_string_lossy())) + .collect::>(); let deno_exe = deno_exe_path(); println!("deno_exe path {}", deno_exe.display()); @@ -1884,17 +1890,21 @@ impl<'a> CheckOutputIntegrationTest<'a> { }; let (mut reader, writer) = pipe().unwrap(); - let testdata_dir = testdata_path(); let deno_dir = new_deno_dir(); // keep this alive for the test let mut command = deno_cmd_with_deno_dir(&deno_dir); - println!("deno_exe args {}", self.args); - println!("deno_exe testdata path {:?}", &testdata_dir); + let cwd = if self.temp_cwd { + deno_dir.path() + } else { + testdata_dir.as_path() + }; + println!("deno_exe args {}", args.join(" ")); + println!("deno_exe cwd {:?}", &testdata_dir); command.args(args.iter()); if self.env_clear { command.env_clear(); } command.envs(self.envs.clone()); - command.current_dir(&testdata_dir); + command.current_dir(&cwd); command.stdin(Stdio::piped()); let writer_clone = writer.try_clone().unwrap(); command.stderr(writer_clone); @@ -1949,7 +1959,7 @@ impl<'a> CheckOutputIntegrationTest<'a> { // deno test's output capturing flushes with a zero-width space in order to // synchronize the output pipes. Occassionally this zero width space // might end up in the output so strip it from the output comparison here. - if args.first() == Some(&"test") { + if args.first().map(|s| s.as_str()) == Some("test") { actual = actual.replace('\u{200B}', ""); } From e43cded97f306560155c95c7d3fde974542e9d15 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 21 Sep 2022 10:36:11 -0400 Subject: [PATCH 09/15] Maybe fix CI --- cli/args/mod.rs | 3 ++- cli/fs_util.rs | 32 +++++++++++++++++++++++++++- cli/npm/resolvers/mod.rs | 21 ++++++++++++------ ext/node/path.rs | 46 ++++++++++++++++++---------------------- 4 files changed, 68 insertions(+), 34 deletions(-) diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 7dede0bc15e839..b98fa0ad93030d 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -42,6 +42,7 @@ use crate::emit::TsConfigWithIgnoredOptions; use crate::emit::TsTypeLib; use crate::file_fetcher::get_root_cert_store; use crate::file_fetcher::CacheSetting; +use crate::fs_util; use crate::lockfile::Lockfile; use crate::version; @@ -161,7 +162,7 @@ impl CliOptions { } else { std::env::current_dir()?.join("node_modules") }; - Ok(Some(path)) + Ok(Some(fs_util::canonicalize_path_maybe_not_exists(&path)?)) } pub fn resolve_root_cert_store(&self) -> Result { diff --git a/cli/fs_util.rs b/cli/fs_util.rs index 4a26f94848e0a8..365c9b430115c4 100644 --- a/cli/fs_util.rs +++ b/cli/fs_util.rs @@ -5,10 +5,11 @@ use deno_core::error::{uri_error, AnyError}; pub use deno_core::normalize_path; use deno_core::ModuleSpecifier; use deno_runtime::deno_crypto::rand; +use deno_runtime::deno_node::PathClean; use std::borrow::Cow; use std::env::current_dir; use std::fs::OpenOptions; -use std::io::{Error, Write}; +use std::io::{Error, ErrorKind, Write}; use std::path::{Path, PathBuf}; use walkdir::WalkDir; @@ -75,6 +76,35 @@ pub fn canonicalize_path(path: &Path) -> Result { return Ok(path); } +/// Canonicalizes a path which might be non-existent by going up the +/// ancestors until it finds a directory that exists, canonicalizes +/// that path, then adds back the remaining path components. +/// +/// Note: When using this, you should be aware that a symlink may +/// subsequently be created along this path by some other code. +pub fn canonicalize_path_maybe_not_exists( + path: &Path, +) -> Result { + let path = path.to_path_buf().clean(); + let mut path = path.as_path(); + let mut names_stack = Vec::new(); + loop { + match canonicalize_path(path) { + Ok(mut canonicalized_path) => { + for name in names_stack.into_iter().rev() { + canonicalized_path = canonicalized_path.join(name); + } + return Ok(canonicalized_path); + } + Err(err) if err.kind() == ErrorKind::NotFound => { + names_stack.push(path.file_name().unwrap()); + path = path.parent().unwrap(); + } + Err(err) => return Err(err), + } + } +} + #[cfg(windows)] fn strip_unc_prefix(path: PathBuf) -> PathBuf { use std::path::Component; diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 31dc31e865d1f1..ddc2eca5555f65 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -57,8 +57,11 @@ impl NpmPackageResolver { &self, pkg_req: &NpmPackageReq, ) -> Result { - log::debug!("Resolving {}", pkg_req); - self.inner.resolve_package_folder_from_deno_module(pkg_req) + let path = self + .inner + .resolve_package_folder_from_deno_module(pkg_req)?; + log::debug!("Resolved {} to {}", pkg_req, path.display()); + Ok(path) } /// Resolves an npm package folder path from an npm package referrer. @@ -67,10 +70,11 @@ impl NpmPackageResolver { name: &str, referrer: &ModuleSpecifier, ) -> Result { - log::debug!("Resolving {} from {}", name, referrer); - self + let path = self .inner - .resolve_package_folder_from_package(name, referrer) + .resolve_package_folder_from_package(name, referrer)?; + log::debug!("Resolved {} from {} to {}", name, referrer, path.display()); + Ok(path) } /// Resolve the root folder of the package the provided specifier is in. @@ -80,8 +84,11 @@ impl NpmPackageResolver { &self, specifier: &ModuleSpecifier, ) -> Result { - log::debug!("Resolving {}", specifier); - self.inner.resolve_package_folder_from_specifier(specifier) + let path = self + .inner + .resolve_package_folder_from_specifier(specifier)?; + log::debug!("Resolved {} to {}", specifier, path.display()); + Ok(path) } /// Gets if the provided specifier is in an npm package. diff --git a/ext/node/path.rs b/ext/node/path.rs index a413cec4464ca2..8477fe7137d48b 100644 --- a/ext/node/path.rs +++ b/ext/node/path.rs @@ -8,35 +8,31 @@ pub trait PathClean { impl PathClean for PathBuf { fn clean(&self) -> PathBuf { - path_clean(self) - } -} + let path = path_clean::PathClean::clean(self); + if cfg!(windows) && path.to_string_lossy().contains("..\\") { + // temporary workaround because path_clean::PathClean::clean is + // not good enough on windows + let mut components = Vec::new(); -fn path_clean(path: &PathBuf) -> PathBuf { - let path = path_clean::PathClean::clean(path); - if path.to_string_lossy().contains("..\\") { - // temporary workaround because path_clean::PathClean::clean is - // not good enough on windows - let mut components = Vec::new(); - - for component in path.components() { - match component { - Component::CurDir => { - // skip - } - Component::ParentDir => { - let poped_component = components.pop(); - if !matches!(poped_component, Some(Component::Normal(_))) { - panic!("Error normalizing: {}", path.display()); + for component in path.components() { + match component { + Component::CurDir => { + // skip + } + Component::ParentDir => { + let poped_component = components.pop(); + if !matches!(poped_component, Some(Component::Normal(_))) { + panic!("Error normalizing: {}", path.display()); + } + } + Component::Normal(_) | Component::RootDir | Component::Prefix(_) => { + components.push(component); } - } - Component::Normal(_) | Component::RootDir | Component::Prefix(_) => { - components.push(component); } } + components.into_iter().collect::() + } else { + path } - components.into_iter().collect::() - } else { - path } } From d4529a7d1f7f6244ce9811f9b3d15ad486d10e66 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 21 Sep 2022 15:31:21 -0400 Subject: [PATCH 10/15] Fix issue. --- cli/npm/resolvers/local.rs | 5 ++--- cli/npm/resolvers/mod.rs | 3 +++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index 21d3058273b29d..6ee23842411a54 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -122,8 +122,8 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { let package_root_path = self.resolve_package_root(&local_path); let mut current_folder = package_root_path.as_path(); loop { - let sub_dir = - join_package_name(¤t_folder.join("node_modules"), name); + current_folder = get_next_node_modules_ancestor(current_folder); + let sub_dir = join_package_name(current_folder, name); if sub_dir.is_dir() { return Ok(sub_dir); } @@ -134,7 +134,6 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver { referrer ); } - current_folder = get_next_node_modules_ancestor(current_folder); } } diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index ddc2eca5555f65..57781986b6d693 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -16,6 +16,8 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use crate::fs_util; + use self::common::InnerNpmPackageResolver; use self::local::LocalNpmPackageResolver; use super::NpmCache; @@ -60,6 +62,7 @@ impl NpmPackageResolver { let path = self .inner .resolve_package_folder_from_deno_module(pkg_req)?; + let path = fs_util::canonicalize_path_maybe_not_exists(&path)?; log::debug!("Resolved {} to {}", pkg_req, path.display()); Ok(path) } From 04154dca529bca2ff058dbfe6046643d0e142619 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 21 Sep 2022 16:18:28 -0400 Subject: [PATCH 11/15] Apparently need to use remove_dir_all --- cli/npm/resolvers/local.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index 6ee23842411a54..16dd8aa933bf85 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -293,7 +293,7 @@ fn symlink_package_dir( } // need to delete the previous symlink before creating a new one - let _ignore = fs::remove_dir(new_path); + let _ignore = fs::remove_dir_all(new_path); fs_util::symlink_dir(old_path, new_path) } From c7cf320ee72093f42a4c92dd9cb42f42632198b7 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 21 Sep 2022 17:08:48 -0400 Subject: [PATCH 12/15] Add copyright --- cli/npm/resolvers/common.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cli/npm/resolvers/common.rs b/cli/npm/resolvers/common.rs index c114101b758420..cc590e2ad66cbb 100644 --- a/cli/npm/resolvers/common.rs +++ b/cli/npm/resolvers/common.rs @@ -1,3 +1,5 @@ +// Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. + use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; From 5a351418dfd9de2df7b5f2a3e01424a1da390f5f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 21 Sep 2022 17:15:33 -0400 Subject: [PATCH 13/15] Comments. --- cli/npm/resolvers/global.rs | 3 +++ cli/npm/resolvers/local.rs | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/cli/npm/resolvers/global.rs b/cli/npm/resolvers/global.rs index 6a5156e90068c7..94b963898a8abf 100644 --- a/cli/npm/resolvers/global.rs +++ b/cli/npm/resolvers/global.rs @@ -1,5 +1,7 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +//! Code for global npm cache resolution. + use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -20,6 +22,7 @@ use crate::npm::NpmRegistryApi; use super::common::ensure_registry_read_permission; use super::common::InnerNpmPackageResolver; +/// Resolves packages from the global npm cache. #[derive(Debug, Clone)] pub struct GlobalNpmPackageResolver { cache: NpmCache, diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index 16dd8aa933bf85..d92ffb84d9e625 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -1,5 +1,7 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +//! Code for local node_modules resolution. + use std::collections::HashSet; use std::collections::VecDeque; use std::fs; @@ -27,6 +29,8 @@ use super::common::cache_packages; use super::common::ensure_registry_read_permission; use super::common::InnerNpmPackageResolver; +/// Resolver that creates a local node_modules directory +/// and resolves packages from it. #[derive(Debug, Clone)] pub struct LocalNpmPackageResolver { cache: NpmCache, From 470167c93ab9c8693007a55a4d92b39a9d42a965 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 21 Sep 2022 17:18:18 -0400 Subject: [PATCH 14/15] Update flag name. --- cli/args/flags.rs | 15 ++++++++------- cli/args/mod.rs | 4 ++-- cli/npm/resolvers/mod.rs | 5 +++-- cli/tests/integration/npm_tests.rs | 4 ++-- cli/tools/standalone.rs | 2 +- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 1910585ed34514..d1cb587062f66d 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -301,7 +301,7 @@ pub struct Flags { pub cached_only: bool, pub type_check_mode: TypeCheckMode, pub config_flag: ConfigFlag, - pub local_npm: bool, + pub node_modules_dir: bool, pub coverage_dir: Option, pub enable_testing_features: bool, pub ignore: Vec, @@ -2153,8 +2153,8 @@ fn no_npm_arg<'a>() -> Arg<'a> { } fn local_npm_arg<'a>() -> Arg<'a> { - Arg::new("local-npm") - .long("local-npm") + Arg::new("node-modules-dir") + .long("node-modules-dir") .help("Creates a local node_modules folder") } @@ -3069,8 +3069,8 @@ fn no_npm_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } fn local_npm_args_parse(flags: &mut Flags, matches: &ArgMatches) { - if matches.is_present("local-npm") { - flags.local_npm = true; + if matches.is_present("node-modules-dir") { + flags.node_modules_dir = true; } } @@ -5049,14 +5049,15 @@ mod tests { #[test] fn local_npm() { - let r = flags_from_vec(svec!["deno", "run", "--local-npm", "script.ts"]); + let r = + flags_from_vec(svec!["deno", "run", "--node-modules-dir", "script.ts"]); assert_eq!( r.unwrap(), Flags { subcommand: DenoSubcommand::Run(RunFlags { script: "script.ts".to_string(), }), - local_npm: true, + node_modules_dir: true, ..Flags::default() } ); diff --git a/cli/args/mod.rs b/cli/args/mod.rs index b98fa0ad93030d..1e0c72b1c8d385 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -147,11 +147,11 @@ impl CliOptions { self.overrides.import_map_specifier = Some(path); } - /// Resolves the folder to use for a local node_modules folder. + /// Resolves the path to use for a local node_modules folder. pub fn resolve_local_node_modules_folder( &self, ) -> Result, AnyError> { - let path = if !self.flags.local_npm { + let path = if !self.flags.node_modules_dir { return Ok(None); } else if let Some(config_path) = self .maybe_config_file diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 57781986b6d693..3a40340f0b14ad 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -37,9 +37,10 @@ impl NpmPackageResolver { api: NpmRegistryApi, unstable: bool, no_npm: bool, - local_node_modules: Option, + local_node_modules_path: Option, ) -> Self { - let inner: Arc = match local_node_modules { + let inner: Arc = match local_node_modules_path + { Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new( cache, api, diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index 44ffdd9a7ff16f..e2dcbb38f71f53 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -425,7 +425,7 @@ itest!(builtin_module_module { itest!(local_npm_require_added_node_modules_folder { args: - "run --unstable --local-npm -A --quiet $TESTDATA/npm/require_added_nm_folder/main.js", + "run --unstable --node-modules-dir -A --quiet $TESTDATA/npm/require_added_nm_folder/main.js", output: "npm/require_added_nm_folder/main.out", envs: env_vars(), http_server: true, @@ -434,7 +434,7 @@ itest!(local_npm_require_added_node_modules_folder { }); itest!(local_npm_with_deps { - args: "run --allow-read --allow-env --unstable --local-npm $TESTDATA/npm/cjs_with_deps/main.js", + args: "run --allow-read --allow-env --unstable --node-modules-dir $TESTDATA/npm/cjs_with_deps/main.js", output: "npm/cjs_with_deps/main.out", envs: env_vars(), http_server: true, diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index 2c1bc9f7a4be8f..35170b6b9c0018 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -270,7 +270,7 @@ pub fn compile_to_runtime_flags( import_map_path: flags.import_map_path.clone(), inspect_brk: None, inspect: None, - local_npm: false, + node_modules_dir: false, location: flags.location.clone(), lock_write: false, lock: None, From 4cdf325ab313c02f9cd4cccad7096b4d00ec275e Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 22 Sep 2022 09:46:57 -0400 Subject: [PATCH 15/15] Add test for deno cache --- cli/tests/integration/npm_tests.rs | 32 ++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index e2dcbb38f71f53..8aeefb6c0444a4 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -423,7 +423,7 @@ itest!(builtin_module_module { http_server: true, }); -itest!(local_npm_require_added_node_modules_folder { +itest!(node_modules_dir_require_added_node_modules_folder { args: "run --unstable --node-modules-dir -A --quiet $TESTDATA/npm/require_added_nm_folder/main.js", output: "npm/require_added_nm_folder/main.out", @@ -433,7 +433,7 @@ itest!(local_npm_require_added_node_modules_folder { temp_cwd: true, }); -itest!(local_npm_with_deps { +itest!(node_modules_dir_with_deps { args: "run --allow-read --allow-env --unstable --node-modules-dir $TESTDATA/npm/cjs_with_deps/main.js", output: "npm/cjs_with_deps/main.out", envs: env_vars(), @@ -441,6 +441,34 @@ itest!(local_npm_with_deps { temp_cwd: true, }); +#[test] +fn node_modules_dir_cache() { + let _server = http_server(); + + let deno_dir = util::new_deno_dir(); + + let deno = util::deno_cmd_with_deno_dir(&deno_dir) + .current_dir(deno_dir.path()) + .arg("cache") + .arg("--unstable") + .arg("--node-modules-dir") + .arg("--quiet") + .arg(util::testdata_path().join("npm/dual_cjs_esm/main.ts")) + .envs(env_vars()) + .spawn() + .unwrap(); + let output = deno.wait_with_output().unwrap(); + assert!(output.status.success()); + + let node_modules = deno_dir.path().join("node_modules"); + assert!(node_modules + .join( + ".deno/@denotest+dual-cjs-esm@1.0.0/node_modules/@denotest/dual-cjs-esm" + ) + .exists()); + assert!(node_modules.join("@denotest/dual-cjs-esm").exists()); +} + #[test] fn ensure_registry_files_local() { // ensures the registry files all point at local tarballs