-
Notifications
You must be signed in to change notification settings - Fork 448
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
Add shims for Volta-like binaries #609
base: master
Are you sure you want to change the base?
Conversation
I am still not sure this will fix the graphic IDE issue, but it will most certainly fix the rehashing issues. Still need to consider how to tackle the IDE issue. Hmm. Maybe I have the strangest idea. I will try.
…o we can't ensure a consistent symlink path
…ew PID so we can't ensure a consistent symlink path" This reverts commit d1f7b9d.
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/schniz/fnm/CkggR7pGuDDtUcAG2qtKPoM8JYHL |
d1f7b9d is because I thought I would be able to change the symlink generation process from random to using the parent process' pid, and therefore making it reproducible, and making Unfortunately, every shell script has its own pid so we need to find a common denominator somehow. Shells are obviously not the common denominator (the shims are a shell script), so we need to know whether it's an interactive shell--or a graphical IDE (like #475 and other issues). If someone has an idea how to pull this off, let me know 😃 |
CLI (pid1) -> node shim (pid2) -> fnm (pid3) fnm tries to take pid2, instead of pid1. The same can be applied in VSCode or XCode or whatever: VSCode (pid1) -> some 3rd party script (pid2) -> another 3rd party (pid3) -> fnm (pid4) and now we're trying to use pid3, which can be a one-off thing. So pids are probably not what we want. I also thought about implementing a table of pids to versions but this is just insanity and composes very very badly (how up the tree we need to update version when we switch?) 😁 |
The premise of reproducible "multishell" paths is good, because it simplifies the installation of fnm, which can be awesome. |
@@ -0,0 +1,3 @@ | |||
#!/usr/bin/env sh | |||
|
|||
fnm exec --using-current -- node "$@" |
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.
May be you should use exec fnm exec ...
so fnm's process replaces shell and process tree would be like this:
CLI (pid1) -> node shim (pid2) -> fnm (pid2)
Not sure how to do this in windows.
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.
oh! TIL!
this is still the problem because we have unknown parents that can bridge it. :(
shell -> scripts/run-tests.sh -> scripts/run-unit-tests.sh -> fnm exec ...
pids don't solve it 😭
Shims are a fundamentally different model than PATH hijacking, and they come with an entirely distinct set of problems; I’ll be very curious to see how fnm deals with both at once. |
I've read some linked issues and it seems that people want fnm to install node globally (or behave like it's installed globally). Am I correct? |
What about this shim? #!/bin/sh
cwd=$PWD
while [ -n "$cwd" ]; do
if [ -f "$cwd/.node-version" ]; then
# echo "Found $cwd/.node-version"
exec fnm exec --using "$cwd/.node-version" -- node "$@"
fi
if [ -f "$cwd/.nvmrc" ]; then
# echo "Found $cwd/.nvmrc"
exec fnm exec --using "$cwd/.nvmrc" -- node "$@"
fi
cwd=${cwd%/*}
done
exec fnm exec --using default -- node "$@" |
We can have I just wonder if there's a better option we haven't though of, that will make |
You meant beside changing default version? |
docs/commands.md
Outdated
@@ -194,6 +194,9 @@ FLAGS: | |||
-V, --version | |||
Prints version information | |||
|
|||
--with-shims |
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.
it seems that this args is removed at the latest version?
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.
it was never added, I'm gonna add this with a proper warning, I will test it on my machine for now
|
main problem I see with it right now, that it will make all global node module installations fail, as they install binaries in gotta dig deeper |
This PR introduces
--with-shims
to thefnm env
command.These shims (currently only
node
, but maybenpm
will need it too) will be used instead of the$FNM_MULTISHELL_PATH
, and although they will introduce some runtime penalty (~8-16ms on my machine), it will be much easier to integrate with other tools and make people understand how fnm works.This should also fix some
rehash
ing issues some users have on newer Zsh versions.