capistrano / rbenv

Idiomatic rbenv support for Capistrano 3.x
MIT License
203 stars 60 forks source link

Fix issue with sshkit escaping `$HOME` #88

Closed KNejad closed 4 years ago

KNejad commented 4 years ago

It appears that in sshkit everything is being escaped except for the "~" sign here: https://github.com/capistrano/sshkit/blob/master/lib/sshkit/backends/abstract.rb#L85

So on the code I have changed it runs this check:

if test ! -d \$HOME/.rbenv/plugins/ruby-build; then echo "Directory does not exist '\$HOME/.rbenv/plugins/ruby-build'" 1>&2; false; fi

instead of this check:

if test ! -d $HOME/.rbenv/plugins/ruby-build; then echo "Directory does not exist '\$HOME/.rbenv/plugins/ruby-build'" 1>&2; false; fi

Note the missing \ before $HOME.

I believe this is intended functionality on sshkit, so I'm not creating a PR over there. However that causes an issue here, where it will never return correctly there, even if the directory $HOME/.rbenv exists.

To fix the issue, I replaced $HOME with ~ which when deploying with my version of the gem, everything then works correctly.

capistrano-bot commented 4 years ago

Thanks for the PR! This project lacks automated tests, which makes reviewing and approving PRs somewhat difficult. Please make sure that your contribution has not broken backwards compatibility or introduced any risky changes.

Generated by :no_entry_sign: Danger

mattbrictson commented 4 years ago

Looks good, thanks for the fix!

FYI please do not bump the version; that happens when the maintainer publish a release. I'll fix it up prior to merging.

Eric-Guo commented 4 years ago

But this PR lead below issue and previous version (2.1.4) have no such problem.

 DEBUG [89e42fce] Command: cd /var/www/oauth2id/releases/20200115021955 && ( export RBENV_ROOT="~/.rbenv" RBENV_VERSION="2.6.5" ; ~/.rbenv/bin/rbenv exec bundle install --path /var/www/oauth2id/shared/bundle --jobs 4 --without development test --deployment --quiet )

 DEBUG [89e42fce]   rbenv: version `2.6.5' is not installed (set by RBENV_VERSION environment variable)
mattbrictson commented 4 years ago

@Eric-Guo Good catch, I'll revert this PR. Sorry about that!

mattbrictson commented 4 years ago

Reverted and released as 2.1.6.

KNejad commented 4 years ago

Sorry about that guys. I tested my change on a fresh Debian server and everything seemed to work fine. cap install worked with my branch and didn't work with the upstream version. Any idea why that happened on @Eric-Guo's project but not mine?

EDIT: Thinking about it now it might have something to do with my project using https://github.com/capistrano-plugins/capistrano-rbenv-install which installs the Ruby version specified in the project, during the installation. Not quite sure how that could effect it, but it's a likely cause worth looking into

mattbrictson commented 4 years ago

I tried running the command from @Eric-Guo's error message locally, and got the same error.

( export RBENV_ROOT="~/.rbenv" RBENV_VERSION="2.6.5" ; ~/.rbenv/bin/rbenv exec bundle install )
rbenv: version `2.6.5' is not installed (set by RBENV_VERSION environment variable)

Even though I do have Ruby 2.6.5 installed in ~/.rbenv.

I think the problem is that ~ is not expanded by the shell in this statement:

export RBENV_ROOT="~/.rbenv"
echo $RBENV_ROOT
# => ~/.rbenv

Whereas $HOME is expanded:

export RBENV_ROOT="$HOME/.rbenv"
echo $RBENV_ROOT
# => /Users/mbrictson/.rbenv
Eric-Guo commented 4 years ago

I use CentOS 7 and installation log can be found in my blog, I think it also not working in AMI Linux as I having one server also.

mleu commented 4 years ago

For a first deploy to a empty server had to do this workaround:

  1. use capistrano-rbenv 2.1.5 to avoid sshkit escaping $HOME and get ruby installed. It then fails with @Eric-Guo's error message.
  2. then switch to capistrano-rbenv 2.1.6 and deploy again

It seems like issue with sshkit escaping $HOME only occurs when you don't have ruby installed yet.