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

[BUG] cache add tarball file does not add to cache #2160

Closed
mjsir911 opened this issue Nov 12, 2020 · 2 comments
Closed

[BUG] cache add tarball file does not add to cache #2160

mjsir911 opened this issue Nov 12, 2020 · 2 comments
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@mjsir911
Copy link
Contributor

Current Behavior:

$ curl -O https://registry.npmjs.org/yn/-/yn-3.1.1.tgz
$ npm cache add --offline yn-3.1.1.tgz --cache=./npm-cache
$ find npm-cache
find: 'npm-cache': No such file or directory
$ npm cache add yn-3.1.1.tgz --cache=./npm-cache
$ find npm-cache
npm-cache/
npm-cache/_cacache
npm-cache/_cacache/tmp
npm-cache/_cacache/index-v5
npm-cache/_cacache/index-v5/af
npm-cache/_cacache/index-v5/af/03
npm-cache/_cacache/index-v5/af/03/5c781820370e585dc2323edbbc80669bf714da5b47d56510c7d0bd7521ee
npm-cache/_cacache/content-v2
npm-cache/_cacache/content-v2/sha512
npm-cache/_cacache/content-v2/sha512/2a
npm-cache/_cacache/content-v2/sha512/2a/c1
npm-cache/_cacache/content-v2/sha512/2a/c1/5ea26b368794fd868d7075435587203f917c6e794d579cb62cf5912dfebde810dd3a8ff59fd3f8f4bba37794b88e1de0675aae047e5298de152eb95cb8ec
npm-cache/_update-notifier-last-checked

The only index file looks like this:

ce4ce0818a4010abd13955ea944894bb892f9dcb	{"key":"make-fetch-happen:request-cache:https://registry.npmjs.org/npm","integrity":"sha512-KsFeoms2h5T9ho1wdUNVhyA/kXxueU1XnLYs9ZEt/r3oEN06j/Wf0/j0u6N3lLiOHeBnWq4EflKY3hUuuVy47A==","time":1605141536980,"size":1371364,"metadata":{"url":"https://registry.npmjs.org/npm","reqHeaders":{"npm-in-ci":["false"],"user-agent":["npm/7.0.8 node/v15.1.0 linux x64"],"pacote-version":["11.1.12"],"pacote-req-type":["packument"],"pacote-pkg-id":["registry:npm"],"accept":["application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*"],"npm-session":["5bd2cd38552c8905"],"connection":["keep-alive"]},"resHeaders":{"date":["Thu, 12 Nov 2020 00:38:56 GMT"],"content-type":["application/vnd.npm.install-v1+json"],"content-length":["1371364"],"connection":["keep-alive"],"set-cookie":["__cfduid=d22e0d5ed9c96f0aeb42e02af40d9f0c91605141536; expires=Sat, 12-Dec-20 00:38:56 GMT; path=/; domain=.npmjs.org; HttpOnly; SameSite=Lax"],"cf-ray":["5f0c3168d920e382-SEA"],"accept-ranges":["bytes"],"age":["2198"],"cache-control":["public, max-age=300"],"etag":["\"a2d3dcb92f0c81d9f15bf0ddba9ce8af\""],"last-modified":["Tue, 10 Nov 2020 19:40:29 GMT"],"vary":["accept-encoding, accept"],"cf-cache-status":["HIT"],"cf-request-id":["065b7d35820000e3825a8dd000000001"],"expect-ct":["max-age=604800, report-uri=\"https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct\""],"server":["cloudflare"],"x-fetch-attempts":["1"]}}}
e232fec39cc6401905a676502561f98cef90ff0a	{"key":"make-fetch-happen:request-cache:https://registry.npmjs.org/npm","integrity":"sha512-KsFeoms2h5T9ho1wdUNVhyA/kXxueU1XnLYs9ZEt/r3oEN06j/Wf0/j0u6N3lLiOHeBnWq4EflKY3hUuuVy47A==","time":1605141537040,"size":1371364,"metadata":{"url":"https://registry.npmjs.org/npm","reqHeaders":{"npm-in-ci":["false"],"user-agent":["npm/7.0.8 node/v15.1.0 linux x64"],"pacote-version":["11.1.12"],"pacote-req-type":["packument"],"pacote-pkg-id":["registry:npm"],"accept":["application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*"],"npm-session":["5bd2cd38552c8905"]},"resHeaders":{"date":["Thu, 12 Nov 2020 00:38:57 GMT"],"connection":["keep-alive"],"set-cookie":["__cfduid=da893011c4928ad38d627f4b0792365fd1605141537; expires=Sat, 12-Dec-20 00:38:57 GMT; path=/; domain=.npmjs.org; HttpOnly; SameSite=Lax"],"cf-ray":["5f0c316e5b7be382-SEA"],"age":["2199"],"cache-control":["public, max-age=300"],"etag":["\"a2d3dcb92f0c81d9f15bf0ddba9ce8af\""],"last-modified":["Tue, 10 Nov 2020 19:40:29 GMT"],"vary":["Accept-Encoding"],"cf-cache-status":["HIT"],"cf-request-id":["065b7d38f50000e38260a85000000001"],"expect-ct":["max-age=604800, report-uri=\"https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct\""],"server":["cloudflare"],"x-fetch-attempts":["1"],"X-Local-Cache":["%2Fnpm-cache%2F_cacache"],"X-Local-Cache-Key":["make-fetch-happen%3Arequest-cache%3Ahttps%3A%2F%2Fregistry.npmjs.org%2Fnpm"],"X-Local-Cache-Hash":["sha512-KsFeoms2h5T9ho1wdUNVhyA%2FkXxueU1XnLYs9ZEt%2Fr3oEN06j%2FWf0%2Fj0u6N3lLiOHeBnWq4EflKY3hUuuVy47A%3D%3D"],"X-Local-Cache-Time":["Thu, 12 Nov 2020 00:38:56 GMT"]}}}

Which explains why it doesn't show up with --offline, but there are no pacote tarball files cached

Expected Behavior:

npm cache add should cache the tarball passed to it, resulting in index files that look like this, even in offline mode (this file was created with npm@6.14.5:

765e28e0759c5341235a3f401e857d517605ac53	{"key":"pacote:tarball:file:/path/to/yn-3.1.1.tgz","integrity":"sha512-Ux4ygGWsu2c7isFWe8Yu1YluJmqVhxqK2cLXNQA5AcC3QfbGNpM7fu0Y8b/z16pXLnFxZYvWhd3fhBY9DLmC6Q==","time":1605141111769,"size":2729}

Of note, npm cache add https://registry.npmjs.org/yn/-/yn-3.1.1.tgz sort of works properly, resulting in an additional index file containing this:

d1cc105d0e42aeee7f15ece24d2f06c58408188c	{"key":"make-fetch-happen:request-cache:https://registry.npmjs.org/yn/-/yn-3.1.1.tgz","integrity":"sha512-Ux4ygGWsu2c7isFWe8Yu1YluJmqVhxqK2cLXNQA5AcC3QfbGNpM7fu0Y8b/z16pXLnFxZYvWhd3fhBY9DLmC6Q==","time":1605141784682,"size":2729,"metadata":{"url":"https://registry.npmjs.org/yn/-/yn-3.1.1.tgz","reqHeaders":{"npm-in-ci":["false"],"user-agent":["npm/7.0.8 node/v15.1.0 linux x64"],"pacote-version":["11.1.12"],"pacote-req-type":["tarball"],"pacote-pkg-id":["remote:https://registry.npmjs.org/yn/-/yn-3.1.1.tgz"],"npm-session":["5bf563259a0edbeb"],"npm-command":["cache"],"connection":["keep-alive"]},"resHeaders":{"date":["Thu, 12 Nov 2020 00:43:04 GMT"],"content-type":["application/octet-stream"],"content-length":["2729"],"connection":["keep-alive"],"set-cookie":["__cfduid=d143f2ea667ec91bfa68efc06341911f41605141784; expires=Sat, 12-Dec-20 00:43:04 GMT; path=/; domain=.npmjs.org; HttpOnly; SameSite=Lax"],"cf-ray":["5f0c377a1f3dc991-SEA"],"accept-ranges":["bytes"],"age":["579188"],"cache-control":["public, immutable, max-age=31557600"],"etag":["\"35f607b3a0defabff7d13fd12ae05ab7\""],"last-modified":["Tue, 30 Jul 2019 18:20:09 GMT"],"vary":["Accept-Encoding"],"cf-cache-status":["HIT"],"cf-request-id":["065b81004b0000c99192a78000000001"],"expect-ct":["max-age=604800, report-uri=\"https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct\""],"server":["cloudflare"],"x-fetch-attempts":["1"]}}}

Environment:

$ docker run -it node:15.1.0 bash
# node --version
v15.1.0
# npm --version
7.0.8
@mjsir911 mjsir911 added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Nov 12, 2020
@mjsir911 mjsir911 changed the title [BUG] cache add does not add to cache [BUG] cache add tarball file does not add to cache Nov 12, 2020
@ljharb
Copy link
Contributor

ljharb commented Nov 12, 2020

Does this still happen on npm v7.0.10?

@mjsir911
Copy link
Contributor Author

Can reproduce with npm@7.0.10, node v15.1.0

@darcyclarke darcyclarke added the Enhancement new feature or improvement label Jan 22, 2021
@darcyclarke darcyclarke added Priority 2 secondary priority issue and removed Needs Triage needs review for next steps Enhancement new feature or improvement labels Feb 2, 2021
mjsir911 added a commit to mjsir911/cli that referenced this issue Mar 27, 2021
Pacote doesn't do this automatically anymore

Closes npm/pacote#73 & Closes npm#2160
mjsir911 added a commit to mjsir911/cli that referenced this issue Mar 27, 2021
Pacote doesn't do this automatically anymore

Closes npm/pacote#73 & Closes npm#2160
mjsir911 added a commit to mjsir911/cli that referenced this issue Mar 27, 2021
Pacote doesn't do this automatically anymore

Closes npm/pacote#73 & Closes npm#2160
mjsir911 added a commit to mjsir911/cli that referenced this issue Mar 27, 2021
Pacote doesn't do this automatically anymore

Closes npm/pacote#73 & Closes npm#2160
mjsir911 added a commit to mjsir911/cli that referenced this issue Mar 28, 2021
Pacote doesn't do this automatically anymore

Closes npm/pacote#73 & Closes npm#2160
mjsir911 added a commit to mjsir911/cli that referenced this issue Apr 1, 2021
Pacote doesn't do this automatically anymore

Closes npm/pacote#73 & Closes npm#2160
mjsir911 added a commit to mjsir911/cli that referenced this issue Apr 1, 2021
Pacote doesn't do this automatically anymore

Closes npm/pacote#73 & Closes npm#2160
mjsir911 added a commit to mjsir911/pacote that referenced this issue Apr 1, 2021
Tell `fetch` not to cache it's downloaded tarballs, leave that to us

Not sure the implications of changing the return type of _istream /
adding a `cacache` to the end of the pipe, too unfamiliar with streams

Fixes npm#73 & Fixes npm/cli#2160

Also see npm/cli#2976 for my first attempt & some discussions as to how
we got here.
isaacs pushed a commit to npm/pacote that referenced this issue Apr 13, 2021
Tell `fetch` not to cache it's downloaded tarballs, leave that to us

Also see npm/cli#2976 for my first attempt & some discussions as to how
we got here.

Fix: #73
Fix: npm/cli#2160
Close: npm/cli#2976

PR-URL: #74
Credit: @mjsir911
Close: #74
Reviewed-by: @isaacs

EDIT(@isaacs): Updated to add test, allow make-fetch-happen to use the
cache, pipe into cache properly, and avoid cache double-writes.
isaacs pushed a commit to npm/pacote that referenced this issue Apr 13, 2021
Tell `fetch` not to cache it's downloaded tarballs, leave that to us

Also see npm/cli#2976 for my first attempt & some discussions as to how
we got here.

Fix: #73
Fix: npm/cli#2160
Close: npm/cli#2976

PR-URL: #74
Credit: @mjsir911
Close: #74
Reviewed-by: @isaacs

EDIT(@isaacs): Updated to add test, allow make-fetch-happen to use the
cache, pipe into cache properly, and avoid cache double-writes.
isaacs pushed a commit to npm/pacote that referenced this issue Apr 15, 2021
Tell `fetch` not to cache it's downloaded tarballs, leave that to us

Also see npm/cli#2976 for my first attempt & some discussions as to how
we got here.

Fix: #73
Fix: npm/cli#2160
Close: npm/cli#2976

PR-URL: #74
Credit: @mjsir911
Close: #74
Reviewed-by: @isaacs

EDIT(@isaacs): Updated to add test, allow make-fetch-happen to use the
cache, pipe into cache properly, and avoid cache double-writes.
isaacs pushed a commit to npm/pacote that referenced this issue Apr 15, 2021
Tell `fetch` not to cache it's downloaded tarballs, leave that to us

Also see npm/cli#2976 for my first attempt & some discussions as to how
we got here.

Fix: #73
Fix: npm/cli#2160
Close: npm/cli#2976

PR-URL: #74
Credit: @mjsir911
Close: #74
Reviewed-by: @isaacs

EDIT(@isaacs): Updated to add test, allow make-fetch-happen to use the
cache, pipe into cache properly, and avoid cache double-writes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
3 participants