From bd159b8bad34acefbd6e222781f3d7c6319c9bef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 16 Nov 2022 19:59:26 +0100 Subject: [PATCH] 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 | 9 +- cli/proc_state.rs | 7 +- cli/tests/integration/npm_tests.rs | 221 +++++++++++++++++++++++++++++ 3 files changed, 232 insertions(+), 5 deletions(-) diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index f7c65c9e96..23cbde5d9c 100644 --- a/cli/npm/resolvers/mod.rs +++ b/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>, ) -> 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(), diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 1558f58659..9e66bdb662 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -224,8 +224,7 @@ impl ProcState { cli_options.cache_setting(), progress_bar.clone(), ); - let maybe_lockfile = - lockfile.as_ref().filter(|l| !l.lock().overwrite).cloned(); + let maybe_lockfile = lockfile.as_ref().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())); diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index a530d75456..e6990f0e0d 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/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();