nvm-sh / nvm

Node Version Manager - POSIX-compliant bash script to manage multiple active node.js versions
MIT License
78.46k stars 7.86k forks source link

macOS Catalina: unable to retrieve the right shell #2148

Open WebReflection opened 4 years ago

WebReflection commented 4 years ago

Operating system and version:

macOS Catalina.

nvm debug output:

as no nvm command becomes available, it's hard to answer this one.

nvm ls output:

as no nvm command becomes available, it's hard to answer this one.

How did you install nvm?

install script in readme

What steps did you perform?

just executed the script in the readme

What happened?

it couldn't find any file to put nvm environment variables, because no ~/.zshrc file was there after a clean Catalina installation.

What did you expect to happen?

I was expecting nvm to check, as last resort, for if [ "$SHELL" = "/bin/zsh" ]; then ... and eventually create by itself the $HOME/.zshrc file with those entries.

Is there anything in any of your profile files that modifies the PATH?

Nope.

WebReflection commented 4 years ago

P.S. the following command should also provide the right file to use echo "${HOME}/.${SHELL:5}rc" ... but I'm not sure it would work in older OSX too.

ljharb commented 4 years ago

Would that command also work for dash and ksh and non-bash sh?

In general, you're expected to have created a profile file for your shell before installing nvm; it's unfortunate catalina doesn't ship with one by default.

Improvements to the install script around profile files are always welcome.

WebReflection commented 4 years ago

Would that command also work for dash and ksh and non-bash sh?

I believe basename $(which $SHELL) would provide the right name in all sh like environments, and the profile file could be modified via >>, so that if the file is not there, it'd be created.

In general, you're expected to have created a profile file for your shell before installing nvm

Then this should be the first check before doing anything else, and bail out if not found/absent, otherwise developers at their first attempt on every new macOS machine would fail.

Improvements to the install script around profile files are always welcome.

I've had a look already at the install script, and there are too many things that would require more background.

As it's obvious that macOS without a profile created manually (and then again, why would an install script mandatory require that upfront?) would fail, would it be OK if I change the whole logic behind profile name detection?

'cause I think it'd be more sensible to simply use this as last resource, if nothing else has been found.

There could be even a prompt call, to avoid creating anything if not wanted, and yet that might also add friction.

WebReflection commented 4 years ago

As example, even if I am not sure where dash and ksh profile files are stored, I believe this might be all it takes to retrieve the right profile file these days:

#!/usr/bin/env sh

getShellProfile() {
  local shell="$(basename $(which "$SHELL"))"
  case $shell in
    "sh" | "bash" )
      if [ "$(uname -s)" = "Linux" ]; then
        echo -n "${HOME}/.bashrc"
      else
        echo -n "${HOME}/.bash_profile"
      fi
      ;;
    "zsh" )
      echo -n "${HOME}/.zshrc" ;;
    "dash" )
      echo -n "${HOME}/.dashrc" ;;
    "ksh" )
      echo -n "${HOME}/.kshrc" ;;
  esac
}

Just call it via getShellProfile and see if the result is the expected one per each platform (if you can test that).

ljharb commented 4 years ago

As long as it does the right thing in all POSIX shells then I'm comfortable with any change; the problem is that I lack the expertise to validate it, so I'd want many reviews and manual tests done on it :-)

justin-elias commented 4 years ago

The installer worked correctly for me by just replacing "bash" with "zsh" after the pipe. So possible easy fix is to just list an install command for each shell in the readme.

ljharb commented 4 years ago

The install script is written to be bash-specific.

jmsgwd commented 4 years ago

The installer worked correctly for me by just replacing "bash" with "zsh" after the pipe. So possible easy fix is to just list an install command for each shell in the readme.

That worked for me too, but:

jmsgwd commented 4 years ago

The install script is written to be bash-specific.

What does "bash-specific" mean - it only runs in bash, or it only updates your bash configuration?

The reason I piped it into zsh is because the readme says the installer "attempts to add the source lines from the snippet below to the correct profile file (~/.bash_profile, ~/.zshrc, ~/.profile, or ~/.bashrc)."

...but it only updated .zshrc when I tried piping the installer into zsh.

ljharb commented 4 years ago

It only runs in bash. It should work based on whatever your default shell is.

jmsgwd commented 4 years ago

OK got it.

My default shell is zsh, but when I pipe the script it into bash (as specified in the readme), it updates .bash_profile and ignores .zshrc.

I'd just like to make it clear that the bug seems to occur even if .zshrc already exists (since the bug report only talks about the scenario where .zshrc doesn't exist yet). Also, I'm running macOS 10.14.6 Mojave.

ljharb commented 4 years ago

Profile file detection is very tricky; I'm always open to PRs to improve the install script here.

marklanhamhc commented 2 years ago

The installer worked correctly for me by just replacing "bash" with "zsh" after the pipe. So possible easy fix is to just list an install command for each shell in the readme.

Yes, this worked for me, thanks! curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.1/install.sh | zsh

I agree, a simple solution would be to include some extra installation examples.

ljharb commented 2 years ago

@marklanhamhc please do not pipe the install script to anything but bash; it's explicitly and only designed to run in bash.