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

Prevent duplicate fnm_multishells in PATH #1309

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

teohhanhui
Copy link

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...)

Copy link

vercel bot commented Nov 3, 2024

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

Copy link

changeset-bot bot commented Nov 3, 2024

⚠️ 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

Only eval `fnm env` in non-login shells.
@Schniz
Copy link
Owner

Schniz commented Nov 4, 2024

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
Copy link
Owner

Schniz commented Nov 4, 2024

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

@teohhanhui
Copy link
Author

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
Copy link
Author

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants