Re: [PATCH 1/3] var: add support for listing the shell

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 22, 2023, 20:42 UTC
Message-ID: <CAPig+cSTR6oHeYjcHZ7m2CKYcFo2eistxz_X-7J2rhd7h+uf3g@mail.gmail.com>
In-Reply-To: <20230622195059.320593-2-sandals@crustytoothpaste.net>

On Thu, Jun 22, 2023 at 4:03 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 27 quoted lines

> On most Unix systems, finding a suitable shell is easy: one simply uses
> "sh" with an appropriate PATH value.  However, in many Windows
> environments, the shell is shipped alongside Git, and it may or may not
> be in PATH, even if Git is.
>
> In such an environment, it can be very helpful to query Git for the
> shell it's using, since other tools may want to use the same shell as
> well.  To help them out, let's add a variable, GIT_SHELL_PATH, that
> points to the location of the shell.
>
> On Unix, we know our shell must be executable to be functional, so
> assume that the distributor has correctly configured their environment,
> and use that as a basic test.  On Git for Windows, we know that our
> shell will be one of a few fixed values, all of which end in "sh" (such
> as "bash").  This seems like it might be a nice test on Unix as well,
> since it is customary for all shells to end in "sh", but there probably
> exist such systems that don't have such a configuration, so be careful
> here not to break them.
>
> Signed-off-by: brian m. carlson <bk2204@github.com>
> ---
> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
> @@ -147,6 +147,18 @@ test_expect_success 'get GIT_SEQUENCE_EDITOR with configuration and environment
> +test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' '
> +       git var GIT_SHELL_PATH >shell &&
> +       test -x "$(cat shell)"
> +'

This can be implemented more simply without a temporary file:

    shpath=$(git var GIT_SHELL_PATH) &&
    test -x "$shpath"

This is safe since the exit code of the Git command is preserved across the `shpath=...` assignment.

Show 6 quoted lines

> +# We know in this environment that our shell will be one of a few fixed values
> +# that all end in "sh".
> +test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' '
> +       git var GIT_SHELL_PATH >shell &&
> +       grep "sh\$" shell
> +'

Similarly, there is no need for a temporary file or an extra process. This can all be done entirely in the shell itself:

    shpath=$(git var GIT_SHELL_PATH) &&
    case "$shpath" in
    *sh) ;;
    *) return 1
    esac