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

Document extra settings for the capistrano-rbenv plugin #47

Closed
wants to merge 1 commit into from

Conversation

guillehorno
Copy link

@guillehorno guillehorno commented May 15, 2020

I found out the extra settings that capistrano-rbenv needs to be able to detect the small differences with the "regular" rbenv (related with this issue: #46) . I've added to the docs in case someone else needs it to. Let me know if you think its ok, or if it needs more explanation or changes.

Thanks!

I found out the extra settings that capistrano-rbenv needs to be able to detect the small differences with the "regular" rbenv. I've added to the docs in case someone else needs it to. Let me know if you think its ok, or if it needs more explanation or changes.

Thanks!
@FooBarWidget
Copy link
Member

Thanks for the pull request @guillehorno, and thanks for figuring out this issue!

I've also done some investigations, and I think the issue can be solved in a better way. Instead of setting rbenv_ruby_dir, we could set:

set :rbenv_custom_path, '/usr/lib/rbenv'

This is the path to fullstaq-rbenv's system-wide installation directory. /usr/lib/rbenv/versions/* are symlinks to /usr/lib/fullstaq-ruby/versions/*. This way, capistrano-rbenv will end up checking whether /usr/lib/rbenv/versions/#{rbenv_ruby} exists.

However, this requires a change on the Fullstaq Ruby side. With the above option, capistrano-rbenv will end up mapping 'ruby' to /usr/lib/rbenv/bin/rbenv exec ruby. However, /usr/lib/rbenv/bin/rbenv does not exist: we don't package it. So we need to modify the fullstaq-rbenv packaging script (container-entrypoints/build-rbenv-{deb,rpm}) to package /usr/lib/rbenv/bin.

We could make things even better if we modify capistrano-rbenv as well. Imagine that, instead of setting rbenv_custom_path, we could set:

set :rbenv_type, :fullstaq

This would make capistrano-rbenv autodetect that rbenv_path should be /usr/lib/rbenv.

Would you like to make these changes? If not, let me know, and I'll do it instead.

If you do choose to make these changes, please don't forget to add a test case as well.

@guillehorno
Copy link
Author

guillehorno commented May 30, 2020

Ok, if I understood correctly, the best option would be to add that option on capistrano-rbenv

set :rbenv_type, :fullstaq

This wouldn't need the changes you proposed as a first alternative right? (changing and testing the new packaging scripts). I feel more comfortable making that PR to capistrano-rbenv (it's ruby) instead of modifying and testing the packaging scripts :)

Let me know if thats ok and I'll move forward and make a PR on capistrano-rbenv asking if they are ok with adding the :rbenv_type option.

@FooBarWidget
Copy link
Member

FooBarWidget commented Jun 15, 2020

The change that I proposed would require a PR to capistrano-rbenv, as well as a change to the fullstaq-ruby packaging scripts:

this requires a change on the Fullstaq Ruby side. [...] capistrano-rbenv will end up mapping 'ruby' to /usr/lib/rbenv/bin/rbenv exec ruby. However, /usr/lib/rbenv/bin/rbenv does not exist: we don't package it. So we need to modify the fullstaq-rbenv packaging script (container-entrypoints/build-rbenv-{deb,rpm}) to package /usr/lib/rbenv/bin.

If you are not comfortable with the latter, then I can pick up that task.

@FooBarWidget FooBarWidget changed the base branch from master to main June 19, 2020 12:23
FooBarWidget added a commit that referenced this pull request Jun 19, 2020
This makes our Rbenv installation look more like a real one,
which solves some compatibility problems. This change allows
implementing the following compatibility fix for capistrano-rbenv:
#47 (comment)
FooBarWidget added a commit that referenced this pull request Jun 19, 2020
This makes our Rbenv installation look more like a real one,
which solves some compatibility problems. This change allows
implementing the following compatibility fix for capistrano-rbenv:
#47 (comment)
FooBarWidget added a commit that referenced this pull request Jun 19, 2020
This makes our Rbenv installation look more like a real one,
which solves some compatibility problems. This change allows
implementing the following compatibility fix for capistrano-rbenv:
#47 (comment)
@FooBarWidget
Copy link
Member

@guillehorno I've gone ahead and created a pull request for capistrano-rbenv: capistrano/rbenv#92

mattbrictson pushed a commit to capistrano/rbenv that referenced this pull request Jul 10, 2020
Fullstaq Ruby installs Ruby to /usr/lib/fullstaq-ruby/versions
(of which /usr/lib/rbenv/versions is a symlink to).
The currently documented way to support this is via setting
`rbenv_ruby` to /usr/lib/fullstaq-ruby/versions, as implemented
in pull request #91. However, the drawback of this approach
is that SSHKit command mappings don't work. When one calls...

    execute(:ruby, '-v')

...capistrano-rbenv ends up executing $HOME/.rbenv/bin/rbenv.
But Rbenv is not installed there, but in /usr/lib/rbenv/bin/rbenv.

This change adds full support for Fullstaq Ruby, in a way
that makes SSHKit command mappings work.

Requires Fullstaq Ruby epic 3.3. See also:
fullstaq-ruby/server-edition#47 (comment)
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.

2 participants