1 files changed, 26 insertions, 56 deletions

diff --git a/cli/npm/tarball.rs b/cli/npm/tarball.rs
index 3971e0b07..751e093f5 100644
--- a/cli/npm/tarball.rs
+++ b/cli/npm/tarball.rs
@@ -6,18 +6,17 @@ use std::path::Path;
 use std::path::PathBuf;
 
 use deno_core::anyhow::bail;
-use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use flate2::read::GzDecoder;
 use tar::Archive;
 use tar::EntryType;
 
-use super::cache::NPM_PACKAGE_SYNC_LOCK_FILENAME;
+use super::cache::with_folder_sync_lock;
 use super::registry::NpmPackageVersionDistInfo;
-use super::NpmPackageId;
+use super::semver::NpmVersion;
 
 pub fn verify_and_extract_tarball(
-  package: &NpmPackageId,
+  package: (&str, &NpmVersion),
   data: &[u8],
   dist_info: &NpmPackageVersionDistInfo,
   output_folder: &Path,
@@ -27,50 +26,19 @@ pub fn verify_and_extract_tarball(
   } else {
     // todo(dsherret): check shasum here
     bail!(
-      "Errored on '{}': npm packages with no integrity are not implemented.",
-      package
+      "Errored on '{}@{}': npm packages with no integrity are not implemented.",
+      package.0,
+      package.1,
     );
   }
 
-  fs::create_dir_all(output_folder).with_context(|| {
-    format!("Error creating '{}'.", output_folder.display())
-  })?;
-
-  // This sync lock file is a way to ensure that partially created
-  // npm package directories aren't considered valid. This could maybe
-  // be a bit smarter in the future to not bother extracting here
-  // if another process has taken the lock in the past X seconds and
-  // wait for the other process to finish (it could try to create the
-  // file with `create_new(true)` then if it exists, check the metadata
-  // then wait until the other process finishes with a timeout), but
-  // for now this is good enough.
-  let sync_lock_path = output_folder.join(NPM_PACKAGE_SYNC_LOCK_FILENAME);
-  match fs::OpenOptions::new()
-    .write(true)
-    .create(true)
-    .open(&sync_lock_path)
-  {
-    Ok(_) => {
-      extract_tarball(data, output_folder)?;
-      // extraction succeeded, so only now delete this file
-      let _ignore = std::fs::remove_file(&sync_lock_path);
-      Ok(())
-    }
-    Err(err) => {
-      bail!(
-        concat!(
-          "Error creating package sync lock file at '{}'. ",
-          "Maybe try manually deleting this folder.\n\n{:#}",
-        ),
-        output_folder.display(),
-        err
-      );
-    }
-  }
+  with_folder_sync_lock(package, output_folder, || {
+    extract_tarball(data, output_folder)
+  })
 }
 
 fn verify_tarball_integrity(
-  package: &NpmPackageId,
+  package: (&str, &NpmVersion),
   data: &[u8],
   npm_integrity: &str,
 ) -> Result<(), AnyError> {
@@ -81,16 +49,18 @@ fn verify_tarball_integrity(
       let algo = match hash_kind {
         "sha512" => &SHA512,
         hash_kind => bail!(
-          "Not implemented hash function for {}: {}",
-          package,
+          "Not implemented hash function for {}@{}: {}",
+          package.0,
+          package.1,
           hash_kind
         ),
       };
       (algo, checksum.to_lowercase())
     }
     None => bail!(
-      "Not implemented integrity kind for {}: {}",
-      package,
+      "Not implemented integrity kind for {}@{}: {}",
+      package.0,
+      package.1,
       npm_integrity
     ),
   };
@@ -101,8 +71,9 @@ fn verify_tarball_integrity(
   let tarball_checksum = base64::encode(digest.as_ref()).to_lowercase();
   if tarball_checksum != expected_checksum {
     bail!(
-      "Tarball checksum did not match what was provided by npm registry for {}.\n\nExpected: {}\nActual: {}",
-      package,
+      "Tarball checksum did not match what was provided by npm registry for {}@{}.\n\nExpected: {}\nActual: {}",
+      package.0,
+      package.1,
       expected_checksum,
       tarball_checksum,
     )
@@ -162,32 +133,31 @@ mod test {
 
   #[test]
   pub fn test_verify_tarball() {
-    let package_id = NpmPackageId {
-      name: "package".to_string(),
-      version: NpmVersion::parse("1.0.0").unwrap(),
-    };
+    let package_name = "package".to_string();
+    let package_version = NpmVersion::parse("1.0.0").unwrap();
+    let package = (package_name.as_str(), &package_version);
     let actual_checksum =
       "z4phnx7vul3xvchq1m2ab9yg5aulvxxcg/spidns6c5h0ne8xyxysp+dgnkhfuwvy7kxvudbeoglodj6+sfapg==";
     assert_eq!(
-      verify_tarball_integrity(&package_id, &Vec::new(), "test")
+      verify_tarball_integrity(package, &Vec::new(), "test")
         .unwrap_err()
         .to_string(),
       "Not implemented integrity kind for package@1.0.0: test",
     );
     assert_eq!(
-      verify_tarball_integrity(&package_id, &Vec::new(), "sha1-test")
+      verify_tarball_integrity(package, &Vec::new(), "sha1-test")
         .unwrap_err()
         .to_string(),
       "Not implemented hash function for package@1.0.0: sha1",
     );
     assert_eq!(
-      verify_tarball_integrity(&package_id, &Vec::new(), "sha512-test")
+      verify_tarball_integrity(package, &Vec::new(), "sha512-test")
         .unwrap_err()
         .to_string(),
       format!("Tarball checksum did not match what was provided by npm registry for package@1.0.0.\n\nExpected: test\nActual: {}", actual_checksum),
     );
     assert!(verify_tarball_integrity(
-      &package_id,
+      package,
       &Vec::new(),
       &format!("sha512-{}", actual_checksum)
     )