fix(lock): ensure npm dependencies are written with --lock-write (#16668)

If "--lock-write" flag was present we never passed instance of the lockfile to 
the npm resolver, which made it skip adding discovered npm packages to
the lockfile. This commit fixes that, by always passing lockfile to the npm
resolver and only regenerating resolver snapshot is "--lock-write" is not
present.

Closes https://github.com/denoland/deno/issues/16666

cli/npm/resolvers/mod.rs

@@ -104,10 +104,16 @@ impl NpmPackageResolver {
 
   /// This function will replace current resolver with a new one built from a
   /// snapshot created out of the lockfile.
-  pub async fn add_lockfile(
+  pub async fn add_lockfile_and_maybe_regenerate_snapshot(
     &mut self,
     lockfile: Arc<Mutex<Lockfile>>,
   ) -> Result<(), AnyError> {
+    self.maybe_lockfile = Some(lockfile.clone());
+
+    if lockfile.lock().overwrite {
+      return Ok(());
+    }
+
     let snapshot =
       NpmResolutionSnapshot::from_lockfile(lockfile.clone(), &self.api)
         .await
@@ -117,7 +123,6 @@ impl NpmPackageResolver {
             lockfile.lock().filename.display()
           )
         })?;
-    self.maybe_lockfile = Some(lockfile);
     if let Some(node_modules_folder) = &self.local_node_modules_path {
       self.inner = Arc::new(LocalNpmPackageResolver::new(
         self.cache.clone(),

cli/proc_state.rs

@@ -224,8 +224,7 @@ impl ProcState {
       cli_options.cache_setting(),
       progress_bar.clone(),
     );
-    let maybe_lockfile =
+    let maybe_lockfile = lockfile.as_ref().cloned();
-      lockfile.as_ref().filter(|l| !l.lock().overwrite).cloned();
     let mut npm_resolver = NpmPackageResolver::new(
       npm_cache.clone(),
       api,
@@ -235,7 +234,9 @@ impl ProcState {
         .with_context(|| "Resolving local node_modules folder.")?,
     );
     if let Some(lockfile) = maybe_lockfile.clone() {
-      npm_resolver.add_lockfile(lockfile).await?;
+      npm_resolver
+        .add_lockfile_and_maybe_regenerate_snapshot(lockfile)
+        .await?;
     }
     let node_analysis_cache =
       NodeAnalysisCache::new(Some(dir.node_analysis_db_file_path()));

cli/tests/integration/npm_tests.rs

@@ -1,5 +1,6 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 
+use pretty_assertions::assert_eq;
 use std::process::Stdio;
 use test_util as util;
 use util::assert_contains;
@@ -1025,6 +1026,226 @@ fn lock_file_missing_top_level_package() {
   );
 }
 
+#[test]
+fn lock_file_lock_write() {
+  // https://github.com/denoland/deno/issues/16666
+  // Ensure that --lock-write still adds npm packages to the lockfile
+  let _server = http_server();
+
+  let deno_dir = util::new_deno_dir();
+  let temp_dir = util::TempDir::new();
+
+  // write empty config file
+  temp_dir.write("deno.json", "{}");
+
+  // write a lock file with borked integrity
+  let lock_file_content = r#"{
+  "version": "2",
+  "remote": {},
+  "npm": {
+    "specifiers": { "cowsay@1.5.0": "cowsay@1.5.0" },
+    "packages": {
+      "ansi-regex@3.0.1": {
+        "integrity": "sha512-+O9Jct8wf++lXxxFc4hc8LsjaSq0HFzzL7cVsw8pRDIPdjKD2mT4ytDZlLuSBZ4cLKZFXIrMGO7DbQCtMJJMKw==",
+        "dependencies": {}
+      },
+      "ansi-regex@5.0.1": {
+        "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==",
+        "dependencies": {}
+      },
+      "ansi-styles@4.3.0": {
+        "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==",
+        "dependencies": { "color-convert": "color-convert@2.0.1" }
+      },
+      "camelcase@5.3.1": {
+        "integrity": "sha512-L28STB170nwWS63UjtlEOE3dldQApaJXZkOI1uMFfzf3rRuPegHaHesyee+YxQ+W6SvRDQV6UrdOdRiR153wJg==",
+        "dependencies": {}
+      },
+      "cliui@6.0.0": {
+        "integrity": "sha512-t6wbgtoCXvAzst7QgXxJYqPt0usEfbgQdftEPbLL/cvv6HPE5VgvqCuAIDR0NgU52ds6rFwqrgakNLrHEjCbrQ==",
+        "dependencies": {
+          "string-width": "string-width@4.2.3",
+          "strip-ansi": "strip-ansi@6.0.1",
+          "wrap-ansi": "wrap-ansi@6.2.0"
+        }
+      },
+      "color-convert@2.0.1": {
+        "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==",
+        "dependencies": { "color-name": "color-name@1.1.4" }
+      },
+      "color-name@1.1.4": {
+        "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==",
+        "dependencies": {}
+      },
+      "cowsay@1.5.0": {
+        "integrity": "sha512-8Ipzr54Z8zROr/62C8f0PdhQcDusS05gKTS87xxdji8VbWefWly0k8BwGK7+VqamOrkv3eGsCkPtvlHzrhWsCA==",
+        "dependencies": {
+          "get-stdin": "get-stdin@8.0.0",
+          "string-width": "string-width@2.1.1",
+          "strip-final-newline": "strip-final-newline@2.0.0",
+          "yargs": "yargs@15.4.1"
+        }
+      },
+      "decamelize@1.2.0": {
+        "integrity": "sha512-z2S+W9X73hAUUki+N+9Za2lBlun89zigOyGrsax+KUQ6wKW4ZoWpEYBkGhQjwAjjDCkWxhY0VKEhk8wzY7F5cA==",
+        "dependencies": {}
+      },
+      "emoji-regex@8.0.0": {
+        "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==",
+        "dependencies": {}
+      },
+      "find-up@4.1.0": {
+        "integrity": "sha512-PpOwAdQ/YlXQ2vj8a3h8IipDuYRi3wceVQQGYWxNINccq40Anw7BlsEXCMbt1Zt+OLA6Fq9suIpIWD0OsnISlw==",
+        "dependencies": {
+          "locate-path": "locate-path@5.0.0",
+          "path-exists": "path-exists@4.0.0"
+        }
+      },
+      "get-caller-file@2.0.5": {
+        "integrity": "sha512-DyFP3BM/3YHTQOCUL/w0OZHR0lpKeGrxotcHWcqNEdnltqFwXVfhEBQ94eIo34AfQpo0rGki4cyIiftY06h2Fg==",
+        "dependencies": {}
+      },
+      "get-stdin@8.0.0": {
+        "integrity": "sha512-sY22aA6xchAzprjyqmSEQv4UbAAzRN0L2dQB0NlN5acTTK9Don6nhoc3eAbUnpZiCANAMfd/+40kVdKfFygohg==",
+        "dependencies": {}
+      },
+      "is-fullwidth-code-point@2.0.0": {
+        "integrity": "sha512-VHskAKYM8RfSFXwee5t5cbN5PZeq1Wrh6qd5bkyiXIf6UQcN6w/A0eXM9r6t8d+GYOh+o6ZhiEnb88LN/Y8m2w==",
+        "dependencies": {}
+      },
+      "is-fullwidth-code-point@3.0.0": {
+        "integrity": "sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg==",
+        "dependencies": {}
+      },
+      "locate-path@5.0.0": {
+        "integrity": "sha512-t7hw9pI+WvuwNJXwk5zVHpyhIqzg2qTlklJOf0mVxGSbe3Fp2VieZcduNYjaLDoy6p9uGpQEGWG87WpMKlNq8g==",
+        "dependencies": { "p-locate": "p-locate@4.1.0" }
+      },
+      "p-limit@2.3.0": {
+        "integrity": "sha512-//88mFWSJx8lxCzwdAABTJL2MyWB12+eIY7MDL2SqLmAkeKU9qxRvWuSyTjm3FUmpBEMuFfckAIqEaVGUDxb6w==",
+        "dependencies": { "p-try": "p-try@2.2.0" }
+      },
+      "p-locate@4.1.0": {
+        "integrity": "sha512-R79ZZ/0wAxKGu3oYMlz8jy/kbhsNrS7SKZ7PxEHBgJ5+F2mtFW2fK2cOtBh1cHYkQsbzFV7I+EoRKe6Yt0oK7A==",
+        "dependencies": { "p-limit": "p-limit@2.3.0" }
+      },
+      "p-try@2.2.0": {
+        "integrity": "sha512-R4nPAVTAU0B9D35/Gk3uJf/7XYbQcyohSKdvAxIRSNghFl4e71hVoGnBNQz9cWaXxO2I10KTC+3jMdvvoKw6dQ==",
+        "dependencies": {}
+      },
+      "path-exists@4.0.0": {
+        "integrity": "sha512-ak9Qy5Q7jYb2Wwcey5Fpvg2KoAc/ZIhLSLOSBmRmygPsGwkVVt0fZa0qrtMz+m6tJTAHfZQ8FnmB4MG4LWy7/w==",
+        "dependencies": {}
+      },
+      "require-directory@2.1.1": {
+        "integrity": "sha512-fGxEI7+wsG9xrvdjsrlmL22OMTTiHRwAMroiEeMgq8gzoLC/PQr7RsRDSTLUg/bZAZtF+TVIkHc6/4RIKrui+Q==",
+        "dependencies": {}
+      },
+      "require-main-filename@2.0.0": {
+        "integrity": "sha512-NKN5kMDylKuldxYLSUfrbo5Tuzh4hd+2E8NPPX02mZtn1VuREQToYe/ZdlJy+J3uCpfaiGF05e7B8W0iXbQHmg==",
+        "dependencies": {}
+      },
+      "set-blocking@2.0.0": {
+        "integrity": "sha512-KiKBS8AnWGEyLzofFfmvKwpdPzqiy16LvQfK3yv/fVH7Bj13/wl3JSR1J+rfgRE9q7xUJK4qvgS8raSOeLUehw==",
+        "dependencies": {}
+      },
+      "string-width@2.1.1": {
+        "integrity": "sha512-nOqH59deCq9SRHlxq1Aw85Jnt4w6KvLKqWVik6oA9ZklXLNIOlqg4F2yrT1MVaTjAqvVwdfeZ7w7aCvJD7ugkw==",
+        "dependencies": {
+          "is-fullwidth-code-point": "is-fullwidth-code-point@2.0.0",
+          "strip-ansi": "strip-ansi@4.0.0"
+        }
+      },
+      "string-width@4.2.3": {
+        "integrity": "sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==",
+        "dependencies": {
+          "emoji-regex": "emoji-regex@8.0.0",
+          "is-fullwidth-code-point": "is-fullwidth-code-point@3.0.0",
+          "strip-ansi": "strip-ansi@6.0.1"
+        }
+      },
+      "strip-ansi@4.0.0": {
+        "integrity": "sha512-4XaJ2zQdCzROZDivEVIDPkcQn8LMFSa8kj8Gxb/Lnwzv9A8VctNZ+lfivC/sV3ivW8ElJTERXZoPBRrZKkNKow==",
+        "dependencies": { "ansi-regex": "ansi-regex@3.0.1" }
+      },
+      "strip-ansi@6.0.1": {
+        "integrity": "sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==",
+        "dependencies": { "ansi-regex": "ansi-regex@5.0.1" }
+      },
+      "strip-final-newline@2.0.0": {
+        "integrity": "sha512-BrpvfNAE3dcvq7ll3xVumzjKjZQ5tI1sEUIKr3Uoks0XUl45St3FlatVqef9prk4jRDzhW6WZg+3bk93y6pLjA==",
+        "dependencies": {}
+      },
+      "which-module@2.0.0": {
+        "integrity": "sha512-B+enWhmw6cjfVC7kS8Pj9pCrKSc5txArRyaYGe088shv/FGWH+0Rjx/xPgtsWfsUtS27FkP697E4DDhgrgoc0Q==",
+        "dependencies": {}
+      },
+      "wrap-ansi@6.2.0": {
+        "integrity": "sha512-r6lPcBGxZXlIcymEu7InxDMhdW0KDxpLgoFLcguasxCaJ/SOIZwINatK9KY/tf+ZrlywOKU0UDj3ATXUBfxJXA==",
+        "dependencies": {
+          "ansi-styles": "ansi-styles@4.3.0",
+          "string-width": "string-width@4.2.3",
+          "strip-ansi": "strip-ansi@6.0.1"
+        }
+      },
+      "y18n@4.0.3": {
+        "integrity": "sha512-JKhqTOwSrqNA1NY5lSztJ1GrBiUodLMmIZuLiDaMRJ+itFd+ABVE8XBjOvIWL+rSqNDC74LCSFmlb/U4UZ4hJQ==",
+        "dependencies": {}
+      },
+      "yargs-parser@18.1.3": {
+        "integrity": "sha512-o50j0JeToy/4K6OZcaQmW6lyXXKhq7csREXcDwk2omFPJEwUNOVtJKvmDr9EI1fAJZUyZcRF7kxGBWmRXudrCQ==",
+        "dependencies": {
+          "camelcase": "camelcase@5.3.1",
+          "decamelize": "decamelize@1.2.0"
+        }
+      },
+      "yargs@15.4.1": {
+        "integrity": "sha512-aePbxDmcYW++PaqBsJ+HYUFwCdv4LVvdnhBy78E57PIor8/OVvhMrADFFEDh8DHDFRv/O9i3lPhsENjO7QX0+A==",
+        "dependencies": {
+          "cliui": "cliui@6.0.0",
+          "decamelize": "decamelize@1.2.0",
+          "find-up": "find-up@4.1.0",
+          "get-caller-file": "get-caller-file@2.0.5",
+          "require-directory": "require-directory@2.1.1",
+          "require-main-filename": "require-main-filename@2.0.0",
+          "set-blocking": "set-blocking@2.0.0",
+          "string-width": "string-width@4.2.3",
+          "which-module": "which-module@2.0.0",
+          "y18n": "y18n@4.0.3",
+          "yargs-parser": "yargs-parser@18.1.3"
+        }
+      }
+    }
+  }
+}
+"#;
+  temp_dir.write("deno.lock", lock_file_content);
+
+  let deno = util::deno_cmd_with_deno_dir(&deno_dir)
+    .current_dir(temp_dir.path())
+    .arg("cache")
+    .arg("--lock-write")
+    .arg("--quiet")
+    .arg("npm:cowsay@1.5.0")
+    .envs(env_vars())
+    .stdout(Stdio::piped())
+    .stderr(Stdio::piped())
+    .spawn()
+    .unwrap();
+  let output = deno.wait_with_output().unwrap();
+  assert!(output.status.success());
+  assert_eq!(output.status.code(), Some(0));
+
+  let stdout = String::from_utf8(output.stdout).unwrap();
+  assert!(stdout.is_empty());
+  let stderr = String::from_utf8(output.stderr).unwrap();
+  assert!(stderr.is_empty());
+  assert_eq!(
+    lock_file_content,
+    std::fs::read_to_string(temp_dir.path().join("deno.lock")).unwrap()
+  );
+}
+
 #[test]
 fn auto_discover_lock_file() {
   let _server = http_server();