On Thu, Mar 26, 2020 at 10:14 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "András Kucsma via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > From: Andras Kucsma <r0maikx02b@xxxxxxxxx> > > > > On Windows with git installed through cygwin, GIT_ASKPASS failed to run > > for relative and absolute paths containing only backslashes as directory > > separators. > > > Subject: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows > > Isn't it curious that there is nothing in the code that was touched > that is specific to GIT_ASKPASS? We shouldn't have to see that in > the title. You're completely right, I'll rephrase the commit message based on your suggestion and resubmit. > Having said all that, I am not sure if we need to change anything. > > As Cygwin is about trying to mimicking UNIXy environment as much as > possible, shouldn't "GIT_ASKPASS=//c/program files/askpass" the way > end-users would expect to work, not the one that uses backslashes? > > And if the user pretends to be on UNIXy system by using Cygwin by > using slashes when specifying these commands run via the run_command > API, the code makes the decision for PATH-lookup quite correctly, > no? > > So... Cygwin provides a Unix like environment, while also maintaining Windows compatibility, at least as far as path handling is concerned. As a quick test, fopen can handle forward slashes, backslashes too. These four all work under cygwin: fopen("C:\\file.txt", "r"); fopen("C:/file.txt", "r"); fopen("/cygdrive/c/file.txt", "r"); fopen("/cygdrive\\c\\file.txt", "r"); There seems to be a precedent to support Cygwin as a kind of "hybrid" platform in the git codebase. In git-compat-util.h, the compat/win32/path-utils.h header is included, but GIT_WINDOWS_NATIVE is not defined. https://github.com/git/git/blob/a7d14a442/git-compat-util.h#L204-L206 https://github.com/git/git/blob/a7d14a442/git-compat-util.h#L157-L165 The compat/win32/path-utils.h header mostly provides utilities dealing with directory separator related logic on Windows, but these utilities are being used in the Unixy code paths on Cygwin. The current version of the patch fits into this pattern. It only changes behaviour under Cygwin, not touching pure Windows and non-Cygwin Unix variants at all.