Fix: do not use "less" when rvm is being run non-interactively #1416

Merged
merged 1 commit into from Jan 1, 2013

Conversation

Projects
None yet
3 participants
Contributor

stuartherbert commented Dec 31, 2012

scripts/cli pipes the output of scripts/requirements to the UNIX "less" pager. This stops rvm from successfully installing a ruby interpreter if rvm is NOT being run from a terminal (e.g., rvm is being run from a provisioning system such as chef or ansible).

This patch updates scripts/cli to only use "less" when rvm is being run from a terminal.

Member

richo commented Jan 1, 2013

Nice! Thanks.

richo added a commit that referenced this pull request Jan 1, 2013

Merge pull request #1416 from stuartherbert/non-interactive-rvm
Fix: do not use "less" when rvm is being run non-interactively

@richo richo merged commit 1e9716d into rvm:master Jan 1, 2013

1 check passed

default The Travis build passed
Details
Owner

mpapis commented Jan 1, 2013

I'm not happy with that change, RVM1 should not be used as function when used not in terminal, this will be fixed in RVM2, but for RVM1 there is more code that can be triggered and still behave not as you would expect.

If it's really required to use rvm as a function in provisoning then use command rvm install ... or /path/to/rvm/bin/rvm install ....

mpapis added a commit that referenced this pull request Jan 1, 2013

Owner

mpapis commented Jan 1, 2013

one more the code was also broken (at least on OSX): http://stackoverflow.com/a/14115292/497756

Contributor

stuartherbert commented Jan 2, 2013

I'm a bit confused ... I'm not using RVM as a function (just calling it from another, non-Ruby script), and the Stack Overflow question is exactly what my patch fixes.

Owner

mpapis commented Jan 2, 2013

the SO problem is with missing (use keyboard arrows to navigate) - which left the user with open less and no instruction how to handle it, it was introduced by your patch where in first check -t 1 was false (because it was piped to rvm_less) and in the second check it was true and opened less.

Owner

mpapis commented Jan 2, 2013

as for rvm is a function - the code block https://github.com/wayneeseguin/rvm/blob/848ef50f1b32c87e853f21e801f167a39e8c9c66/scripts/cli#L1066-L1078 is only executed only when RVM is loaded as a function, so you had to load rvm as a function somehow.

Contributor

stuartherbert commented Jan 2, 2013

I've updated my branch with a simple fix to avoid the second -t test.

There seems to be some mistake .. the code block is executed when I run the command 'rvm install ruby' from the command-line. Quite happy to record a short screencast showing this to be the case if necessary to convince you that this is happening.

I'm very keen for this patch (or an equivalent) to get into RVM. At the moment, RVM is unusable from automatic deployment tools such as Ansible because of this interactive prompt. Hope you can help.

Owner

mpapis commented Jan 2, 2013

As I have already explained in wayneeseguin#1416 (comment) :

If it's really required to use rvm as a function in provisoning then use command rvm install ... or /path/to/rvm/bin/rvm install ....

There is no way you are getting into this code without loading rvm as a function, either use above solution or stop sourcing rvm in provisioning, sourced rvm is available as a function and is intended only for interactive use, for not interactive use make sure to not source rvm, it's enough to use rvm binary by full path: /path/to/rvm/bin/rvm.

Contributor

stuartherbert commented Jan 2, 2013

I can't find the approach that you're advocating in the documentation. Do you have the docs in a GitHub repo, so that I can submit a pull request against those?

Owner

mpapis commented Jan 2, 2013

@stuartherbert that looks like an documentation fail, I would like to have the missing docs here https://github.com/rvm/rvm-site/blob/master/content/rubies/installing.haml#L53 - let me know if you want to add it by yourself or if I should do it.

Contributor

stuartherbert commented Jan 2, 2013

Thanks for the pointer. I'm not setup here for testing any changes I submit to that file, so it might be quicker for you to add it?

Owner

mpapis commented Jan 2, 2013

ok I will, thanks for pointing it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment