npm / cli

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

[BUG] Invalid package name ".DS_Store" #7492

Closed xiaozhuai closed 3 weeks ago

xiaozhuai commented 3 weeks ago

Is there an existing issue for this?

This issue exists in the latest npm version

Current Behavior

Exec npm update -g.

npm error code EINVALIDPACKAGENAME
npm error Invalid package name ".DS_Store" of package ".DS_Store@*": name cannot start with a period.

npm error A complete log of this run can be found in: /Users/xiaozhuai/.cache/npm/_logs/2024-05-09T05_40_34_718Z-debug-0.log

2024-05-09T05_40_34_718Z-debug-0.log

If I remove .DS_Store with rm /opt/node/npm/lib/node_modules/.DS_Store, then it works. I am on macOS, and I've set prefix=/opt/node/npm in .npmrc.

Environment

; prefix = "/opt/homebrew" ; overridden by user

; "user" config from /Users/xiaozhuai/.npmrc

//registry.npmjs.org/:_authToken = (protected) cache = "/Users/xiaozhuai/.cache/npm" prefix = "/opt/node/npm" strict-ssl = false

; node bin location = /opt/node/node/bin/node ; node version = v18.19.1 ; npm local prefix = /opt ; npm version = 10.7.0 ; cwd = /opt ; HOME = /Users/xiaozhuai

ljharb commented 3 weeks ago

Why is prefix set?

xiaozhuai commented 3 weeks ago

Why is prefix set?

I just want to custom it.

ljharb commented 3 weeks ago

Why? npm only comes with node, so it should only be located in the location that node ships it in.

xiaozhuai commented 3 weeks ago

Why? npm only comes with node, so it should only be located in the location that node ships it in.

I separated them. I manually installed node to /opt/node/node, and set npm prefix to /opt/node/npm.

This has nothing to do with why I set the prefix, so far everything works fine with my configuration instead of this issue. It seems that npm did not use the correct filter results when scanning the global lib directory, which caused this problem.

And here is a related issue in old repo. https://github.com/npm/npm/issues/20493

ljharb commented 3 weeks ago

prefix changes npm root -g, so it very well might be related. Have you tried a standard unmodified install to see if it's your modifications causing the problem?

xiaozhuai commented 3 weeks ago

prefix changes npm root -g, so it very well might be related. Have you tried a standard unmodified install to see if it's your modifications causing the problem?

Would you please try following steps to repro.

touch "$(npm config get prefix)/lib/node_modules/.DS_Store"
npm update -g
ljharb commented 3 weeks ago

I definitely get the same error, but npm update -g isn't a command that should work anyways, since there's no global package.json to update. Global packages need to be updated manually, one at a time.

xiaozhuai commented 3 weeks ago

npm update -g isn't a command that should work anyways.

image

The help message says it is ok. <pkg> is optional.

ljharb commented 3 weeks ago

Seems like two legit issues: a documentation error, as well as "it should be failing with a clearer message".

xiaozhuai commented 3 weeks ago

I bet npm update -g works fine, I often do it. This can easily update all global libraries. The only problem is that .DS_Store is not filtered out correctly. In addition to .DS_Store, there may be some other files that need to be filtered. But at least all regular file type should be excluded from the results and only the folder type should be kept as package name.

xiaozhuai commented 3 weeks ago

BTW, npm list -g works fine with .DS_Store.

image
xiaozhuai commented 3 weeks ago

https://github.com/npm/cli/blob/e9fdc9a792666d5b9aa73833c65516bafcb80863/workspaces/arborist/lib/arborist/build-ideal-tree.js#L455

I tried change line 445 to following, and it works.

const paths = (await readdirScoped(nm).catch(() => []))
        .filter(p => p !== '.DS_Store')

Maybe we should fix it in readdir-scoped.js https://github.com/npm/cli/blob/e9fdc9a792666d5b9aa73833c65516bafcb80863/node_modules/%40npmcli/fs/lib/readdir-scoped.js#L1-L20

ljharb commented 3 weeks ago

Possibly better logic would be, to filter out anything that can’t be a valid package name?

xiaozhuai commented 3 weeks ago

Possibly better logic would be, to filter out anything that can’t be a valid package name?

I agree, Maybe we should refer to npm list -g, since it works fine.

wraithgar commented 3 weeks ago

It looks like your operating system put an invalid name in your node_modules folder. npm can't support things like this, it has to assume everything in there is something it put there. It has to error to let you know that you are now going to get unexpected results.

npm update -g does work. Because there is no global manifest it will always update to the latest version.

The solution here is to remove those files that osx put there, and configure it to stop doing that.

xiaozhuai commented 3 weeks ago

It looks like your operating system put an invalid name in your node_modules folder.

As we all know, macOS may place a .DS_Store file in any directory.

It's unimaginable that npm doesn't handle it.

it has to assume everything in there is something it put there

Too idealistic.

The solution here is to remove those files that osx put there, and configure it to stop doing that.

This is not a solution.

I'm starting to understand why npm is the default package manager distributed with node, but people still invent alternatives.

I will move to yarn or pnpm, whatever.

xu-minghao317 commented 2 days ago

I don’t understand why this issue is closed. This should be handled by npm, not macOS. There’s no way to disable '.DS_Store' for a single folder. A workaround is to add find "$(npm list -g | head -1)" -name '.DS_Store' -type f -delete to your shell startup files, but that’s awkward. Good software should accommodate users, not the other way around.