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 issue with sshkit escaping $HOME #88

Merged
merged 3 commits into from
Jan 15, 2020
Merged

Conversation

KNejad
Copy link
Contributor

@KNejad KNejad commented Jan 12, 2020

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
Copy link

capistrano-bot commented Jan 12, 2020

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 🚫 Danger

@KNejad KNejad changed the title Use tilde instead of $HOME Fix issue with sshkit escaping $HOME Jan 12, 2020
@mattbrictson
Copy link
Member

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.

@mattbrictson mattbrictson merged commit fea2cf7 into capistrano:master Jan 15, 2020
@Eric-Guo
Copy link

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)

Eric-Guo added a commit to thape-cn/oauth2id that referenced this pull request Jan 15, 2020
@mattbrictson
Copy link
Member

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

mattbrictson added a commit that referenced this pull request Jan 15, 2020
mattbrictson added a commit that referenced this pull request Jan 15, 2020
* Revert "Fix issue with sshkit escaping `$HOME` (#88)"

This reverts commit fea2cf7.

* Add PR link
@mattbrictson
Copy link
Member

Reverted and released as 2.1.6.

@KNejad
Copy link
Contributor Author

KNejad commented Jan 15, 2020

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
Copy link
Member

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
Copy link

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
Copy link

mleu commented May 14, 2020

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.

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.

5 participants