Re: [PATCH] var(win32): do report the GIT_SHELL_PATH that is actually used

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 8, 2024, 19:03 UTC
Message-ID: <xmqq7cdvg105.fsf@gitster.g>
In-Reply-To: <pull.1760.git.1720443778074.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 16 quoted lines

> diff --git a/builtin/var.c b/builtin/var.c
> index 5dc384810c0..f4b1f34e403 100644
> --- a/builtin/var.c
> +++ b/builtin/var.c
> @@ -51,7 +51,13 @@ static char *default_branch(int ident_flag UNUSED)
>  
>  static char *shell_path(int ident_flag UNUSED)
>  {
> +#ifdef WIN32
> +	char *p = locate_in_PATH("sh");
> +	convert_slashes(p);
> +	return p;
> +#else
>  	return xstrdup(SHELL_PATH);
> +#endif
>  }

We have done well without any ugly conditional compilation in this file so far. If we needed to report dynamic path on certain platforms, we'd need something a bit cleaner, like

 * rename "shell_path()" to something that is unlikely to conflict
   with others (e.g., "git_shell_path()") and make it extern

 * allow it to be overridden from compat/

which is often how we hide such an implementation quirk from the general code paths.

Show 16 quoted lines

>  static char *git_attr_val_system(int ident_flag UNUSED)
> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
> index ff4fd9348cc..9fc58823873 100755
> --- a/t/t0007-git-var.sh
> +++ b/t/t0007-git-var.sh
> @@ -157,7 +157,7 @@ test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' '
>  test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '
>  	shellpath=$(git var GIT_SHELL_PATH) &&
>  	case "$shellpath" in
> -	*sh) ;;
> +	[A-Z]:/*/sh.exe) test -f "$shellpath";;
>  	*) return 1;;
>  	esac
>  '
>
> base-commit: 06e570c0dfb2a2deb64d217db78e2ec21672f558