On Sat, Apr 25, 2015 at 06:03:22PM +0200, Torsten Bögershausen wrote: > (I'm not sute if the commit message describes the problem deep enough > for readers which are not familar with all the details of the original > report): > A feature implemented for Windows may break things for e.g. Linux users) > The following may read exaggerated, so please read it as a suggestion. > > The git_connect function has code to handle plink and tortoiseplink > specially, as they require different command line arguments compared to > OpenSSH (-P instead of -p, tortoiseplink uses -batch), commit 36ad53ffee6ed. > The special handling is only needed for Windows, and a sloppy > case-insensitve search for "plink" will trigger that an the extra > parameter "-batch" is added to the command line. > > This was observed on a Linux system where a command line including > "/uplink_deploy/" was used. > > There are different ways to improve the situation: > (The following mentions only plink, but assumes that tortoiseplink is handled > similar) > a) Disable the plink/tortoiseplink special handling on non-Windows systems > b) Tighten the search for plink: > Allow basename() == plink || !!strcasestr(ssh, "plink.exe") > c) Tighten the search for plink: > Allow basename() == plink || !!strcasestr(ssh, "plink.exe") > d) Tighten the check for tortoiseplink. > Today we set "int putty" to true when plink is found, and -batch > is set when tortoiseplink is not found. > This fixes the reported bug, but still has the -P problem. > e) Unix users typically use shell scripts and could use GIT_SSH_COMMAND. > Declare the GIT_SSH as not-well-documented (and to obsolete ?) for non-Windows systems, > > This patch implements c): > Extract the basename and compare it to plink, plink.exe respective > tortoiseplink/tortoiseplink.exe > > Note that there is a slight risk of breakage for Windows users: > Strings like "myplink" or "plink-0.83" are no longer accepted. I can certainly expand the commit message when I reroll. > ------------- > I would probably vote for a), as Unix/Linux/Mac OS users don't use plink/tortoiseplink > at all. I have putty on my system: vauxhall ok % uname -a && which plink Linux vauxhall 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt7-1 (2015-03-01) x86_64 GNU/Linux /usr/bin/plink While it's clearly not very common to use putty on Unix systems, it certainly is possible and it would need to work the same way. > ------------- > What about adding test-cases in t5601, > this will ease the documentation later. > f:/util/plink > /c/util/plink.exe > f:/util/tortoiseplink > /c/util/tortoiseplink.exe > /usr/local/uplink/sshwrapper.sh It looks like there's already a framework for that, so sure, I can add some tests. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Attachment:
signature.asc
Description: Digital signature