Fixes #1221: Remove static path definition for git ssh wrapper

[signe]

Used bare 'ssh' rather than '$(which ssh)' because /bin/sh could be something other than bash (e.g. csh or bourne sh) on platforms such as Solaris.

Since $(which ssh) (or which ssh) is simply using the path, it serves no specific purpose in the code.

[leehambley]

Since $(which ssh) (or which ssh) is simply using the path, it serves no specific purpose in the code.

On the contrary, it's clear that it doesn't work my magic, and that people can look up in the man what /usr/bin/env does, and it's greppable, etc.

Changing this for everyone is such a probably huge breaking change, I can't imagine it passing review.

That said, if this is a problem for you, why not replace the SSHKit command map with a default that suits you:

SSHKit.config.command_map = Hash.new do |hash, command|
  hash[command] = "/usr/local/rbenv/shims/#{command}"
end

see https://github.com/capistrano/sshkit#the-command-map

[leehambley]

I'd consider replacing /usr/bin/ssh with /usr/bin/env ssh to make it clear that we defer to whatever semblance of a PATH there may be under this highly restrictive shell

[signe]

Can you elaborate on why you think that this would be a breaking change? I don't see that logic. /usr/bin is part of the default path on every system I've used in the last 23 years, so assuming that there isn't another ssh binary that is higher in the path, it would still be the one that's used. If there is one higher, then that's what the sysadmin would prefer was used anyway.

I can see using env, so I have updated the patch to use that.

I don't understand the SSHKit command-map referral. This is a hardcoded string in a file that's executed on the remote system, not something that's being executed by the system running cap, and not dynamically created.

Currently overriding the path in git-ssh.sh requires this, which is entirely unhelpful and un-decipherable to most people. (I had to find it referenced on StackOverflow, when I needed it.)

Rake::Task["deploy:check"].clear_actions

namespace :deploy do
    task check: :'git:wrapper'  do
        on release_roles :all do
            execute :mkdir, "-p", "#{fetch(:tmp_dir)}/#{fetch(:application)}/"
            upload! StringIO.new("#!/bin/sh -e\nexec /usr/local/bin/ssh -o PasswordAuthentication=no -o StrictHostKeyChecking=no \"$@\"\n"), "#{fetch(:tmp_dir)}/#{fetch(:application)}/git-ssh.sh"
            execute :chmod, "+x", "#{fetch(:tmp_dir)}/#{fetch(:application)}/git-ssh.sh"
        end
    end
end

[mattbrictson]

This PR changes /usr/bin/ssh to /usr/bin/env ssh, which seems pretty benign to me. This is something that can't be done with the command-map, because the command in question is inside the git-ssh.sh script that gets uploaded.

So, 👍 from me.

[ferdynator]

👍 Current way, described by @signe is pretty hard and definitely not straight forward.

[will-in-wi]

I think this got lost in the shuffle. If it could be rebased on latest Capistrano, it sounds like we have consensus. This makes sense to me too. The biggest change is that lib/capistrano/tasks/git.rake has moved to lib/capistrano/scm/tasks/git.rake.

@signe, are you interested in doing this? Otherwise, I will.

[will-in-wi]

This PR has been rebased into #2020. Further discussion there.