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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 26, 2020, 21:14 UTC
Message-ID: <xmqqmu82izt4.fsf@gitster.c.googlers.com>
In-Reply-To: <pull.587.v2.git.1585143910604.gitgitgadget@gmail.com>

"András Kucsma via GitGitGadget"  <gitgitgadget@gmail.com> writes:

Show 5 quoted lines

> From: Andras Kucsma <r0maikx02b@gmail.com>
>
> 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.

Perhaps

    Subject: run-command: notice needs for PATH-lookup correctly on Cygwin

    On Cygwin, the codepath for POSIX-like systems is taken in
    run-command.c::start_command().  The prepare_cmd() helper
    function is called to decide if the command needs to be looked
    up in the $PATH, and the logic there is to do the PATH-lookup if
    and only if it does not have any slash '/' in it.

    Unfortunately, a end-user can give "c:\program files\askpass" or
    "a\b\c" to be absolute or relative path to the command, but in
    these strings there is no '/'.  We end up attempting to run the
    command by appending the absoluter or relative path after each
    colon-separated component of $PATH.

    Instead, introduce a has_dir_sep(path) helper function to
    abstract away the difference between true POSIX and Cygwin, and
    use it to make the decision for PATH-lookup.

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...