Re: [PATCH 1/2] subtree: fix the GIT_EXEC_PATH sanity check to work on Windows

From: Luke Shumaker <lukeshu@lukeshu.com>
Date: Jun 11, 2021, 04:31 UTC
Message-ID: <877dj16n02.wl-lukeshu@lukeshu.com>
In-Reply-To: <xmqqim2l5ghj.fsf@gitster.g>

On Thu, 10 Jun 2021 19:37:12 -0600, Junio C Hamano wrote:

Show 20 quoted lines

> Luke Shumaker <lukeshu@lukeshu.com> writes:
> 
> >> +if test -z "$GIT_EXEC_PATH" || {
> >> +	test "${PATH#"${GIT_EXEC_PATH}:"}" = "$PATH" && {
> >> +		# On Windows, PATH might be Unix-style, GIT_EXEC_PATH not
> >> +		! type -p cygpath >/dev/null 2>&1 ||
> >> +		test "${PATH#$(cygpath -au "$GIT_EXEC_PATH"):}" = "$PATH"
> >
> > Nit: That should have a couple more `"` in it:
> >
> >     test "${PATH#"$(cygpath -au "$GIT_EXEC_PATH"):"}" = "$PATH"
> >
> > But no need to re-roll for just that.
> 
> Does the nit purely cosmetic, or does it affect correctness?  I'd
> assume the former, as it would not make sense to say "no need to
> reroll" if leaving it as-is would mean a breakage, but trying to
> make sure.
> 
> Thanks.

Quoting in that shell construct can in theory affect correctness, but my intuition was that it can't affect this specific case. However, upon thinking about it a bit more, I realize that I was mistaken. If the git exec path contains a shell glob that is not self-matching, then it will break in that `git subtree` will refuse to run even though the install is fine.

For instance,

    GIT_EXEC_PATH => \Home\Tricky[x]User\git-core
    PATH          => /Home/Tricky[x]User/git-core:/bin

I'd think that this type of thing would be unlikely to happen in the wild, but yeah, it needs to be fixed. So a reroll is needed.

It is also broken in the other direction (it might run even though the install is broken), but only in situations that I don't think I actually care about. It would happen if the glob is self-matching, your GIT_EXEC_PATH and PATH disagree, and the glob matches PATH. The point of the check is to look for ways that installs are actually accidentally broken in the wild, I'm pretty sure the only way all 3 of those things can happen together is if you're actively trying to break it. And if you're actively trying to break a shell script, you can do so a lot simpler by just setting PATH to something silly.

-- 
Happy hacking,
~ Luke Shumaker