refactor(compile): store the npm snapshot in the eszip (#19343)

Cargo.lock

@@ -1233,9 +1233,9 @@ dependencies = [
 
 [[package]]
 name = "deno_npm"
-version = "0.6.0"
+version = "0.8.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "54607b69689ab1e778e5e00545456e6f0c2310205e1bdae01af601c2dace0121"
+checksum = "a6ea17879274b55063c6b14488a4f9352b651f8d42ed129e6e2c1e351761175d"
 dependencies = [
  "anyhow",
  "async-trait",
@@ -1842,14 +1842,16 @@ dependencies = [
 
 [[package]]
 name = "eszip"
-version = "0.43.0"
+version = "0.44.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9034d1749b91ac4fab0ed7b1d849f9f933099d1c1d021d42d6f54dd265b27d83"
+checksum = "176a97e524a9cfa38393fae75c97d249cf41742fc40664529206c5249c12b599"
 dependencies = [
  "anyhow",
  "base64 0.21.0",
  "deno_ast",
  "deno_graph",
+ "deno_npm",
+ "deno_semver",
  "futures",
  "hashlink",
  "serde",

Cargo.toml

@@ -53,7 +53,7 @@ deno_bench_util = { version = "0.101.0", path = "./bench_util" }
 test_util = { path = "./test_util" }
 deno_lockfile = "0.14.1"
 deno_media_type = { version = "0.1.0", features = ["module_specifier"] }
-deno_npm = "0.6.0"
+deno_npm = "0.8.0"
 deno_semver = "0.2.1"
 
 # exts

cli/Cargo.toml

@@ -51,7 +51,7 @@ deno_npm.workspace = true
 deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] }
 deno_semver.workspace = true
 deno_task_shell = "=0.12.0"
-eszip = "=0.43.0"
+eszip = "=0.44.0"
 napi_sym.workspace = true
 
 async-trait.workspace = true

cli/args/lockfile.rs

@@ -15,6 +15,7 @@ use deno_npm::resolution::SerializedNpmResolutionSnapshot;
 use deno_npm::resolution::SerializedNpmResolutionSnapshotPackage;
 use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot;
 use deno_npm::NpmPackageId;
+use deno_npm::NpmResolutionPackageSystemInfo;
 use deno_semver::npm::NpmPackageReq;
 
 use crate::args::ConfigFile;
@@ -95,8 +96,7 @@ pub async fn snapshot_from_lockfile(
         id,
         dependencies,
         // temporarily empty
-        os: Default::default(),
-        cpu: Default::default(),
+        system: Default::default(),
         dist: Default::default(),
         optional_dependencies: Default::default(),
       });
@@ -124,8 +124,10 @@ pub async fn snapshot_from_lockfile(
       Ok(version_info) => {
         let mut package = &mut packages[i];
         package.dist = version_info.dist;
-        package.cpu = version_info.cpu;
-        package.os = version_info.os;
+        package.system = NpmResolutionPackageSystemInfo {
+          cpu: version_info.cpu,
+          os: version_info.os,
+        };
         package.optional_dependencies =
           version_info.optional_dependencies.into_keys().collect();
       }

cli/factory.rs

@@ -30,6 +30,7 @@ use crate::npm::create_npm_fs_resolver;
 use crate::npm::CliNpmRegistryApi;
 use crate::npm::CliNpmResolver;
 use crate::npm::NpmCache;
+use crate::npm::NpmCacheDir;
 use crate::npm::NpmPackageFsResolver;
 use crate::npm::NpmResolution;
 use crate::npm::PackageJsonDepsInstaller;
@@ -270,8 +271,9 @@ impl CliFactory {
   pub fn npm_cache(&self) -> Result<&Arc<NpmCache>, AnyError> {
     self.services.npm_cache.get_or_try_init(|| {
       Ok(Arc::new(NpmCache::new(
-        self.deno_dir()?.npm_folder_path(),
+        NpmCacheDir::new(self.deno_dir()?.npm_folder_path()),
         self.options.cache_setting(),
+        self.fs().clone(),
         self.http_client().clone(),
         self.text_only_progress_bar().clone(),
       )))

cli/lsp/language_server.rs

@@ -97,6 +97,7 @@ use crate::npm::create_npm_fs_resolver;
 use crate::npm::CliNpmRegistryApi;
 use crate::npm::CliNpmResolver;
 use crate::npm::NpmCache;
+use crate::npm::NpmCacheDir;
 use crate::npm::NpmResolution;
 use crate::tools::fmt::format_file;
 use crate::tools::fmt::format_parsed_source;
@@ -542,12 +543,13 @@ fn create_npm_api_and_cache(
   progress_bar: &ProgressBar,
 ) -> (Arc<CliNpmRegistryApi>, Arc<NpmCache>) {
   let npm_cache = Arc::new(NpmCache::new(
-    dir.npm_folder_path(),
+    NpmCacheDir::new(dir.npm_folder_path()),
     // Use an "only" cache setting in order to make the
     // user do an explicit "cache" command and prevent
     // the cache from being filled with lots of packages while
     // the user is typing.
     CacheSetting::Only,
+    Arc::new(deno_fs::RealFs),
     http_client.clone(),
     progress_bar.clone(),
   ));

cli/npm/cache.rs

@@ -15,6 +15,7 @@ use deno_core::parking_lot::Mutex;
 use deno_core::url::Url;
 use deno_npm::registry::NpmPackageVersionDistInfo;
 use deno_npm::NpmPackageCacheFolderId;
+use deno_runtime::deno_fs;
 use deno_semver::npm::NpmPackageNv;
 use deno_semver::Version;
 use once_cell::sync::Lazy;
@@ -113,13 +114,13 @@ pub fn with_folder_sync_lock(
 }
 
 #[derive(Clone, Debug)]
-pub struct ReadonlyNpmCache {
+pub struct NpmCacheDir {
   root_dir: PathBuf,
   // cached url representation of the root directory
   root_dir_url: Url,
 }
 
-impl ReadonlyNpmCache {
+impl NpmCacheDir {
   pub fn new(root_dir: PathBuf) -> Self {
     fn try_get_canonicalized_root_dir(
       root_dir: &Path,
@@ -279,8 +280,9 @@ impl ReadonlyNpmCache {
 /// Stores a single copy of npm packages in a cache.
 #[derive(Debug)]
 pub struct NpmCache {
-  readonly: ReadonlyNpmCache,
+  cache_dir: NpmCacheDir,
   cache_setting: CacheSetting,
+  fs: Arc<dyn deno_fs::FileSystem>,
   http_client: Arc<HttpClient>,
   progress_bar: ProgressBar,
   /// ensures a package is only downloaded once per run
@@ -289,22 +291,24 @@ pub struct NpmCache {
 
 impl NpmCache {
   pub fn new(
-    cache_dir_path: PathBuf,
+    cache_dir: NpmCacheDir,
     cache_setting: CacheSetting,
+    fs: Arc<dyn deno_fs::FileSystem>,
     http_client: Arc<HttpClient>,
     progress_bar: ProgressBar,
   ) -> Self {
     Self {
-      readonly: ReadonlyNpmCache::new(cache_dir_path),
+      cache_dir,
       cache_setting,
+      fs,
       http_client,
       progress_bar,
       previously_reloaded_packages: Default::default(),
     }
   }
 
-  pub fn as_readonly(&self) -> ReadonlyNpmCache {
-    self.readonly.clone()
+  pub fn as_readonly(&self) -> NpmCacheDir {
+    self.cache_dir.clone()
   }
 
   pub fn cache_setting(&self) -> &CacheSetting {
@@ -312,7 +316,7 @@ impl NpmCache {
   }
 
   pub fn root_dir_url(&self) -> &Url {
-    self.readonly.root_dir_url()
+    self.cache_dir.root_dir_url()
   }
 
   /// Checks if the cache should be used for the provided name and version.
@@ -350,13 +354,13 @@ impl NpmCache {
     registry_url: &Url,
   ) -> Result<(), AnyError> {
     let package_folder = self
-      .readonly
+      .cache_dir
       .package_folder_for_name_and_version(package, registry_url);
     if self.should_use_global_cache_for_package(package)
-      && package_folder.exists()
+      && self.fs.exists(&package_folder)
       // if this file exists, then the package didn't successfully extract
       // the first time, or another process is currently extracting the zip file
-      && !package_folder.join(NPM_PACKAGE_SYNC_LOCK_FILENAME).exists()
+      && !self.fs.exists(&package_folder.join(NPM_PACKAGE_SYNC_LOCK_FILENAME))
     {
       return Ok(());
     } else if self.cache_setting == CacheSetting::Only {
@@ -370,6 +374,10 @@ impl NpmCache {
       );
     }
 
+    if dist.tarball.is_empty() {
+      bail!("Tarball URL was empty.");
+    }
+
     let guard = self.progress_bar.update(&dist.tarball);
     let maybe_bytes = self
       .http_client
@@ -395,8 +403,9 @@ impl NpmCache {
     registry_url: &Url,
   ) -> Result<(), AnyError> {
     assert_ne!(folder_id.copy_index, 0);
-    let package_folder =
-      self.readonly.package_folder_for_id(folder_id, registry_url);
+    let package_folder = self
+      .cache_dir
+      .package_folder_for_id(folder_id, registry_url);
 
     if package_folder.exists()
       // if this file exists, then the package didn't successfully extract
@@ -408,7 +417,7 @@ impl NpmCache {
     }
 
     let original_package_folder = self
-      .readonly
+      .cache_dir
       .package_folder_for_name_and_version(&folder_id.nv, registry_url);
     with_folder_sync_lock(&folder_id.nv, &package_folder, || {
       hard_link_dir_recursive(&original_package_folder, &package_folder)
@@ -421,7 +430,7 @@ impl NpmCache {
     id: &NpmPackageCacheFolderId,
     registry_url: &Url,
   ) -> PathBuf {
-    self.readonly.package_folder_for_id(id, registry_url)
+    self.cache_dir.package_folder_for_id(id, registry_url)
   }
 
   pub fn package_folder_for_name_and_version(
@@ -430,16 +439,16 @@ impl NpmCache {
     registry_url: &Url,
   ) -> PathBuf {
     self
-      .readonly
+      .cache_dir
       .package_folder_for_name_and_version(package, registry_url)
   }
 
   pub fn package_name_folder(&self, name: &str, registry_url: &Url) -> PathBuf {
-    self.readonly.package_name_folder(name, registry_url)
+    self.cache_dir.package_name_folder(name, registry_url)
   }
 
   pub fn registry_folder(&self, registry_url: &Url) -> PathBuf {
-    self.readonly.registry_folder(registry_url)
+    self.cache_dir.registry_folder(registry_url)
   }
 
   pub fn resolve_package_folder_id_from_specifier(
@@ -448,7 +457,7 @@ impl NpmCache {
     registry_url: &Url,
   ) -> Result<NpmPackageCacheFolderId, AnyError> {
     self
-      .readonly
+      .cache_dir
       .resolve_package_folder_id_from_specifier(specifier, registry_url)
   }
 }
@@ -474,14 +483,14 @@ mod test {
   use deno_semver::npm::NpmPackageNv;
   use deno_semver::Version;
 
-  use super::ReadonlyNpmCache;
+  use super::NpmCacheDir;
   use crate::npm::cache::NpmPackageCacheFolderId;
 
   #[test]
   fn should_get_package_folder() {
     let deno_dir = crate::cache::DenoDir::new(None).unwrap();
     let root_dir = deno_dir.npm_folder_path();
-    let cache = ReadonlyNpmCache::new(root_dir.clone());
+    let cache = NpmCacheDir::new(root_dir.clone());
     let registry_url = Url::parse("https://registry.npmjs.org/").unwrap();
 
     assert_eq!(

cli/npm/mod.rs

@@ -9,6 +9,7 @@ mod tarball;
 
 pub use cache::should_sync_download;
 pub use cache::NpmCache;
+pub use cache::NpmCacheDir;
 pub use installer::PackageJsonDepsInstaller;
 pub use registry::CliNpmRegistryApi;
 pub use resolution::NpmResolution;

cli/npm/resolution.rs

@@ -18,7 +18,6 @@ use deno_npm::resolution::NpmResolutionSnapshotCreateOptions;
 use deno_npm::resolution::PackageNotFoundFromReferrerError;
 use deno_npm::resolution::PackageNvNotFoundError;
 use deno_npm::resolution::PackageReqNotFoundError;
-use deno_npm::resolution::SerializedNpmResolutionSnapshot;
 use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot;
 use deno_npm::NpmPackageCacheFolderId;
 use deno_npm::NpmPackageId;
@@ -50,7 +49,7 @@ impl std::fmt::Debug for NpmResolution {
   fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
     let snapshot = self.snapshot.read();
     f.debug_struct("NpmResolution")
-      .field("snapshot", &snapshot.as_serialized())
+      .field("snapshot", &snapshot.as_valid_serialized().as_serialized())
       .finish()
   }
 }
@@ -263,8 +262,20 @@ impl NpmResolution {
     self.snapshot.read().clone()
   }
 
-  pub fn serialized_snapshot(&self) -> SerializedNpmResolutionSnapshot {
-    self.snapshot.read().as_serialized()
+  pub fn serialized_valid_snapshot(
+    &self,
+  ) -> ValidSerializedNpmResolutionSnapshot {
+    self.snapshot.read().as_valid_serialized()
+  }
+
+  pub fn serialized_valid_snapshot_for_system(
+    &self,
+    system_info: &NpmSystemInfo,
+  ) -> ValidSerializedNpmResolutionSnapshot {
+    self
+      .snapshot
+      .read()
+      .as_valid_serialized_for_system(system_info)
   }
 
   pub fn lock(&self, lockfile: &mut Lockfile) -> Result<(), AnyError> {

cli/npm/resolvers/mod.rs

@@ -200,7 +200,10 @@ impl CliNpmResolver {
   /// Gets the state of npm for the process.
   pub fn get_npm_process_state(&self) -> String {
     serde_json::to_string(&NpmProcessState {
-      snapshot: self.resolution.serialized_snapshot(),
+      snapshot: self
+        .resolution
+        .serialized_valid_snapshot()
+        .into_serialized(),
       local_node_modules_path: self
         .fs_resolver
         .node_modules_path()

cli/standalone/binary.rs

@@ -18,7 +18,6 @@ use deno_core::futures::AsyncSeekExt;
 use deno_core::serde_json;
 use deno_core::url::Url;
 use deno_npm::registry::PackageDepNpmSchemeValueParseError;
-use deno_npm::resolution::SerializedNpmResolutionSnapshot;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::permissions::PermissionsOptions;
 use deno_semver::npm::NpmPackageReq;
@@ -140,7 +139,6 @@ pub struct Metadata {
   pub entrypoint: ModuleSpecifier,
   /// Whether this uses a node_modules directory (true) or the global cache (false).
   pub node_modules_dir: bool,
-  pub npm_snapshot: Option<SerializedNpmResolutionSnapshot>,
   pub package_json_deps: Option<SerializablePackageJsonDeps>,
 }
 
@@ -475,7 +473,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
     &self,
     writer: &mut impl Write,
     original_bin: Vec<u8>,
-    eszip: eszip::EszipV2,
+    mut eszip: eszip::EszipV2,
     entrypoint: &ModuleSpecifier,
     cli_options: &CliOptions,
     compile_flags: &CompileFlags,
@@ -492,14 +490,16 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       .resolve_import_map(self.file_fetcher)
       .await?
       .map(|import_map| (import_map.base_url().clone(), import_map.to_json()));
-    let (npm_snapshot, npm_vfs, npm_files) =
-      if self.npm_resolution.has_packages() {
-        let (root_dir, files) = self.build_vfs()?.into_dir_and_files();
-        let snapshot = self.npm_resolution.serialized_snapshot();
-        (Some(snapshot), Some(root_dir), files)
-      } else {
-        (None, None, Vec::new())
-      };
+    let (npm_vfs, npm_files) = if self.npm_resolution.has_packages() {
+      let (root_dir, files) = self.build_vfs()?.into_dir_and_files();
+      let snapshot = self
+        .npm_resolution
+        .serialized_valid_snapshot_for_system(&self.npm_system_info);
+      eszip.add_npm_snapshot(snapshot);
+      (Some(root_dir), files)
+    } else {
+      (None, Vec::new())
+    };
 
     let metadata = Metadata {
       argv: compile_flags.args.clone(),
@@ -517,7 +517,6 @@ impl<'a> DenoCompileBinaryWriter<'a> {
       entrypoint: entrypoint.clone(),
       maybe_import_map,
       node_modules_dir: self.npm_resolver.node_modules_path().is_some(),
-      npm_snapshot,
       package_json_deps: self
         .package_json_deps_provider
         .deps()

cli/standalone/mod.rs

@@ -18,6 +18,7 @@ use crate::npm::create_npm_fs_resolver;
 use crate::npm::CliNpmRegistryApi;
 use crate::npm::CliNpmResolver;
 use crate::npm::NpmCache;
+use crate::npm::NpmCacheDir;
 use crate::npm::NpmResolution;
 use crate::resolver::MappedSpecifierResolver;
 use crate::util::progress_bar::ProgressBar;
@@ -273,7 +274,7 @@ impl RootCertStoreProvider for StandaloneRootCertStoreProvider {
 }
 
 pub async fn run(
-  eszip: eszip::EszipV2,
+  mut eszip: eszip::EszipV2,
   metadata: Metadata,
 ) -> Result<(), AnyError> {
   let main_module = &metadata.entrypoint;
@@ -296,26 +297,14 @@ pub async fn run(
   let root_path = std::env::temp_dir()
     .join(format!("deno-compile-{}", current_exe_name))
     .join("node_modules");
-
-  let npm_cache = Arc::new(NpmCache::new(
-    root_path.clone(),
-    CacheSetting::Use,
-    http_client.clone(),
-    progress_bar.clone(),
-  ));
-  let npm_api = Arc::new(CliNpmRegistryApi::new(
-    npm_registry_url.clone(),
-    npm_cache.clone(),
-    http_client.clone(),
-    progress_bar.clone(),
-  ));
+  let npm_cache_dir = NpmCacheDir::new(root_path.clone());
   let (fs, vfs_root, node_modules_path, snapshot) = if let Some(snapshot) =
-    metadata.npm_snapshot
+    eszip.take_npm_snapshot()
   {
     let vfs_root_dir_path = if metadata.node_modules_dir {
       root_path
     } else {
-      npm_cache.registry_folder(&npm_registry_url)
+      npm_cache_dir.registry_folder(&npm_registry_url)
     };
     let vfs = load_npm_vfs(vfs_root_dir_path.clone())
       .context("Failed to load npm vfs.")?;
@@ -328,7 +317,7 @@ pub async fn run(
       Arc::new(DenoCompileFileSystem::new(vfs)) as Arc<dyn deno_fs::FileSystem>,
       Some(vfs_root_dir_path),
       node_modules_path,
-      Some(snapshot.into_valid()?),
+      Some(snapshot),
     )
   } else {
     (
@@ -338,6 +327,20 @@ pub async fn run(
       None,
     )
   };
+
+  let npm_cache = Arc::new(NpmCache::new(
+    npm_cache_dir,
+    CacheSetting::Only,
+    fs.clone(),
+    http_client.clone(),
+    progress_bar.clone(),
+  ));
+  let npm_api = Arc::new(CliNpmRegistryApi::new(
+    npm_registry_url.clone(),
+    npm_cache.clone(),
+    http_client.clone(),
+    progress_bar.clone(),
+  ));
   let npm_resolution = Arc::new(NpmResolution::from_serialized(
     npm_api.clone(),
     snapshot,

cli/tests/integration/compile_tests.rs

@@ -1068,6 +1068,9 @@ fn run_npm_bin_compile_test(opts: RunNpmBinCompileOptions) {
   output.assert_exit_code(0);
   output.skip_output_check();
 
+  // delete the npm folder in the DENO_DIR to ensure it's not using it
+  context.deno_dir().remove_dir_all("./npm");
+
   // run
   let binary_path = if cfg!(windows) {
     temp_dir.path().join(format!("{}.exe", opts.expected_name))