-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
fix: avoid glob expansion errors when no node versions installed #3741
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
base: master
Are you sure you want to change the base?
Conversation
|
Instead of altering a bunch of the logic, could we perhaps just disable nullglob when needed for the |
1a95084 to
da2a20f
Compare
…ions installed Fixes nvm-sh#3727 When no node versions are installed, nvm ls produces errors like: `find: /path/.nvm/versions/node/*: No such file or directory` This happens because bash expands `*` literally when nullglob is not set and the directory is empty. Solution: - For bash: temporarily enable nullglob before the find command - For other shells (sh, dash, zsh): suppress stderr (2>/dev/null) - Restore nullglob to its previous state after the command completes - Uses BASH_VERSION check to detect bash vs other shells
da2a20f to
9449392
Compare
|
so this kind of works, but relies on nothing exiting prior to the restoration. i was thinking instead, of wrapping every call to |
Move nullglob enabling into the command substitution subshell so that the shell option change cannot persist to the parent shell, even if something exits early. This is a safer approach than save/restore. The subshell automatically cleans up when it exits, making it impossible for nullglob to leak into the user's shell environment.
|
Awesome! Now we need a test that has the wrong nullglob setting and will prevent regressions :-) |
Adds a test that explicitly disables nullglob and runs nvm ls with an empty versions directory to ensure the fix prevents regressions. The test verifies: - nvm ls does not produce 'No such file or directory' errors - nvm ls exits with code 0 when no versions are installed
done |
|
@ljharb can you review this? |
ljharb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately the test passes even without the fix.
Summary
Fixes #3727
When no node versions are installed,
nvm lsproduces errors like:This happens because bash expands
*literally whennullglobis not set and the directory is empty.Solution
find DIR -mindepth 1 -maxdepth 1instead offind DIR/*Before
After
$ nvm ls -> systemTesting
~/.nvm/versions/node/directory