NVM_NOUSE on install.sh to pass --no-use to nvm.sh

[wesleytodd]

Similar to my other PR (#2131) I figured I would see if this idea was good and I can flesh out the PR more if it is good.

This adds a NVM_NOUSE environment variable to the install.sh script so you can still use the install script but also pass --no-use.

Example:

$ curl -o- ... | NVM_NOUSE=true bash

Thoughts?

[ljharb]

I suppose this is low-impact and makes some sense - although --no-use is a performance workaround I really don't want to encourage; you can also nvm unalias default and not open your shell in a dir with .nvmrc and you get the same behavior.

[wesleytodd]

My use case is that I am wrapping nvm in another tool. I don’t want nvm to do anything on install other than install. So this was one part I could not figure out how to do with the current implementation.

To me it makes sense to have this control even if you don’t intent to push users to it.

[ljharb]

I suppose adding it tested, but not documented, seems reasonable.

[wesleytodd]

To follow up on this, is the only thing blocking it tests? Anything else?

[ljharb]

I think tests (and a rebase ofc) are the only thing.

[lstrojny]

This would also make special integrations simpler, e.g. on a CI system where you don’t want to modify local state but instead handle the PATH "by hand". I have the situation right now, that --no-use is properly passed to nvm.sh but nvm_supports_source_options() returns false so --no-use is ignored. The workaround for now is dynamically patching nvm.sh which is – well – ugly.

[ljharb]

@lstrojny this PR won't solve that for you; please file a new issue and we can explore why nvm_supports_source_options is returning false.

[wesleytodd]

Actually this is why I originally opened this PR. I am doing exactly what @lstrojny mentions. I cannot remember the exact details without digging back into this code, but what is described sounds familiar.

[wesleytodd]

So I did look a bit at this again, and here is an illustrative implementation which relies on this PR:

https://github.com/wesleytodd/nvmjs/blob/master/index.js#L68

The point here is that the installNvm method should install nvm but not a version of node. Obviously I think in the end we would also want the ability to do the normal nvm behavior here, but I started with installNvm only "installing nvm". Without this, that version breaks.

[ljharb]

@wesleytodd i'm confused; the install script doesn't install a version of node, only sourcing the profile files does; but either way I'm still happy to merge this with tests.

[dan2k3k4]

So this NVM_NOUSE variable would be useful for my use-case too.

I have a build that installs NVM script in the "container" which worked fine until someone added a .nvmrc file and now it seems the install.sh script runs and picks up that .nvmrc file as it's in the same directory /app.

So I end up with failed builds:

    => Downloading nvm from git to '/app/.nvm'
    W: Cloning into '/app/.nvm'...
=> => Compressing and cleaning up git repository
    
    => Appending nvm source string to /app/.bash_profile
    => Appending bash_completion source string to /app/.bash_profile
    W: N/A: version "node -> N/A" is not yet installed.
    W: 
    W: You need to run "nvm install node" to install it before using it.
    => Close and reopen your terminal to start using nvm or run the following to use it now:
    
    export NVM_DIR="$HOME/.nvm"
    [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"  # This loads nvm
    [ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"  # This loads nvm bash_completion
    W: N/A: version "node -> N/A" is not yet installed.
    W: 
    W: You need to run "nvm install node" to install it before using it.
  
  E: Error building project: Step failed with status code 3.

E: Error: Unable to build application, aborting.

I thought adding --no-use would fix things for me:

curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.35.3/install.sh | dash
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" --no-use

But turns out it actually fails inside the install.sh

Unless there's some other way to install the node version that it finds inside the .nvmrc before it tries to do nvm use ?

[ljharb]

@dan2k3k4 you can run nvm install, and it will install if needed, and then use.

[dan2k3k4]

So when would I run that when the issue happens when it's installing nvm? It's the install.sh script that fails for me since it tries to run nvm use during that setup

[ljharb]

ah, gotcha. in that case you would need this PR (which is still awaiting tests).

if you'd like to contribute tests, please comment with a link to your branch (not another PR) and i'll pull them in :-)

[dan2k3k4]

@ljharb what type of tests? I see there's a Dockerfile and a .travis.yml file so not sure if you want a test that uses those?

I then see /test folder with a /test/install_script/nvm_do_install file. So are you referring to updating that? Or perhaps duplicating it for a nvm_do_install_nouse version?

[ljharb]

The tests don't have to have anything to do with docker (that's just the motivating use case); they need to ensure that nvm use is not ran when the install script is. so yes, i'd expect that existing test file to be either modified (additively) to test the new functionality in this PR (so future changes don't break it)

[dan2k3k4]

Ok, I've tried in:
dan2k3k4/nvm@4198a670fec19b454636ece94b55fde71cb91adf

Branch
https://github.com/dan2k3k4/nvm/tree/add-tests-nvm-install-nouse

I'm not familiar with testing this way so I'm not sure if I went in the right direction or not. I tried to base it on how the other scripts in the same directory were done, and I tried to remove NVM too. I have no idea if it works.

[wesleytodd]

Testing is hard for this! It is actually why I did not get back to this. And in a typical software engineer fashion, instead of doing the hard thing I spent more time than I would have just learning it and wrote a new thing which might enable us to use "normal" JS testing.

I brought it up with @ljharb in a slack convo, but I will post it here since this came up again. Here is a JS wrapper which would enable us to test NVM via a JS interface. I haven't finished it because @ljharb expressed some concerns about the theory of "js testing bash to install js". But if we can get agreement on a good path forward I will be happy to do more leg work to get it done.

[ljharb]

@dan2k3k4 this is a great start :-) the thing that seems missing is that there's nothing testing that "use" is not called. Perhaps adding one check that use is called to the normal case, and then an opposite assertion in the NOUSE case?

[dan2k3k4]

Hey @ljharb and @wesleytodd - I haven't had time in the past week to check this as I was busy with launching the https://celebratedrupal.org site.

The place where I had errors is in the:
.platform.app.yaml#L63-L68 file. For now as a workaround, I just don't include the .nvmrc file and then it does not try to "use" any node version until I've installed it in the two lines after the NVM installer script runs.

I'm not really sure what needs to be done for checking if the use is called. Is there any artefact written anywhere after nvm use is run? Or perhaps we need a failing test to see if it tries to use a node version which doesn't exist by adding a .nvmrc file into the test?

[ljharb]

@dan2k3k4 nvm current will report the expected version if use has been called.