Prevent duplicate fnm_multishells in PATH

[teohhanhui]

Only eval fnm env in non-login shells.

The duplicate fnm_multishells in PATH leads to many problems, including a very broken fnm use system.

(This will of course also need to be fixed in the installer script as well...)

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fnm	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 3, 2024 6:59pm

[changeset-bot]

⚠️ No Changeset found

Latest commit: 8ae819f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Schniz]

please explain this change further
it really de-simplifies the installation process and i don't get why you wouldn't want to have fnm env in a script, if you actually call it

[Schniz]

none of my scripts actually run fnm env but maybe because my scripts are in bash and i'm using zsh as shell

[teohhanhui]

The reason is that it's standard to source ~/.bashrc from ~/.bash_profile, e.g. the default in Fedora. (And it's also documented in bash documentation...)

If we don't have this check, it'll lead to duplicate fnm_multishells paths in PATH (once from login, once from the interactive shell). This means even if you try to disable fnm by fnm use system, it'd still fall back to the other instance in fnm_multishells, leading to errors like wrongly using the default version, or in some cases a non-functional node or npm.

[teohhanhui]

One possible alternative is to add a check to fnm env to output nothing if it detects fnm_multishells is already in PATH. (We can't just skip adding to PATH because that means we'll end up with a mismatch of which fnm_multishells instance is being used.)

[Schniz]

One possible alternative is to add a check to fnm env to output nothing if it detects fnm_multishells is already in PATH. (We can't just skip adding to PATH because that means we'll end up with a mismatch of which fnm_multishells instance is being used.)

this sounds better. filtering out other multishell paths sounds like the best way to do that.

[fabricionaweb]

Im playing around here, although I dont know Rust I was able to read from $PATH and to filter out the fnm_multishell values before adding the new one.

The bad take on this is that it returns the full $PATH to be set, instead of merging to...

Example:

before

$ fnm env --shell bash
export PATH="/run/user/1000/fnm_multishells/5255_1743507809600/bin":"$PATH"
export FNM_MULTISHELL_PATH="/run/user/1000/fnm_multishells/5255_1743507809600"
export FNM_VERSION_FILE_STRATEGY="local"
export FNM_DIR="/home/fabricio/.local/share/fnm"
export FNM_LOGLEVEL="info"
export FNM_NODE_DIST_MIRROR="https://nodejs.org/dist"
export FNM_COREPACK_ENABLED="false"
export FNM_RESOLVE_ENGINES="true"
export FNM_ARCH="x64"

after

$ ./target/debug/fnm env --shell bash
export PATH="/run/user/1000/fnm_multishells/7193_1743508011184/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/usr/lib/wsl/lib:/home/fabricio/.local/bin:/home/fabricio/.asdf/shims:/home/fabricio/.asdf/installs/golang/1.23.3/packages/bin"
export FNM_MULTISHELL_PATH="/run/user/1000/fnm_multishells/7193_1743508011184"
export FNM_VERSION_FILE_STRATEGY="local"
export FNM_DIR="/home/fabricio/.local/share/fnm"
export FNM_LOGLEVEL="info"
export FNM_NODE_DIST_MIRROR="https://nodejs.org/dist"
export FNM_COREPACK_ENABLED="false"
export FNM_RESOLVE_ENGINES="true"
export FNM_ARCH="x64"

and filtering it (note the 6993_1743507996360 striped out):

echo $PATH
/run/user/1000/fnm_multishells/6993_1743507996360/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/usr/lib/wsl/lib:/home/fabricio/.local/bin:/home/fabricio/.asdf/shims:/home/fabricio/.asdf/installs/golang/1.23.3/packages/bin
$ ./target/debug/fnm env --shell bash
export PATH="/run/user/1000/fnm_multishells/8013_1743508192319/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/usr/lib/wsl/lib:/home/fabricio/.local/bin:/home/fabricio/.asdf/shims:/home/fabricio/.asdf/installs/golang/1.23.3/packages/bin"
export FNM_MULTISHELL_PATH="/run/user/1000/fnm_multishells/8013_1743508192319"
export FNM_VERSION_FILE_STRATEGY="local"
export FNM_DIR="/home/fabricio/.local/share/fnm"
export FNM_LOGLEVEL="info"
export FNM_NODE_DIST_MIRROR="https://nodejs.org/dist"
export FNM_COREPACK_ENABLED="false"
export FNM_RESOLVE_ENGINES="true"
export FNM_ARCH="x64"

Would this approach be any good @Schniz ? I can send MR/patch of changes if you agree on the idea.

---

Honest, I think this is simplier... I will add an if before load the eval $(fnm env)

[ -z "$FNM_MULTISHELL_PATH" ] && eval "$(fnm env --shell zsh --use-on-cd)"

[teohhanhui]

@fabricionaweb We could just make fnm env output nothing at all when fnm_multishells is already in PATH.

It could be understood as: "The env is already correct, we have nothing to set."

[fabricionaweb]

@fabricionaweb We could just make fnm env output nothing at all when fnm_multishells is already in PATH.

It could be understood as: "The env is already correct, we have nothing to set."

To return nothing, IMO seems too much, the FNM_ variables are isolated to fnm only and plus it just set values. These values can change according to the parameters, for example fnm env --enable-corepack or --resolve-engines=false. I dont think they are an issue, I can be wrong tho.

I made the PR where I only touch on PATH and FNM_MULTISHELL_PATH since they are related.

I hope that helps

[teohhanhui]

These values can change according to the parameters, for example fnm env --enable-corepack or --resolve-engines=false.

Then I believe it just brings us full circle. The whole problem was that fnm env was being (re-)invoked in the shell's init script at the wrong times.

I only proposed the workaround of not setting anything in fnm env to avoid mangling the PATH, because the maintainer was understandably not happy with adding complexity to the shell init instructions.

But if things can change, then it gets more complicated... I think we need to replace the old fnm_multishells in PATH, since it might no longer match the flags / options passed in the current fnm env invocation, right?

[fabricionaweb]

But if things can change, then it gets more complicated... I think we need to replace the old fnm_multishells in PATH, since it might no longer match the flags / options passed in the current fnm env invocation, right?

That is good point! And I dont know the answer 😅 But from what I've seen, the multishell_path does not need to change because of those flags. It is just like runtime things that reads on install and use.

I need to wait for Schniz and what he thinks. To not return nothing it is easy and much simple than what I have done. And it could also have a --force flag to force it to return or something in that way.