Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true

2015-04-23 Thread Patrick Sharp
Torsten,

The relevant part of the path in GIT_SSH was ‘/uplink_deploy/‘.

I did begin to use GIT_SSH_COMMAND as a workaround, but regardless this still 
feels like an overly broad way to determine the value of the putty flag.

I was kind of surprised to find it being inferred from the value of GIT_SSH 
instead of being explicitly set by some additional variable.  GIT_USE_PUTTY or 
some such, though I can understand there may be some reluctance to put the onus 
of that on the end user.

Part of the issue stemmed from not being able to find any documentation on 
this.  After I discovered what was happening I found plenty of instructions 
that indicated to enable putty support set GIT_SSH to /path/to/plink.exe, but I 
didn’t find it stated anywhere that if ‘plink’ was found in GIT_SSH, then git 
will add the -batch option to the command args.  In other words, I was able to 
find instructions on what to do if we had been using putty, but not 
instructions on unexpected behavior if using GIT_SHH while NOT using putty.


> On Apr 23, 2015, at 12:08 AM, Torsten Bögershausen  wrote:
> 
> On 04/22/2015 09:12 PM, Patrick Sharp wrote:
>> Johannes,
>> 
>> You’re correct, looking back over it, I was pretty vague.
>> 
>> In truth, we are not using Windows OR putty at all.  Running git on an 
>> Ubuntu system, but we are setting the GIT_SSH environment variable to point 
>> to a shell script to use.
>> 
>> Upon attempting to run git ls-remote, the system was spitting out 
>> getaddrinfo errors for ‘atch’ .
>> 
>> Setting GIT_TRACE=2 showed that -batch was being added to the git command.
>> 
>> This was seen on several different servers with git versions 1.8.5.2, 1.9.1 
>> and 2.3.5
>> 
>> After a bit we realized that it was the string ‘uplink’ in the GIT_SSH 
>> variable that was linked to the extra -batch flag.
>> 
>> Finally, after searching the git source, we narrowed it down to the ‘plink’ 
>> portion of the string.
>> 
>> https://github.com/git/git/blob/7c597ef345aed345576de616c51f27e6f4b342b3/connect.c#L747-L7
> Brian, I got your patch,
> but can't see it in the list yet
> 1/2 looks good, thanks.
> (And add msygit)
> 
> My feeling is that  patch 2/2 may break things for an unknown
> amount of users which e.g. use "myplink".
> 
> Patrick,
> did you ever tell us, what you put into $GIT_SSH ?
> 
> Can your use case be covered by using $GIT_SSH_COMMAND ?
> 
> 
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true

2015-04-22 Thread Patrick Sharp
Johannes,

You’re correct, looking back over it, I was pretty vague.

In truth, we are not using Windows OR putty at all.  Running git on an Ubuntu 
system, but we are setting the GIT_SSH environment variable to point to a shell 
script to use.

Upon attempting to run git ls-remote, the system was spitting out getaddrinfo 
errors for ‘atch’ .

Setting GIT_TRACE=2 showed that -batch was being added to the git command.

This was seen on several different servers with git versions 1.8.5.2, 1.9.1 and 
2.3.5

After a bit we realized that it was the string ‘uplink’ in the GIT_SSH variable 
that was linked to the extra -batch flag.

Finally, after searching the git source, we narrowed it down to the ‘plink’ 
portion of the string.

https://github.com/git/git/blob/7c597ef345aed345576de616c51f27e6f4b342b3/connect.c#L747-L756


> On Apr 22, 2015, at 12:46 PM, Johannes Schindelin 
>  wrote:
> 
> Hi Patrick,
> 
> On 2015-04-22 16:36, Patrick Sharp wrote:
>> The plink string detection in GIT_SSH for setting putty to true is very 
>> broad.
> 
> Wow. You probably wanted to state that you are using Windows, downloaded Git 
> from [link here], that you are using [version] and that you use PLink 
> [version] (from the Putty package downloaded [link here]) to do your ssh 
> business.
> 
> Without that information, you leave readers who have no idea about Putty 
> *quite* puzzled.
> 
>> If plink is anywhere in the path to the shell file then putty gets set
>> to true and ssh will fail trying to parse -batch as the hostname.
> 
> This is cryptic even for me.
> 
>> Wouldn’t searching for plink.exe be better?--
> 
> I invite you to try your hand at improving anything you find flawed. For 
> example, if you want to improve the PLink detection in Git for Windows 1.x, 
> this would be the correct place to start: 
> https://github.com/msysgit/msysgit/blob/70f24b4b0f5f86a5e85f7264a4ee2c0fec2d4391/share/WinGit/install.iss#L232-L253
>  (yes, you would have to download the development environment from 
> https://msysgit.github.com/#download-msysgit and rebuild your own installer 
> using `/share/msysGit/WinGit/release.sh 1.9.5-patrick` after editing the 
> installer script).
> 
> Ciao,
> Johannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true

2015-04-22 Thread Patrick Sharp
The plink string detection in GIT_SSH for setting putty to true is very broad.

If plink is anywhere in the path to the shell file then putty gets set to true 
and ssh will fail trying to parse -batch as the hostname.

Wouldn’t searching for plink.exe be better?--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html