Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: "András Kucsma" <r0maikx02b@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "András Kucsma via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "Torsten Bögershausen" <tboegi@web.de>
Subject: Re: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows
Date: Fri, 27 Mar 2020 01:21:33 +0100
Message-ID: <CANPdQvJk4kJSMitaV_cEk-xY3_a=o1VN4fAqFCPiXWJf2ahScw@mail.gmail.com> (raw)
In-Reply-To: <xmqqmu82izt4.fsf@gitster.c.googlers.com>

On Thu, Mar 26, 2020 at 10:14 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "András Kucsma via GitGitGadget"  <gitgitgadget@gmail.com> writes:
>
> > 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.

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.

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-23 21:13 [PATCH] " András Kucsma via GitGitGadget
2020-03-24 20:51 ` Junio C Hamano
2020-03-25 13:45 ` [PATCH v2] " András Kucsma via GitGitGadget
2020-03-25 16:35   ` Torsten Bögershausen
2020-03-25 17:09     ` András Kucsma
2020-03-26 21:16     ` Junio C Hamano
2020-03-26 21:14   ` Junio C Hamano
2020-03-27  0:21     ` András Kucsma [this message]
2020-03-27  0:36   ` [PATCH v3] run-command: trigger PATH lookup properly on Cygwin András Kucsma via GitGitGadget
2020-03-27 18:04     ` Junio C Hamano
2020-03-27 18:10       ` András Kucsma
2020-03-27 18:41       ` Andreas Schwab
2020-03-27 21:27         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANPdQvJk4kJSMitaV_cEk-xY3_a=o1VN4fAqFCPiXWJf2ahScw@mail.gmail.com' \
    --to=r0maikx02b@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git