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

Fix user rbenv installs to use $HOME instead of ~ #57

Closed
wants to merge 1 commit into from

Conversation

cshaffer
Copy link

As a result of capistrano/sshkit#250 user installs of rbenv result in incorrect ENV in the mapped bins. SSHKit now quotes all ENV values, resulting in the rbenv root path to not be expanded, and thus invalid.

So what used to be this:
RBENV_ROOT=~/.rbenv RBENV_VERSION=2.2.2 ~/.rbenv/bin/rbenv exec gem query --quiet --installed --name-matches ^bundler$

Became this:
RBENV_ROOT="~/.rbenv" RBENV_VERSION="2.2.2" ~/.rbenv/bin/rbenv exec gem query --quiet --installed --name-matches ^bundler$

This is easy to workaround by explicitly setting the rbenv_path property, however the default behavior is broken when using the latest SSHKit.

This PR changes the install path to use $HOME instead of ~

@mattbrictson
Copy link
Member

Related: capistrano/sshkit#303

@mattbrictson
Copy link
Member

A fix is definitely needed, since capistrano-rbenv is now broken with sshkit 1.8.0. However I don't think this PR goes far enough. It requires developers to set a new :rbenv_user value, and even then, it makes assumptions about the location of that user's home directory being in /home (usually true, but not always).

@cshaffer cshaffer changed the title New property specifying username for user rbenv installs Fix user rbenv installs to use $HOME instead of ~ Dec 3, 2015
@cshaffer
Copy link
Author

cshaffer commented Dec 3, 2015

Thanks for reviewing @mattbrictson, sorry for the hardcoding laziness before. Per your suggestion in capistrano/sshkit#303 I switched to use $HOME and verified this works locally as well.

@mattbrictson
Copy link
Member

@cshaffer Great, thanks for verifying! It looks like we both had the same idea: I just opened #59 before seeing your commit. Oops!

@kirs Feel free to merge either one. Thanks

@kirs
Copy link
Member

kirs commented Jan 3, 2016

Thanks for your PR! I merged @mattbrictson's patch. Feel free to update to 2.0.4

@kirs kirs closed this Jan 3, 2016
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.

3 participants