-
Notifications
You must be signed in to change notification settings - Fork 254
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
improve command map combined with prefix #311
base: master
Are you sure you want to change the base?
Conversation
b530fbf
to
1d95fcd
Compare
I think this could break capistrano-rbenv. It adds the prefix
|
@mattbrictson will check, thanks! I'd expect I think running anything in |
1d95fcd
to
87d8186
Compare
So did we think we would merge this? This would also solve #299 which I'd be glad of, reading the notes here from you both it looks like the |
I believe |
@mattbrictson the example in #311 (comment) should fail also locally, right? $ rbenv exec /usr/bin/env ruby -v
ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-darwin15] Works just fine for me. We are using this patch in production for few weeks and successfully migrated to ruby 2.2 with this. |
so this example works properly: SSHKit.config.command_map[:bundle] = -> { "#{fetch(:ruby_cmd)} /usr/bin/local/bundle" }
it is valid use case to set both command map and prefix in case of using different binary, the map should still be active and prefix should be applied as extra so following example correclty executes rake2.2 within bundler: SSHKit.config.command_map[:rake] = 'rake2.2' SSHKit.config.command_map.prefix[:rake] = 'bundle exec' Only drawback is, that resulting command will be: bundle exec /usr/bin/env rake2.2
87d8186
to
a13b366
Compare
Rebased this to be mergeable again. @mattbrictson I really don't have any server with rbenv. If you have any, could you try to do a deploy with this? I tried those commands locally and all works. From all the source code, it should be just fine. Wrapping a command in |
I'll see if I can check now. |
Unfortunately it doesn't work in my local environment (OS X).
|
Just to prove it works without the
|
@mattbrictson that is so weird. It works for me. Also rbenv 1.0.0. Both in zsh and bash.
I edited by
I really don't see how that could fail. Could you also try running:
I have no idea how to reproduce this. That really should not happen AFAIK. |
Well, clearly something strange is going on. My own OS X machine fails (rbenv installed via Homebrew), and other Ubuntu VMs that I have also fail (rbenv installed via Git clone). However, just now I provisioned a brand new Ubuntu 14.04 VM, installed rbenv, and Perhaps it is a certain rbenv plugin that is causing issues, but I'm grasping at straws here. I'll try to figure it out. |
I've narrowed it down: rbenv fails and prints |
So: Could you try to run it with Internally it is using There are some script inclusions, that could affect it. It is hard to help when I can't make it fail. edit: |
OK, if you look here: https://github.com/rbenv/rbenv/blob/e851250da66307a2584218b555068c941b493d44/libexec/rbenv-which#L38-L43 Then you'll see that Example:
But if I switch to
Long story short, it only works if you don't have a default version set (and are therefore using the system version). |
So. You cannot run ANY command that is not provided by ruby, via Ok. Guess that is not going away no matter how wrong it is. Lets figure out a way to disable this in the capistrano-rbenv plugin? Would you think that would be the right place to do so? |
Honestly I'm not sure how to approach this. I don't use capistrano-rbenv in production, so I'm probably not a good person to give advice. But to recap: my understanding is that capistrano-rbenv prefixes known Ruby binaries (e.g. rake, bundler) with There isn't really an SSHKit API to disable the default I don't see a way out. What's your take? |
It is valid use case to set both command map and prefix.
In case of using different binary, the map should still be active and prefixed.
Includes and closes #310.
Will squash the commits when we agree that it makes sense.
It changes prefix to generate:
bundle exec /usr/bin/env rake
instead ofbundle exec rake
.Would that be a breaking change?