git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Re: [PATCH bc/connect-plink] t5601-clone: remove broken and pointless check for plink.exe

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Aug 13, 2015, 07:30 UTC
Message-ID
<43f88e9611755e20715bf9f38795f276@www.dscho.org>
In-Reply-To
<55CB9110.4060005@kdbg.org>
Hi Johannes,
On 2015-08-12 20:31, Johannes Sixt wrote:
Show 19 quoted lines
> Am 12.08.2015 um 13:58 schrieb Erik Faye-Lund:
>> On Wed, Aug 12, 2015 at 1:07 PM, Johannes Schindelin
>> <johannes.schindelin@gmx.de> wrote:
>>> FWIW Git for Windows has this patch (that I wanted to contribute
>>> in  due time, what with being busy with all those tickets) to solve the
>>> problem mentioned in your patch in a different way:
>>>
>>> https://github.com/git-for-windows/git/commit/2fff4b54a0d4e5c5e2e4638c9b0739d3c1ff1e45
>>
>> Yuck. On Windows, it's the extension of a file that dictates what kind
>> of file it is (and if it's executable or not), not the contents. If we
>> get a shell script written with the ".exe"-prefix, it's considered as
>> an invalid executable by the system. We should consider it the same
>> way, otherwise we're on the path to user-experience schizophrenia.
>>
>> I'm not sure I consider this commit a step in the right direction.
> 
> I, too, think that it is a wrong decision to pessimize git for the
> sake of a single test case.
Oh, you make it sound as if you believe that I had indeed weakened Git *just* for a single test case.
That is quite a strong assumption, and could not be further from the truth, though, I have to point out. It is important to keep in mind that we (actually, IIRC it was you) taught Git to recognize shell scripts when executing external programs *because Windows does not do that for us*. So yes, we are deviating from the standard Windows way of things, and we do that quite intentionally so.
Now, let's look at the test case for a moment and let's try to understand the technique it uses (that breaks the test case currently). It puts a script in place of an `.exe`, with the intention to execute the script instead of the original executable.
This technique is an age-old technique on Unix, and it just works. There are a range of valid reasons, from debugging to slightly modifying the function of a particular `.exe` (possibly renaming the original `.exe` and calling it from the script) in the easiest way: by scripting on top of it.
If we want to allow such a thing -- allowing users to use scripts to modify the behavior of executables -- then we *have* to allow `.exe` suffixes for scripts, because that happens to be the suffix of executables on Windows.
I guess you would have had an easier time to understand my thinking if I had replaced the sentence
    So the assumption that the `.exe` extension implies that the file is *not* a shell script is now wrong.
by
    So this is a strong indicator that it was wrong to assume that `.exe` extensions imply that the file is *not* a shell script.
Further, I even looked at the performance impact, but that is at least well documented in the commit message.
I also have to point out that the alternative "solution" presented by your patch -- to disable the test case -- is no solution at all: the very platform that is most likely to have plink users is Windows. And to *exclude* that platform from running that unit test is questionable at best.

Ciao, Johannes

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Previous: Johannes SixtNext: Johannes Sixt
Message 9 of 12 in “t5601-clone: remove broken and pointless check for plink.exe”
  1. t5601-clone: remove broken and pointless check for plink.exeJohannes Sixt, Aug 11, 2015
  2. Eric SunshineAug 11, 2015
  3. Junio C HamanoAug 11, 2015
  4. Junio C HamanoAug 11, 2015
  5. Eric SunshineAug 11, 2015
  6. Johannes SchindelinAug 12, 2015
  7. Erik Faye-LundAug 12, 2015
  8. Johannes SixtAug 12, 2015
  9. Johannes SchindelinAug 13, 2015
  10. Johannes SixtAug 13, 2015
  11. Johannes SchindelinAug 13, 2015
  12. Erik Faye-LundAug 13, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.