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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 11, 2021, 01:37 UTC
Message-ID: <xmqqim2l5ghj.fsf@gitster.g>
In-Reply-To: <87bl8d6xoq.wl-lukeshu@lukeshu.com>

Luke Shumaker <lukeshu@lukeshu.com> writes:

Show 11 quoted lines

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

> Do we also need to handle the reverse case, where PATH uses
> backslashes but GIT_EXEC_PATH uses forward slashes?