On 2015-04-25 00.28, brian m. carlson wrote: > The git_connect function has code to handle plink and tortoiseplink > specially, as they require different command line arguments from > OpenSSH. However, the match was done by checking for "plink" > case-insensitively in the string, which led to false positives when > GIT_SSH contained "uplink". Improve the check by looking for "plink" or > "tortoiseplink" (or those names suffixed with ".exe") in the final > component of the path. > (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 would probably vote for a), as Unix/Linux/Mac OS users don't use plink/tortoiseplink at all. ------------- 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 Other opinions, other thoughts ? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html