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

npm pack crash in Node 23 #55410

Closed
Tracked by #6
RobinTail opened this issue Oct 16, 2024 · 16 comments · Fixed by #55414
Closed
Tracked by #6

npm pack crash in Node 23 #55410

RobinTail opened this issue Oct 16, 2024 · 16 comments · Fixed by #55414
Labels
confirmed-bug Issues with confirmed bugs. npm Issues and PRs related to the npm client dependency or the npm registry. regression Issues related to regressions. v23.x v23.x Issues that can be reproduced on v23.x or PRs targeting the v23.x-staging branch.

Comments

@RobinTail
Copy link

RobinTail commented Oct 16, 2024

Version

v23.0.0

Platform

Darwin ....local 23.6.0 Darwin Kernel Version 23.6.0: Wed Jul 31 20:48:44 PDT 2024; root:xnu-10063.141.1.700.5~1/RELEASE_X86_64 x86_64

same in CI

Ubuntu
22.04.5
LTS

Subsystem

No response

What steps will reproduce the bug?

nvm install 23
corepack enable
npm pack

How often does it reproduce? Is there a required condition?

it's only happening on Node 23.0.0.
No such issues in Node 18, Node 20 and Node 22.

What is the expected behavior? Why is that the expected behavior?

it should pack

What do you see instead?

(node:9110) ExperimentalWarning: Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
npm warn tarball tarball data for file:/.../ (null) seems to be corrupted. Trying again.
npm warn tarball tarball data for file:/.../ (null) seems to be corrupted. Trying again.
npm error code ENOENT
npm error syscall lstat
npm error path /Users/.../ist/index.cjs
npm error errno -2
npm error enoent ENOENT: no such file or directory, lstat '/.../ist/index.cjs'
npm error enoent This is related to npm not being able to find a file.
npm error enoent
npm error A complete log of this run can be found in: /Users/.../.npm/_logs/2024-10-16T18_41_11_606Z-debug-0.log

⚠️ what's interesting is that ist is actually dist — it's loosing the first letter of the directory name

Additional information

from package.json:

  "files": [
    "dist",
    "migration",
    "*.md"
  ],

from .log file:

0 verbose cli /Users/.../.nvm/versions/node/v23.0.0/bin/node /Users/.../.nvm/versions/node/v23.0.0/bin/npm
1 info using npm@10.9.0
2 info using node@v23.0.0
3 silly config load:file:/Users/.../.nvm/versions/node/v23.0.0/lib/node_modules/npm/npmrc
4 silly config load:file:/Users/.../.npmrc
5 silly config load:file:/Users/.../.npmrc
6 silly config load:file:/Users/.../.nvm/versions/node/v23.0.0/etc/npmrc
7 verbose title npm pack
8 verbose argv "pack"
9 verbose logfile logs-max:10 dir:/Users/.../.npm/_logs/2024-10-16T18_41_11_606Z-
10 verbose logfile /Users/.../.npm/_logs/2024-10-16T18_41_11_606Z-debug-0.log
11 silly logfile start cleaning logs, removing 1 files
12 silly logfile done cleaning log files
13 silly packumentCache heap:4345298944 maxSize:1086324736 maxEntrySize:543162368
14 warn tarball tarball data for file:/Users/.../ (null) seems to be corrupted. Trying again.
15 warn tarball tarball data for file:/Users/.../ (null) seems to be corrupted. Trying again.
16 verbose stack Error: ENOENT: no such file or directory, lstat '/Users/.../ist/index.cjs'
17 error code ENOENT
18 error syscall lstat
19 error path /Users/.../ist/index.cjs
20 error errno -2
21 error enoent ENOENT: no such file or directory, lstat '/Users/.../ist/index.cjs'
22 error enoent This is related to npm not being able to find a file.
22 error enoent
23 verbose cwd /Users/...
24 verbose os Darwin 23.6.0
25 verbose node v23.0.0
26 verbose npm  v10.9.0
27 verbose exit -2
28 verbose code -2
29 error A complete log of this run can be found in: /Users/.../.npm/_logs/2024-10-16T18_41_11_606Z-debug-0.log
@avivkeller
Copy link
Member

avivkeller commented Oct 16, 2024

I can't reproduce. Does the file that it's looking for exist?

$ node -v 
v23.0.0
                                                                                                                                                                                                                                
$ cat package.json
{
    "name": "package",
    "version": "0.0.0"
}                                                                                                                                                                                                                                
$ npm pack
(node:112161) ExperimentalWarning: Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
npm notice
npm notice 📦  package@0.0.0
npm notice Tarball Contents
npm notice 49B package.json
npm notice Tarball Details
npm notice name: package
npm notice version: 0.0.0
npm notice filename: package-0.0.0.tgz
npm notice package size: 140 B
npm notice unpacked size: 49 B
npm notice shasum: 9b86ea669576bef9600e739c770b2e5635f75482
npm notice integrity: sha512-sKnSxYIx7NX5u[...]PXtisbzPyqMIA==
npm notice total files: 1
npm notice
package-0.0.0.tgz

@RobinTail
Copy link
Author

RobinTail commented Oct 16, 2024

I can't reproduce. Does the file that it's looking for exist?

It's looking for /ist/index.cjs while it's dist/index.cjs, @redyetidev
The first letter is lost for some reason

@avivkeller avivkeller added npm Issues and PRs related to the npm client dependency or the npm registry. v23.x v23.x Issues that can be reproduced on v23.x or PRs targeting the v23.x-staging branch. labels Oct 16, 2024
@avivkeller
Copy link
Member

avivkeller commented Oct 16, 2024

Why is it looking for that file? Can you provide your package.json file and directory structure?

@avivkeller avivkeller added repro-exists confirmed-bug Issues with confirmed bugs. and removed repro-exists labels Oct 16, 2024
@avivkeller
Copy link
Member

avivkeller commented Oct 16, 2024

I've been able to reproduce now:

$ node -v 
v23.0.0
                                                                                                                                                                                                                                
$ cat package.json
{
    "name": "package",
    "version": "0.0.0"
}

$ tree                                                                   
.
├── dist
│   └── index.cjs
└── package.json

$ npm pack
(node:114615) ExperimentalWarning: Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
npm warn tarball tarball data for file:/path/ (null) seems to be corrupted. Trying again.
npm warn tarball tarball data for file:/path/ (null) seems to be corrupted. Trying again.
npm error code ENOENT
npm error syscall lstat
npm error path /path/ist/index.cjs
npm error errno -2
npm error enoent ENOENT: no such file or directory, lstat '/path/ist/index.cjs'
npm error enoent This is related to npm not being able to find a file.
npm error enoent
npm error A complete log of this run can be found in: /home/user/.npm/_logs/2024-10-16T18_55_56_185Z-debug-0.log

@avivkeller
Copy link
Member

avivkeller commented Oct 16, 2024

Looks like a Node.js side issue. I downgraded my npm to v10.8.3 (the version w/ Node.js v22.9.0) and the error still occured. I then downgraded my Node.js to v22.9.0, and kept npm at v10.9.0, and the error no longer occured.

CC @nodejs/npm

@avivkeller avivkeller added the regression Issues related to regressions. label Oct 16, 2024
@RobinTail
Copy link
Author

Why is it looking for that file? Can you provide your package.json file and directory structure?

I don't know why, but it does not in any other Node.js version.

package.json is here:
https://github.com/RobinTail/express-zod-api/blob/test-node23/package.json

directory structure, @redyetidev

$ tree -d -L 1
.
├── dist
├── example
├── migration
├── node_modules
├── src
├── tests
└── tools

@avivkeller
Copy link
Member

avivkeller commented Oct 16, 2024

Here's the stacktrace

READ /.../ist/index.cjs
    at Object.lstat (node:fs:1549:15)
    at Object.lstat (/node/deps/npm/node_modules/graceful-fs/polyfills.js:309:16)
    at [stat] (/node/deps/npm/node_modules/tar/lib/pack.js:181:13)
    at [processJob] (/node/deps/npm/node_modules/tar/lib/pack.js:278:19)
    at [process] (/node/deps/npm/node_modules/tar/lib/pack.js:232:23)
    at [addFSEntry] (/node/deps/npm/node_modules/tar/lib/pack.js:174:18)
    at Pack.write (/node/deps/npm/node_modules/tar/lib/pack.js:150:23)
    at Pack.add (/node/deps/npm/node_modules/tar/lib/pack.js:129:10)
    at addFilesAsync (/node/deps/npm/node_modules/tar/lib/create.js:95:9)
    at create (/node/deps/npm/node_modules/tar/lib/create.js:109:3)
    at module.exports (/node/deps/npm/node_modules/tar/lib/create.js:39:7)
    at /node/deps/npm/node_modules/pacote/lib/dir.js:76:26

Intrestingly, a few calls before this is:

READ /.../dist/index.cjs
    at Object.lstat (node:fs:1549:15)
    at Object.lstat (/node/deps/npm/node_modules/graceful-fs/polyfills.js:309:16)
    at PackWalker.stat (/node/deps/npm/node_modules/ignore-walk/lib/index.js:177:8)
    at PackWalker.stat (/node/deps/npm/node_modules/npm-packlist/lib/index.js:192:18)
    at /node/deps/npm/node_modules/ignore-walk/lib/index.js:153:14
    at Array.forEach (<anonymous>)
    at PackWalker.filterEntries (/node/deps/npm/node_modules/ignore-walk/lib/index.js:149:16)
    at PackWalker.filterEntries (/node/deps/npm/node_modules/npm-packlist/lib/index.js:173:18)
    at PackWalker.onReaddir (/node/deps/npm/node_modules/ignore-walk/lib/index.js:78:14)
    at /node/deps/npm/node_modules/ignore-walk/lib/index.js:54:42
    at /node/deps/npm/node_modules/graceful-fs/graceful-fs.js:228:16
    at FSReqCallback.oncomplete (node:fs:188:23)

Update

The failing call is the call to libpack from libnpmpack in

https://github.com/npm/cli/blob/62c71e5128a01283f97bd62da30ddc673bddda0b/lib/commands/pack.js#L49

More specifically, the call to tarball in https://github.com/npm/cli/blob/62c71e5128a01283f97bd62da30ddc673bddda0b/workspaces/libnpmpack/lib/index.js#L31

@jasonwashburn
Copy link

Same/Similar issue here that just started occuring today when trying to install renovatebot's pre-commit hook (https://github.com/renovatebot/pre-commit-hooks) which has been working for me up until today.
image

Worth noting, similar to the ist vs dist mentioned earlier in the thread, if you look closely at this error message, it's looking for a path that includes ode_env-default rather than node_env-default

@avivkeller
Copy link
Member

avivkeller commented Oct 16, 2024

This is a reminder that "me too" comments only add noise. For everyone considering commenting, please only comment if you have something meaningful to add to the conversation.

If you're experiencing this issue, please 👍 the issue or a comment you agree with. There is no need to comment if you are also affected by the this.

@ovflowd
Copy link
Member

ovflowd commented Oct 16, 2024

Self-note: We should add pack and other common commands to the test suite of https://github.com/nodejs/citgm/tree/main/test/npm (PRs welcome)

@avivkeller
Copy link
Member

avivkeller commented Oct 16, 2024

I've made a minimal reproduction:

const walk = require('ignore-walk');
const { resolve, join } = require('path')
const packageDir = resolve('../testing') + '/';

walk({
    path: join(packageDir, 'dist'),
    parent: {
        root: packageDir,
        result: new Set([]),
    },
}, console.log)

@avivkeller
Copy link
Member

This seems to be an issue with some logic in ignore-walk. I'll continue investigating the root cause.

@richardlau
Copy link
Member

git bisect is pointing to efbba60 / #54224

$ git bisect log
git bisect start
# status: waiting for both good and bad commits
# bad: [e2242b4e256082e01d8df6a4155ddc80e146c123] 2024-10-16, Version 22.10.0 (Current)
git bisect bad e2242b4e256082e01d8df6a4155ddc80e146c123
# status: waiting for good commit(s), bad commit known
# bad: [20d8b85d3493bec944de541a896e0165dd356345] deps: upgrade npm to 10.9.0
git bisect bad 20d8b85d3493bec944de541a896e0165dd356345
# status: waiting for good commit(s), bad commit known
# bad: [cde6dccb656ad8ad134c2e509c584711156a908c] tools: refactor js2c.cc to use c++20
git bisect bad cde6dccb656ad8ad134c2e509c584711156a908c
# status: waiting for good commit(s), bad commit known
# good: [d2479fa020866f038785fb7373baf2621d7cc1c5] vm: return all own names and symbols in property enumerator interceptor
git bisect good d2479fa020866f038785fb7373baf2621d7cc1c5
# good: [7c58645aca2b53d15dbf698efa43cf934a436d91] lib: move `Symbol[Async]Dispose` polyfills to `internal/util`
git bisect good 7c58645aca2b53d15dbf698efa43cf934a436d91
# bad: [e4692eee75af720640beb557d9279adc0f3e648d] benchmark: --no-warnings to avoid DEP/ExpWarn log
git bisect bad e4692eee75af720640beb557d9279adc0f3e648d
# good: [d38dc99a95fd9dfd634bb938a53871719115fb50] src: add Cleanable class to Environment
git bisect good d38dc99a95fd9dfd634bb938a53871719115fb50
# good: [415905712e59c0d8d7270de27a3b789a121968cd] test: improve test-internal-fs-syncwritestream
git bisect good 415905712e59c0d8d7270de27a3b789a121968cd
# bad: [a65105ec284023960e93b3a66f6661ddd2f4121f] test: adjust tls test for OpenSSL32
git bisect bad a65105ec284023960e93b3a66f6661ddd2f4121f
# good: [f468509cf6f6ebc7d753e8f30686fe0c6b609dc5] zlib: add typings for better dx
git bisect good f468509cf6f6ebc7d753e8f30686fe0c6b609dc5
# bad: [efbba60e5b8aed95b2413ff4169632bf3605c963] path: fix bugs and inconsistencies
git bisect bad efbba60e5b8aed95b2413ff4169632bf3605c963
# good: [c3e1c31baa8f4a59ef49ea0d0429ec55efe2ca81] build: upgrade clang-format to v18
git bisect good c3e1c31baa8f4a59ef49ea0d0429ec55efe2ca81
# first bad commit: [efbba60e5b8aed95b2413ff4169632bf3605c963] path: fix bugs and inconsistencies

cc @huseyinacacak-janea

@avivkeller
Copy link
Member

avivkeller commented Oct 17, 2024

git bisect is pointing to efbba60 / #54224

path.resolve("/path/");
// v23.0.0: "/path/"
// v22.9.0: "/path"

@avivkeller
Copy link
Member

IMHO path separators shouldn't be preserved since that would make the resolution of identical paths (one with / and one without) return different results. See my revert for more info

@repnop

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. npm Issues and PRs related to the npm client dependency or the npm registry. regression Issues related to regressions. v23.x v23.x Issues that can be reproduced on v23.x or PRs targeting the v23.x-staging branch.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants