Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "András Kucsma via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Torsten Bögershausen" <tboegi@web.de>,
	"András Kucsma" <r0maikx02b@gmail.com>
Subject: Re: [PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows
Date: Thu, 26 Mar 2020 14:14:31 -0700
Message-ID: <xmqqmu82izt4.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <pull.587.v2.git.1585143910604.gitgitgadget@gmail.com> (=?utf-8?Q?=22Andr=C3=A1s?= Kucsma via GitGitGadget"'s message of "Wed, 25 Mar 2020 13:45:10 +0000")

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

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

  parent 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 [this message]
2020-03-27  0:21     ` András Kucsma
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=xmqqmu82izt4.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=r0maikx02b@gmail.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