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

Define and update default_environment to use rbenv over capistrano #5

Merged
merged 1 commit into from Mar 6, 2013
Merged

Define and update default_environment to use rbenv over capistrano #5

merged 1 commit into from Mar 6, 2013

Conversation

tk0miya
Copy link
Contributor

@tk0miya tk0miya commented Mar 3, 2013

currently, run(), capture() and sudo() do not use ruby binary installed from rbenv.
so many people defines :default_environment (RBENV_ROOT and PATH) manually.

this fix changes that the remote-executions use rbenv by default.

@ghost ghost assigned yyuu Mar 3, 2013
@yyuu
Copy link
Owner

yyuu commented Mar 3, 2013

Thanks for PR :)

I understood that you're requesting to load rbenv into your shell. But I cannot agree to set default_environment in capistrano-rbenv since modifying some kind of global variables in 3rd party plugin may corrupt interoperability of plugins. I think that those global variables should be set by users explicitly in their recipes.

Now I'm working for same issue by another approach, and will show you in few days.

@tk0miya
Copy link
Contributor Author

tk0miya commented Mar 4, 2013

Thank you for comment.

I thought :default_environment is good way for switching ruby environment seemlessly.
but, your comment is correct. updating :default_environment is a little corrupt way.

I'd known it, and added :rbenv_define_default_environment to avoid it.

Now I'm working for same issue by another approach, and will show you in few days.

I'm looking forward to see your fix :-)

Thanks.

@yyuu
Copy link
Owner

yyuu commented Mar 6, 2013

Sorry to be late. I decided to merge this request.

As of writing previous comment, I didn't like capistrano-rbenv to depend on non-standard recipes of capistrano even if it was capistrano-ext. But I changed my mind because your hack seems much simpler than mine, and also it just worked.

I thought that initializing :rbenv_path without capturing remote may solve this situation, and created my workaround as 5b61b3b. With that patch, if we'd like to use :rbenv_path in :default_environment, we have to set :rbenv_expand_path as false before capistrano/ext/multistage has been required. It is not so quite simple and hard to understand.

Thanks so much for comments and patches 👍

yyuu pushed a commit that referenced this pull request Mar 6, 2013
Define and update default_environment to use rbenv over capistrano
@yyuu yyuu merged commit 0f4f955 into yyuu:master Mar 6, 2013
@tk0miya tk0miya deleted the setup_default_environment branch March 6, 2013 08:39
@tk0miya
Copy link
Contributor Author

tk0miya commented Mar 6, 2013

Thank you for merging!

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.

None yet

2 participants