Re: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux