npm / cli

the package manager for JavaScript
https://docs.npmjs.com/cli/
Other
8.36k stars 3.09k forks source link

[BUG] `npm pack <git-dependency>#<commit-hash>` fails in npm >= 9.6.5 #6723

Open chmeliik opened 1 year ago

chmeliik commented 1 year ago

Is there an existing issue for this?

This issue exists in the latest npm version

Current Behavior

npm pack with a specific commit of a git dependency fails as follows:

$ npm pack vercel/ms#1304f150b38027e0818cc122106b5c7322d68d0c                                   
npm ERR! GitFetcher requires an Arborist constructor to pack a tarball

npm ERR! A complete log of this run can be found in: /home/acmiel/.npm/_logs/2023-08-21T08_27_53_861Z-debug-0.log

Expected Behavior

Expected npm pack to succeed (like it does in version <= 9.6.4).

Note that it still works for things that are not commit hashes:

$ npm pack vercel/ms#3.0.0-canary.1
...
ms-3.0.0-canary.1.tgz

Steps To Reproduce

  1. With npm version 9.6.5 or higher
  2. Run npm pack vercel/ms#1304f150b38027e0818cc122106b5c7322d68d0c or any other git dependency + commit hash
  3. And get npm ERR! GitFetcher requires an Arborist constructor to pack a tarball

Environment

; node bin location = /usr/bin/node-18
; node version = v18.17.1
; npm local prefix = /tmp/test
; npm version = 9.8.1
; cwd = /tmp/test
; HOME = /home/acmiel
; Run `npm config ls -l` to show all defaults.
chmeliik commented 1 year ago

So, I think I've found the problem.

npm pack calls pacote.manifest() without passing in an Arborist (why would it, it's just getting a package.json)

https://github.com/npm/cli/blob/6ec6ff0660f774f852cd14a413f538ffbfaf3cc9/lib/commands/pack.js#L37

For a git dependency which is "hosted" and resolved, GitFetcher.manifest uses FileFetcher.manifest instead

https://github.com/npm/cli/blob/6ec6ff0660f774f852cd14a413f538ffbfaf3cc9/node_modules/pacote/lib/git.js#L312-L313

FileFetcher.manifest calls extract()

https://github.com/npm/cli/blob/6ec6ff0660f774f852cd14a413f538ffbfaf3cc9/node_modules/pacote/lib/file.js#L27-L28

extract calls _tarballFromResolved

https://github.com/npm/cli/blob/6ec6ff0660f774f852cd14a413f538ffbfaf3cc9/node_modules/pacote/lib/fetcher.js#L331

And finally, GitFetcher's _tarballFromResolved does a full clone, prepare and would go on to do pretty much an entire pack operation if not for the missing Arborist error

https://github.com/npm/cli/blob/6ec6ff0660f774f852cd14a413f538ffbfaf3cc9/node_modules/pacote/lib/git.js#L209-L213

chmeliik commented 1 year ago

I think the fix is to remove the FileFetcher.manifest usage and always just clone

       return Promise.resolve(this.package)
     }

-    return this.spec.hosted && this.resolved
-      ? FileFetcher.prototype.manifest.apply(this)
-      : this[_clone](dir =>
+    return this[_clone](dir =>
         this[_readPackageJson](dir + '/package.json')
           .then(mani => this.package = {
             ...mani,

Would anyone happen to know why FileFetcher.manifest was used in the first place?

itavero commented 4 months ago

I'm getting the same error when trying to use npm exec / npx with a commit hash on npm 10.7.0. Should this already have been fixed (as I do see some referenced issues thath ave been closed)?

milaninfy commented 3 weeks ago

evidently it works for short commit hash format and fails for long one.

~/workarea/npm-cli $ npx npm/cli#ec105f400281a5bfd17885de1ea3d54d0c231b27 -v
npm error GitFetcher requires an Arborist constructor to pack a tarball
npm error A complete log of this run can be found in: /Users/milaninfy/.npm/_logs/2024-08-30T17_55_32_028Z-debug-0.log

~/workarea/npm-cli $ npx npm/cli#ec105f4 -v                                 
Need to install the following packages:
github:npm/cli#ec105f4
Ok to proceed? (y) y

10.8.3