On Thu, 10 Jun 2021 19:37:12 -0600,
Junio C Hamano wrote:
> Luke Shumaker <lukeshu@xxxxxxxxxxx> 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
