Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support lockfileVersion 3 #58

Merged
merged 3 commits into from
Sep 6, 2023
Merged

Support lockfileVersion 3 #58

merged 3 commits into from
Sep 6, 2023

Conversation

mdarocha
Copy link
Contributor

Fixes #55

Info on lockfile: https://docs.npmjs.com/cli/v9/configuring-npm/package-lock-json#lockfileversion.

Lockfile v3 is the same as v2, but contains only the packages field, not dependencies. This required a new path when parsing packages-lock.json. This new logic can also be extended to run on v2 lockfiles, but I decided to limit it to v3 to not accidentally break somebody's project.

@izelnakri
Copy link

izelnakri commented Aug 2, 2023

Hi @jtojnar,

This seems like an important update. I've already encountered this problem when I was developing my npm packages as they were using v3 format. I had to do $ npm install --lockfile-version 2 --package-lock-only, which is not ideal and I'm sure I'm not the only person who is facing this issue as evidenced by this PR.

Can we merge this PR quickly to napalm? This feature could help with the adoption and I'd much rather use napalm than what author suggests here manually: https://www.nmattia.com/posts/2022-12-18-lockfile-trick-package-npm-project-with-nix/

@mdarocha seems to have created good changes that are backwards compatible. Let me know if I can help facilitate merging this any way. Thanks!

@WilliButz
Copy link
Member

I'd like to suggest a small addition to handle a case that currently causes a 500 response from the napalm registry if certain conditions are met by the locked dependencies.

These conditions are for example met by projects that specify isaacs/cliui@8.0.1 as a dependency, which causes the following combination of transitive dependencies to be resolved: https://github.com/npm/cli/blob/latest/package-lock.json#L15575-L15609
The important bit here is that the first resolved dependency is wrap-ansi@8.1.0 and the second is wrap-ansi-cjs@7.0.0 but the latter is aliased to wrap-ansi@7.0.0 and requested as such from the registry by npm.

Using the current state of this PR, napalm parses the package-lock.json without respecting the specified alias (in this case "name": "wrap-ansi"). Instead the dependency's key in napalm's snapshot file is wrap-ansi-cjs with version 7.0.0. This results in a 500 response (to me it's unclear why not 404) when npm requests wrap-ansi@7.0.0 from the napalm registry.

The following patch fixes this and makes a specified name attribute take precedence over the dependency object's name. Feel free to add this as is, or a modified version, to your PR :)

diff --git a/default.nix b/default.nix
index fa420be..7ddac09 100644
--- a/default.nix
+++ b/default.nix
@@ -141,12 +141,18 @@ let
         version = ifNotNull version fallbackPackageVersion;
       } // obj;
 
-      parsePackageNameVersion = name': version': let
+      parsePackageNameVersion = name': originalObj: let
+        version' = originalObj.version;
         # Version can be a pointer like “npm:vue-loader@15.10.0”.
         # In that case we need to replace the name and version with the target one.
         isPointer = lib.hasPrefix "npm:" version';
         fragments = lib.splitString "@" (lib.removePrefix "npm:" version');
-        name = if isPointer then builtins.concatStringsSep "@" (lib.init fragments) else name';
+        name = if builtins.hasAttr "name" originalObj
+               then originalObj.name
+               else
+                 if isPointer
+                 then builtins.concatStringsSep "@" (lib.init fragments)
+                 else name';
         version = if isPointer then lib.last fragments else version';
       in
       { inherit name version; };
@@ -159,7 +165,7 @@ let
         originalName:
         originalObj:
         let
-          inherit (parsePackageNameVersion originalName originalObj.version) name version;
+          inherit (parsePackageNameVersion originalName originalObj) name version;
           obj = originalObj // {
             inherit name version;
           };
@@ -192,7 +198,7 @@ let
           # filter out the top-level package, which has an empty name
           (lib.filterAttrs (name: _: name != ""))
           (lib.mapAttrsToList (originalName: originalObj: let
-            inherit (parsePackageNameVersion (pathToName originalName) originalObj.version) name version;
+            inherit (parsePackageNameVersion (pathToName originalName) originalObj) name version;
             obj = originalObj // {
               inherit name version;
             };

Here is a small reproducer that adds cliui as a dependency to the test added in this PR (including the fix above):

reproducer.patch
diff --git a/default.nix b/default.nix
index fa420be..7ddac09 100644
--- a/default.nix
+++ b/default.nix
@@ -141,12 +141,18 @@ let
         version = ifNotNull version fallbackPackageVersion;
       } // obj;
 
-      parsePackageNameVersion = name': version': let
+      parsePackageNameVersion = name': originalObj: let
+        version' = originalObj.version;
         # Version can be a pointer like “npm:vue-loader@15.10.0”.
         # In that case we need to replace the name and version with the target one.
         isPointer = lib.hasPrefix "npm:" version';
         fragments = lib.splitString "@" (lib.removePrefix "npm:" version');
-        name = if isPointer then builtins.concatStringsSep "@" (lib.init fragments) else name';
+        name = if builtins.hasAttr "name" originalObj
+               then originalObj.name
+               else
+                 if isPointer
+                 then builtins.concatStringsSep "@" (lib.init fragments)
+                 else name';
         version = if isPointer then lib.last fragments else version';
       in
       { inherit name version; };
@@ -159,7 +165,7 @@ let
         originalName:
         originalObj:
         let
-          inherit (parsePackageNameVersion originalName originalObj.version) name version;
+          inherit (parsePackageNameVersion originalName originalObj) name version;
           obj = originalObj // {
             inherit name version;
           };
@@ -192,7 +198,7 @@ let
           # filter out the top-level package, which has an empty name
           (lib.filterAttrs (name: _: name != ""))
           (lib.mapAttrsToList (originalName: originalObj: let
-            inherit (parsePackageNameVersion (pathToName originalName) originalObj.version) name version;
+            inherit (parsePackageNameVersion (pathToName originalName) originalObj) name version;
             obj = originalObj // {
               inherit name version;
             };
diff --git a/test/hello-world-deps-v3/package-lock.json b/test/hello-world-deps-v3/package-lock.json
index 372f4e6..f05d2d7 100644
--- a/test/hello-world-deps-v3/package-lock.json
+++ b/test/hello-world-deps-v3/package-lock.json
@@ -10,12 +10,12 @@
       "license": "ISC",
       "dependencies": {
         "@babel/helper-string-parser": "^7.22.5",
+        "@isaacs/cliui": "^8.0.2",
         "underscore": "^1.9.1"
       },
       "bin": {
         "say-hello": "cli.js"
-      },
-      "devDependencies": {}
+      }
     },
     "node_modules/@babel/helper-string-parser": {
       "version": "7.22.5",
@@ -25,10 +25,254 @@
         "node": ">=6.9.0"
       }
     },
+    "node_modules/@isaacs/cliui": {
+      "version": "8.0.2",
+      "resolved": "https://registry.npmjs.org/@isaacs/cliui/-/cliui-8.0.2.tgz",
+      "integrity": "sha512-O8jcjabXaleOG9DQ0+ARXWZBTfnP4WNAqzuiJK7ll44AmxGKv/J2M4TPjxjY3znBCfvBXFzucm1twdyFybFqEA==",
+      "dependencies": {
+        "string-width": "^5.1.2",
+        "string-width-cjs": "npm:string-width@^4.2.0",
+        "strip-ansi": "^7.0.1",
+        "strip-ansi-cjs": "npm:strip-ansi@^6.0.1",
+        "wrap-ansi": "^8.1.0",
+        "wrap-ansi-cjs": "npm:wrap-ansi@^7.0.0"
+      },
+      "engines": {
+        "node": ">=12"
+      }
+    },
+    "node_modules/ansi-regex": {
+      "version": "6.0.1",
+      "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-6.0.1.tgz",
+      "integrity": "sha512-n5M855fKb2SsfMIiFFoVrABHJC8QtHwVx+mHWP3QcEqBHYienj5dHSgjbxtC0WEZXYt4wcD6zrQElDPhFuZgfA==",
+      "engines": {
+        "node": ">=12"
+      },
+      "funding": {
+        "url": "https://github.com/chalk/ansi-regex?sponsor=1"
+      }
+    },
+    "node_modules/ansi-styles": {
+      "version": "6.2.1",
+      "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-6.2.1.tgz",
+      "integrity": "sha512-bN798gFfQX+viw3R7yrGWRqnrN2oRkEkUjjl4JNn4E8GxxbjtG3FbrEIIY3l8/hrwUwIeCZvi4QuOTP4MErVug==",
+      "engines": {
+        "node": ">=12"
+      },
+      "funding": {
+        "url": "https://github.com/chalk/ansi-styles?sponsor=1"
+      }
+    },
+    "node_modules/color-convert": {
+      "version": "2.0.1",
+      "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-2.0.1.tgz",
+      "integrity": "sha512-RRECPsj7iu/xb5oKYcsFHSppFNnsj/52OVTRKb4zP5onXwVF3zVmmToNcOfGC+CRDpfK/U584fMg38ZHCaElKQ==",
+      "dependencies": {
+        "color-name": "~1.1.4"
+      },
+      "engines": {
+        "node": ">=7.0.0"
+      }
+    },
+    "node_modules/color-name": {
+      "version": "1.1.4",
+      "resolved": "https://registry.npmjs.org/color-name/-/color-name-1.1.4.tgz",
+      "integrity": "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA=="
+    },
+    "node_modules/eastasianwidth": {
+      "version": "0.2.0",
+      "resolved": "https://registry.npmjs.org/eastasianwidth/-/eastasianwidth-0.2.0.tgz",
+      "integrity": "sha512-I88TYZWc9XiYHRQ4/3c5rjjfgkjhLyW2luGIheGERbNQ6OY7yTybanSpDXZa8y7VUP9YmDcYa+eyq4ca7iLqWA=="
+    },
+    "node_modules/emoji-regex": {
+      "version": "9.2.2",
+      "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-9.2.2.tgz",
+      "integrity": "sha512-L18DaJsXSUk2+42pv8mLs5jJT2hqFkFE4j21wOmgbUqsZ2hL72NsUU785g9RXgo3s0ZNgVl42TiHp3ZtOv/Vyg=="
+    },
+    "node_modules/is-fullwidth-code-point": {
+      "version": "3.0.0",
+      "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-3.0.0.tgz",
+      "integrity": "sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg==",
+      "engines": {
+        "node": ">=8"
+      }
+    },
+    "node_modules/string-width": {
+      "version": "5.1.2",
+      "resolved": "https://registry.npmjs.org/string-width/-/string-width-5.1.2.tgz",
+      "integrity": "sha512-HnLOCR3vjcY8beoNLtcjZ5/nxn2afmME6lhrDrebokqMap+XbeW8n9TXpPDOqdGK5qcI3oT0GKTW6wC7EMiVqA==",
+      "dependencies": {
+        "eastasianwidth": "^0.2.0",
+        "emoji-regex": "^9.2.2",
+        "strip-ansi": "^7.0.1"
+      },
+      "engines": {
+        "node": ">=12"
+      },
+      "funding": {
+        "url": "https://github.com/sponsors/sindresorhus"
+      }
+    },
+    "node_modules/string-width-cjs": {
+      "name": "string-width",
+      "version": "4.2.3",
+      "resolved": "https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz",
+      "integrity": "sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==",
+      "dependencies": {
+        "emoji-regex": "^8.0.0",
+        "is-fullwidth-code-point": "^3.0.0",
+        "strip-ansi": "^6.0.1"
+      },
+      "engines": {
+        "node": ">=8"
+      }
+    },
+    "node_modules/string-width-cjs/node_modules/ansi-regex": {
+      "version": "5.0.1",
+      "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz",
+      "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==",
+      "engines": {
+        "node": ">=8"
+      }
+    },
+    "node_modules/string-width-cjs/node_modules/emoji-regex": {
+      "version": "8.0.0",
+      "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz",
+      "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A=="
+    },
+    "node_modules/string-width-cjs/node_modules/strip-ansi": {
+      "version": "6.0.1",
+      "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz",
+      "integrity": "sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==",
+      "dependencies": {
+        "ansi-regex": "^5.0.1"
+      },
+      "engines": {
+        "node": ">=8"
+      }
+    },
+    "node_modules/strip-ansi": {
+      "version": "7.1.0",
+      "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-7.1.0.tgz",
+      "integrity": "sha512-iq6eVVI64nQQTRYq2KtEg2d2uU7LElhTJwsH4YzIHZshxlgZms/wIc4VoDQTlG/IvVIrBKG06CrZnp0qv7hkcQ==",
+      "dependencies": {
+        "ansi-regex": "^6.0.1"
+      },
+      "engines": {
+        "node": ">=12"
+      },
+      "funding": {
+        "url": "https://github.com/chalk/strip-ansi?sponsor=1"
+      }
+    },
+    "node_modules/strip-ansi-cjs": {
+      "name": "strip-ansi",
+      "version": "6.0.1",
+      "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz",
+      "integrity": "sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==",
+      "dependencies": {
+        "ansi-regex": "^5.0.1"
+      },
+      "engines": {
+        "node": ">=8"
+      }
+    },
+    "node_modules/strip-ansi-cjs/node_modules/ansi-regex": {
+      "version": "5.0.1",
+      "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz",
+      "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==",
+      "engines": {
+        "node": ">=8"
+      }
+    },
     "node_modules/underscore": {
       "version": "1.9.1",
       "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.9.1.tgz",
       "integrity": "sha512-5/4etnCkd9c8gwgowi5/om/mYO5ajCaOgdzj/oW+0eQV9WxKBDZw5+ycmKmeaTXjInS/W0BzpGLo2xR2aBwZdg=="
+    },
+    "node_modules/wrap-ansi": {
+      "version": "8.1.0",
+      "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-8.1.0.tgz",
+      "integrity": "sha512-si7QWI6zUMq56bESFvagtmzMdGOtoxfR+Sez11Mobfc7tm+VkUckk9bW2UeffTGVUbOksxmSw0AA2gs8g71NCQ==",
+      "dependencies": {
+        "ansi-styles": "^6.1.0",
+        "string-width": "^5.0.1",
+        "strip-ansi": "^7.0.1"
+      },
+      "engines": {
+        "node": ">=12"
+      },
+      "funding": {
+        "url": "https://github.com/chalk/wrap-ansi?sponsor=1"
+      }
+    },
+    "node_modules/wrap-ansi-cjs": {
+      "name": "wrap-ansi",
+      "version": "7.0.0",
+      "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-7.0.0.tgz",
+      "integrity": "sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==",
+      "dependencies": {
+        "ansi-styles": "^4.0.0",
+        "string-width": "^4.1.0",
+        "strip-ansi": "^6.0.0"
+      },
+      "engines": {
+        "node": ">=10"
+      },
+      "funding": {
+        "url": "https://github.com/chalk/wrap-ansi?sponsor=1"
+      }
+    },
+    "node_modules/wrap-ansi-cjs/node_modules/ansi-regex": {
+      "version": "5.0.1",
+      "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz",
+      "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==",
+      "engines": {
+        "node": ">=8"
+      }
+    },
+    "node_modules/wrap-ansi-cjs/node_modules/ansi-styles": {
+      "version": "4.3.0",
+      "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-4.3.0.tgz",
+      "integrity": "sha512-zbB9rCJAT1rbjiVDb2hqKFHNYLxgtk8NURxZ3IZwD3F6NtxbXZQCnnSi1Lkx+IDohdPlFp222wVALIheZJQSEg==",
+      "dependencies": {
+        "color-convert": "^2.0.1"
+      },
+      "engines": {
+        "node": ">=8"
+      },
+      "funding": {
+        "url": "https://github.com/chalk/ansi-styles?sponsor=1"
+      }
+    },
+    "node_modules/wrap-ansi-cjs/node_modules/emoji-regex": {
+      "version": "8.0.0",
+      "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz",
+      "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A=="
+    },
+    "node_modules/wrap-ansi-cjs/node_modules/string-width": {
+      "version": "4.2.3",
+      "resolved": "https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz",
+      "integrity": "sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==",
+      "dependencies": {
+        "emoji-regex": "^8.0.0",
+        "is-fullwidth-code-point": "^3.0.0",
+        "strip-ansi": "^6.0.1"
+      },
+      "engines": {
+        "node": ">=8"
+      }
+    },
+    "node_modules/wrap-ansi-cjs/node_modules/strip-ansi": {
+      "version": "6.0.1",
+      "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz",
+      "integrity": "sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==",
+      "dependencies": {
+        "ansi-regex": "^5.0.1"
+      },
+      "engines": {
+        "node": ">=8"
+      }
     }
   }
 }
diff --git a/test/hello-world-deps-v3/package.json b/test/hello-world-deps-v3/package.json
index 2802419..f961490 100644
--- a/test/hello-world-deps-v3/package.json
+++ b/test/hello-world-deps-v3/package.json
@@ -12,6 +12,7 @@
   },
   "dependencies": {
     "@babel/helper-string-parser": "^7.22.5",
+    "@isaacs/cliui": "^8.0.2",
     "underscore": "^1.9.1"
   },
   "description": ""

@jtojnar
Copy link
Contributor

jtojnar commented Sep 4, 2023

Sorry, I can no longer find time for napalm so I am stepping down as maintainer.

@mdarocha
Copy link
Contributor Author

mdarocha commented Sep 5, 2023

Sorry, I can no longer find time for napalm so I am stepping down as maintainer.

Well, if you're looking for replacements, I could take the mantle. I wouldn't probably introduce many new changes and mostly focus on bug fixing and keeping up with npm compatibility.

@Mic92
Copy link
Member

Mic92 commented Sep 5, 2023

I invited @WilliButz to be able to merge in this repository to have someone again that can merge in this project. I can add @mdarocha certainly as well in the future. It's just at the moment I don't know this user well, so I would like to either have someone vouch for them or I see some more activity (currently the contribution graph is private, so I can not really judge based on that).

@mdarocha
Copy link
Contributor Author

mdarocha commented Sep 5, 2023

@Mic92 right, I updated my profile, you can check it now

@mdarocha
Copy link
Contributor Author

mdarocha commented Sep 5, 2023

@WilliButz I incorporated your changes. Thanks!

@WilliButz WilliButz merged commit a8215cc into nix-community:master Sep 6, 2023
@mdarocha mdarocha deleted the lockfile-v3 branch September 6, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails on lockfileVersion: 3
5 participants