[PATCH] rebase: Fix documentation about used shell in -x

[PATCH] rebase: Fix documentation about used shell in -x

[Nikolay Borisov]

The shell used when using the -x option is the one pointed to by the
SHELL_PATH constant at build time. This erroneous statement in the
documentation sent me on a 10 minute wild goose chase wondering why my
$SHELL was pointing to /bin/bash and my /bin/sh to dash and git was
using dash and not bash.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
 Documentation/git-rebase.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 25516c45d8b8..08cf52daf39e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -964,7 +964,7 @@ non-0 status) to give you an opportunity to fix the problem. You can
 continue with `git rebase --continue`.
 
 The "exec" command launches the command in a shell (the one specified
-in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
+by the build-time SHELL_PATH variable, usually /bin/sh), so you can
 use shell features (like "cd", ">", ";" ...). The command is run from
 the root of the working tree.
 
-- 
2.34.1

Re: [PATCH] rebase: Fix documentation about used shell in -x

[Jeff King]

On Tue, Jan 16, 2024 at 04:18:42PM +0200, Nikolay Borisov wrote:

> The shell used when using the -x option is the one pointed to by the
> SHELL_PATH constant at build time. This erroneous statement in the
> documentation sent me on a 10 minute wild goose chase wondering why my
> $SHELL was pointing to /bin/bash and my /bin/sh to dash and git was
> using dash and not bash.

Good catch. It originally used $SHELL when the documentation was added
in cd035b1cef (rebase -i: add exec command to launch a shell command,
2010-08-10). But that was lost when it was converted to C (which is
perhaps a regression, but nobody seems to have noticed or cared until
now, and at this point we should stick with the new behavior).

(I don't have an exact date since the conversion was somewhat piecemeal,
but it was done by 2018).

Since then, we use the code in run-command.c's prepare_shell_cmd().

> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 25516c45d8b8..08cf52daf39e 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -964,7 +964,7 @@ non-0 status) to give you an opportunity to fix the problem. You can
>  continue with `git rebase --continue`.
>  
>  The "exec" command launches the command in a shell (the one specified
> -in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
> +by the build-time SHELL_PATH variable, usually /bin/sh), so you can
>  use shell features (like "cd", ">", ";" ...). The command is run from
>  the root of the working tree.

Avoiding $SHELL is obviously correct, but I think mentioning SHELL_PATH
is a little hairy. It is not used on Windows; see 776297548e (Do not use
SHELL_PATH from build system in prepare_shell_cmd on Windows, 2012-04-17).
Maybe it makes sense to just say:

  ...in a shell (the default one, usually /bin/sh), ...

It might even make sense to just drop the parenthetical phrase entirely.
Git executes lots of things using a shell, and it is always "the default
one", but we don't bother saying so in most places.

-Peff

Re: [PATCH] rebase: Fix documentation about used shell in -x

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Maybe it makes sense to just say:
>
>   ...in a shell (the default one, usually /bin/sh), ...
>
> It might even make sense to just drop the parenthetical phrase entirely.
> Git executes lots of things using a shell, and it is always "the default
> one", but we don't bother saying so in most places.

Thanks for your archaeological skill, as always.  I like the
deliberate "vagueness" of the above.

Re: [PATCH] rebase: Fix documentation about used shell in -x

[Kristoffer Haugsbakk]

Hi

Some nitpicks since it seems like there will be another round (v2):

> rebase: Fix documentation about used shell in -x

Lower-case “fix” is more conventional.[1]

> SHELL_PATH constant at build time. This erroneous statement in the
> documentation sent me on a 10 minute wild goose chase wondering why my
> $SHELL was pointing to /bin/bash and my /bin/sh to dash and git was
> using dash and not bash.

I think anecdotes are not kept in the commit message, usually? Often they
are put after the three-hyphen/three-dash line. But I didn’t manage to
find any email that says that.

    The shell used when using the -x option is the one pointed to by the
    SHELL_PATH constant at build time.

    Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
    ---
      This erroneous statement in the documentation sent me on a 10 minute
      wild goose chase wondering why my $SHELL was pointing to /bin/bash and
      my /bin/sh to dash and git was using dash and not bash.

     Documentation/git-rebase.txt | 2 +-
     1 file changed, 1 insertion(+), 1 deletion(-)

† 1: SubmittingPatches:

  “ [[summary-section]]
    The title sentence after the "area:" prefix omits the full stop at the
    end, and its first word is not capitalized (the omission
    of capitalization applies only to the word after the "area:"
    prefix of the title)

-- 
Kristoffer Haugsbakk

Re: [PATCH] rebase: Fix documentation about used shell in -x

[Junio C Hamano]

"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

> Hi
>
> Some nitpicks since it seems like there will be another round (v2):
>
>> rebase: Fix documentation about used shell in -x
>
> Lower-case “fix” is more conventional.[1]
>
>> SHELL_PATH constant at build time. This erroneous statement in the
>> documentation sent me on a 10 minute wild goose chase wondering why my
>> $SHELL was pointing to /bin/bash and my /bin/sh to dash and git was
>> using dash and not bash.
>
> I think anecdotes are not kept in the commit message, usually? Often they
> are put after the three-hyphen/three-dash line.
> ...
>     The shell used when using the -x option is the one pointed to by the
>     SHELL_PATH constant at build time.
>
>     Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
>     ---
>       This erroneous statement in the documentation sent me on a 10 minute
>       wild goose chase wondering why my $SHELL was pointing to /bin/bash and
>       my /bin/sh to dash and git was using dash and not bash.
>
>      Documentation/git-rebase.txt | 2 +-
>      1 file changed, 1 insertion(+), 1 deletion(-)

Yup, that looks better.  Here is what I will queue tentatively, with
the improvement suggested by Peff.

----- >8 -----
Subject: [PATCH] rebase: fix documentation about used shell in -x

The shell used when using the -x option is the one pointed to by the
SHELL_PATH constant at build time, not $SHELL environment variable.

We could leave the parenthetical explanation about what shell is
used, but it depends on the build and platform (Windows do not even
use SHELL_PATH build-time knob).  Because Git executes lots of
things using a shell, and it always uses the default shell, it
probably is better to just stop at saying "launches the command in a
shell" without going into more details.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * We could say things like

    - it is a possibility for the future to add how the default
      shell is decided (including the use of SHELL_PATH) in a more
      central part of the doucmentation like git(1)

    - at least not giving a wrong information would prevent the
      future developers wasting time on experimenting various
      settings of the $SHELL variable

   in the log message, if we want.

 Documentation/git-rebase.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index b4526ca246..51489ea686 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -957,8 +957,7 @@ The interactive rebase will stop when a command fails (i.e. exits with
 non-0 status) to give you an opportunity to fix the problem. You can
 continue with `git rebase --continue`.
 
-The "exec" command launches the command in a shell (the one specified
-in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
+The "exec" command launches the command in a shell, so you can
 use shell features (like "cd", ">", ";" ...). The command is run from
 the root of the working tree.