update search path for 'ssh.exe' #743
Conversation
|
This LGTM assuming it works on windows. In testing, lets use git from the link we recommend to ensure that things work with that new version of git-for-windows. |
sallyom
force-pushed
the
bz1294401
branch
3 times, most recently
from
ed8e9fd
to
a03c5c8
Compare
|
@tiwillia I have confirmed that this fix works in Windows. |
|
@sallyom, this looks great. Adding each directory in $PATH to the list of paths to search was an excellent idea, I'm surprised that was not already implemented. Lets [test] this before we merge. |
|
[merge] please! |
|
@tiwillia it was a great idea ;) My awesome friend, @DirectXMan12 suggested it....he sits next to us. |
| @@ -461,9 +461,11 @@ def discover_ssh_executable | |||
| # looks for ssh.exe from msysgit or plink.exe from PuTTY, either on path or specific locations | |||
| guessing_locations << | |||
| discover_windows_executables do |base| | |||
| [ | |||
| from_path = ENV['PATH'].split(File::PATH_SEPARATOR).map {|p| p + File::ALT_SEPARATOR + "ssh.exe" } | |||
There was a problem hiding this comment.
Wouldn't it make more sense to add ENV['PATH'].split(File::PATH_SEPARATOR) to base_files in discover_windows_executables?
sallyom
force-pushed
the
bz1294401
branch
3 times, most recently
from
dbd1eb7
to
684dd52
Compare
|
@Miciah, woohoo, i confirmed this fix works in Windows! |
|
@Miciah, per our conversation, without re-writing the whole discover_windows_executables I believe this is the best fix? It will do a few extra searches but won't impact any other part of the code. |
| guessing_locations << | ||
| discover_windows_executables do |base| | ||
| [ | ||
| from_files = ENV['PATH'].split(File::PATH_SEPARATOR).map {|p| p + File::ALT_SEPARATOR + 'ssh.exe'} |
There was a problem hiding this comment.
We lost the indentation here.
|
Yeah, I'm micro-irked by the extra searches this will incur, but it probably ultimately is the simplest way to fix the problem. Sorry for all the extra effort it took to come to that conclusion. |
bz 1294401 bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1294401 'rhc ssh <app>' fails with Windows 10 Home Edition because rhc does not recognize 'C:\Program Files\Git\usr\bin\ssh.exe' as a possible location for 'ssh.exe'. Updated to add this path. Also, updated the Git install site recommendation since the current site is obsolete.
|
Please [merge]! |
|
Evaluated for online test up to 390a0af |
|
Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/6698/) (Image: devenv_5760) |
|
Evaluated for online merge up to 390a0af |
|
Online Test Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/test_pull_requests/9174/) |
Merged by openshift-bot
bz 1294401
bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1294401
'rhc ssh ' fails with Windows 10 Home Edition because rhc does
not recognize 'C:\Program Files\Git\usr\bin\ssh.exe' as a possible
location for 'ssh.exe'. Updated to add this path. Also, updated the
Git install site recommendation since the current site is obsolete.