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

nvm use should always activate given version regardless of nvm_current_version #222

Open
dangh opened this issue Apr 5, 2024 · 12 comments

Comments

@dangh
Copy link

dangh commented Apr 5, 2024

Hello,

This is my setup.

  • I use fish_user_paths to my global utils dir, which happen to have node 20 inside.

  • I'm activating node 16 for work with nvm use 16. nvm_current_version is 16 and exported.

  • I'm run a subshell for my script.

    fish -c "
        set -S nvm_current_version # -> 16 as it was imported from parent process
    
        nvm use 16 # has no effect as nvm_current_version is already 16
    
        node -v # -> 20 because of fish_user_paths
    "

The culprit is the check here:

nvm.fish/functions/nvm.fish

Lines 150 to 153 in c69e5d1

if test $ver != "$nvm_current_version"
set --query nvm_current_version && _nvm_version_deactivate $nvm_current_version
test $ver != system && _nvm_version_activate $ver
end

I don't know the reason for this, but I would expected nvm use to always active the given version.

What do you think?

@dangh dangh changed the title nvm use should always activate given version regardless of nvm_current_version nvm use should always activate given version regardless of nvm_current_version Apr 5, 2024
@jorgebucaran
Copy link
Owner

nvm_current_version is internal to nvm.fish and isn't meant for user modification. Can you describe the actual problem you had? I'm not sure what you're attempting.

@dangh
Copy link
Author

dangh commented Apr 6, 2024

The script above is just for debugging the issue. I'm not using nvm_current_version directly. Only nvm use.

Let me try again.

  1. In my interactive shell, I ran nvm use to read from .nvmrc. nvm_current_version is resolved and exported. Let say it's 16
  2. I run my build script. Inside my build script, I ensure the node version by nvm use again. Ideally, it should read .nvmrc and update PATH accordingly. But because nvm_current_version is imported from interactive shell above (both are 16), nvm choose to skip it and does not updating PATH.
  3. The problem arise when I have a pinned node 20 in my config (archived using fish_user_paths and is NOT managed by nvm). Because nvm use doesn't updating PATH in step 2, my script failed because it run on a different version of node.

My use case is rare as node should always be managed by nvm. But it's unavoidable for me. And I would say, nvm use should be deterministic and always updating the PATH accordingly.

@jorgebucaran
Copy link
Owner

What steps did you follow according to the nvm documentation, and what didn't work?

@dangh
Copy link
Author

dangh commented Apr 6, 2024

I only use nvm use to switch to version stated by .nvmrc.

  • In the main process, nvm use works. No issue here.
  • In the sub process, nvm use see that nvm_current_version already exist (it's imported from the main process), so it doesn't update the PATH.

You can do these steps to reproduce it.

# assuming you has node v20.11.1 in your system
fish_add_path ~/.local/share/nvm/v20.11.1/bin

# inside test dir, we need node 16
echo 16 > .nvmrc

function my_script
    nvm use
    node -v # -> it's 16 as expected
    fish -c '
       nvm use
       node -v # -> expected 16 but its 20
    '
end

# run test script inside test dir
my_script

@jorgebucaran
Copy link
Owner

Why do you need to run nvm use twice?

@dangh
Copy link
Author

dangh commented Apr 6, 2024

The first is for my interactive use. The one inside the script is because I want to run it for multiple projects that use different node versions.

@jorgebucaran
Copy link
Owner

But do you need to start a new shell in your function? nvm is designed for interactive use first.

@dangh
Copy link
Author

dangh commented Apr 6, 2024

The first nvm use should be outside my_script. Sorry my bad example cause a confusion.

I spin a new shell in the function because there are some other stuff that will affect my interactive shell that I don't want to.

@jorgebucaran
Copy link
Owner

Not sure why your setup is so restrictive, but it seems like you might be using the wrong tool for the job. Regardless, I'm not sure about the ideal nvm use behavior in a subshell. Wonder what nvm.sh does in this context.

@dangh
Copy link
Author

dangh commented Apr 6, 2024

I never use nvm.sh before so I'm not sure. nvm.fish is managed node for me so I want to rely on it as much as I can.

I have some questions.

  • If nvm is not intended for non-interactive, do we need to export nvm_current_version?
  • What is the purpose of bail out based on nvm_current_version? Updating PATH seem very fast.
  • Should we bail out using _nvm_node_info which leverage the actual node?

@jorgebucaran
Copy link
Owner

Great questions. Perhaps exporting isn't necessary, but I am sure I had a good reason for it when I originally wrote the code. I'll need to take some time to revisit this. Can't say exactly when, but for now, consider removing --export from:

function _nvm_version_activate --argument-names ver
set --global --export nvm_current_version $ver
set --prepend PATH $nvm_data/$ver/bin
end

@dangh
Copy link
Author

dangh commented Apr 6, 2024

Thanks you. For now I'll fork nvm.fish and unexporting it. Also thanks a ton for making fisher. It makes installing packages super convenient.

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

No branches or pull requests

2 participants