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, 00:40 UTC
- Message-ID
- <87bl8d6xoq.wl-lukeshu@lukeshu.com>
- In-Reply-To
- <a91ac6c18938116c4a74e19466da456b67376fa5.1623316412.git.gitgitgadget@gmail.com>
On Thu, 10 Jun 2021 03:13:30 -0600, Johannes Schindelin via GitGitGadget wrote:
Show 40 quoted lines
>
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In 22d550749361 (subtree: don't fuss with PATH, 2021-04-27), `git
> subtree` was broken thoroughly on Windows.
>
> The reason is that it assumes Unix semantics, where `PATH` is
> colon-separated, and it assumes that `$GIT_EXEC_PATH:` is a verbatim
> prefix of `$PATH`. Neither are true, the latter in particular because
> `GIT_EXEC_PATH` is a Windows-style path, while `PATH` is a Unix-style
> path list.
>
> Let's keep the original spirit, and hack together something that
> unbreaks the logic on Windows.
>
> A more thorough fix would look at the inode of `$GIT_EXEC_PATH` and of
> the first component of `$PATH`, to make sure that they are identical,
> but that is even trickier to do in a portable way.
>
> This fixes https://github.com/git-for-windows/git/issues/3260
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> contrib/subtree/git-subtree.sh | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index b06782bc7955..6bd689a6bb92 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -5,7 +5,13 @@
> # Copyright (C) 2009 Avery Pennarun <apenwarr@gmail.com>
> #
>
> -if test -z "$GIT_EXEC_PATH" || test "${PATH#"${GIT_EXEC_PATH}:"}" = "$PATH" || ! test -f "$GIT_EXEC_PATH/git-sh-setup"
> +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.
Do we also need to handle the reverse case, where PATH uses backslashes but GIT_EXEC_PATH uses forward slashes?
-- Happy hacking, ~ Luke Shumaker