{"thread":{"id":"60750","subject":"[PATCH] rebase: Fix documentation about used shell in -x","startedAt":"2024-01-16T14:18:47Z","lastAt":"2024-01-17T22:35:12Z","messageCount":5,"participants":["Nikolay Borisov","Jeff King","Kristoffer Haugsbakk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"486838","messageId":"20240116141842.193151-1-nik.borisov@suse.com","threadId":"60750","inReplyTo":null,"subject":"[PATCH] rebase: Fix documentation about used shell in -x","fromName":"Nikolay Borisov","fromEmail":"nik.borisov@suse.com","sentAt":"2024-01-16T14:18:42Z","receivedAt":"2024-01-16T14:18:47Z","isPatch":true,"sender":{"key":"nik.borisov@suse.com","avatar":null},"body":"The shell used when using the -x option is the one pointed to by the\nSHELL_PATH constant at build time. This erroneous statement in the\ndocumentation sent me on a 10 minute wild goose chase wondering why my\n$SHELL was pointing to /bin/bash and my /bin/sh to dash and git was\nusing dash and not bash.\n\nSigned-off-by: Nikolay Borisov \n---\n Documentation/git-rebase.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 25516c45d8b8..08cf52daf39e 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -964,7 +964,7 @@ non-0 status) to give you an opportunity to fix the problem. You can\n continue with `git rebase --continue`.\n \n The \"exec\" command launches the command in a shell (the one specified\n-in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n+by the build-time SHELL_PATH variable, usually /bin/sh), so you can\n use shell features (like \"cd\", \">\", \";\" ...). The command is run from\n the root of the working tree.\n \n-- \n2.34.1\n\n"},{"id":"486839","messageId":"20240116143757.GA2119690@coredump.intra.peff.net","threadId":"60750","inReplyTo":"20240116141842.193151-1-nik.borisov@suse.com","subject":"Re: [PATCH] rebase: Fix documentation about used shell in -x","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-01-16T14:37:57Z","receivedAt":"2024-01-16T14:38:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 16, 2024 at 04:18:42PM +0200, Nikolay Borisov wrote:\n\n> The shell used when using the -x option is the one pointed to by the\n> SHELL_PATH constant at build time. This erroneous statement in the\n> documentation sent me on a 10 minute wild goose chase wondering why my\n> $SHELL was pointing to /bin/bash and my /bin/sh to dash and git was\n> using dash and not bash.\n\nGood catch. It originally used $SHELL when the documentation was added\nin cd035b1cef (rebase -i: add exec command to launch a shell command,\n2010-08-10). But that was lost when it was converted to C (which is\nperhaps a regression, but nobody seems to have noticed or cared until\nnow, and at this point we should stick with the new behavior).\n\n(I don't have an exact date since the conversion was somewhat piecemeal,\nbut it was done by 2018).\n\nSince then, we use the code in run-command.c's prepare_shell_cmd().\n\n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index 25516c45d8b8..08cf52daf39e 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -964,7 +964,7 @@ non-0 status) to give you an opportunity to fix the problem. You can\n> continue with `git rebase --continue`.\n> \n> The \"exec\" command launches the command in a shell (the one specified\n> -in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n> +by the build-time SHELL_PATH variable, usually /bin/sh), so you can\n> use shell features (like \"cd\", \">\", \";\" ...). The command is run from\n> the root of the working tree.\n\nAvoiding $SHELL is obviously correct, but I think mentioning SHELL_PATH\nis a little hairy. It is not used on Windows; see 776297548e (Do not use\nSHELL_PATH from build system in prepare_shell_cmd on Windows, 2012-04-17).\nMaybe it makes sense to just say:\n\n ...in a shell (the default one, usually /bin/sh), ...\n\nIt might even make sense to just drop the parenthetical phrase entirely.\nGit executes lots of things using a shell, and it is always \"the default\none\", but we don't bother saying so in most places.\n\n-Peff\n"},{"id":"486850","messageId":"b491d954-b1a4-4000-95fb-fc83bf815edc@app.fastmail.com","threadId":"60750","inReplyTo":"20240116141842.193151-1-nik.borisov@suse.com","subject":"Re: [PATCH] rebase: Fix documentation about used shell in -x","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2024-01-16T16:50:56Z","receivedAt":"2024-01-16T16:51:58Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi\n\nSome nitpicks since it seems like there will be another round (v2):\n\n> rebase: Fix documentation about used shell in -x\n\nLower-case “fix” is more conventional.[1]\n\n> SHELL_PATH constant at build time. This erroneous statement in the\n> documentation sent me on a 10 minute wild goose chase wondering why my\n> $SHELL was pointing to /bin/bash and my /bin/sh to dash and git was\n> using dash and not bash.\n\nI think anecdotes are not kept in the commit message, usually? Often they\nare put after the three-hyphen/three-dash line. But I didn’t manage to\nfind any email that says that.\n\n The shell used when using the -x option is the one pointed to by the\n SHELL_PATH constant at build time.\n\n Signed-off-by: Nikolay Borisov \n ---\n This erroneous statement in the documentation sent me on a 10 minute\n wild goose chase wondering why my $SHELL was pointing to /bin/bash and\n my /bin/sh to dash and git was using dash and not bash.\n\n Documentation/git-rebase.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\n† 1: SubmittingPatches:\n\n “ [[summary-section]]\n The title sentence after the \"area:\" prefix omits the full stop at the\n end, and its first word is not capitalized (the omission\n of capitalization applies only to the word after the \"area:\"\n prefix of the title)\n\n-- \nKristoffer Haugsbakk\n"},{"id":"486855","messageId":"xmqq5xzt17py.fsf@gitster.g","threadId":"60750","inReplyTo":"20240116143757.GA2119690@coredump.intra.peff.net","subject":"Re: [PATCH] rebase: Fix documentation about used shell in -x","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-16T17:58:01Z","receivedAt":"2024-01-16T17:58:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Maybe it makes sense to just say:\n>\n> ...in a shell (the default one, usually /bin/sh), ...\n>\n> It might even make sense to just drop the parenthetical phrase entirely.\n> Git executes lots of things using a shell, and it is always \"the default\n> one\", but we don't bother saying so in most places.\n\nThanks for your archaeological skill, as always. I like the\ndeliberate \"vagueness\" of the above.\n\n"},{"id":"486942","messageId":"xmqqv87rtwq2.fsf@gitster.g","threadId":"60750","inReplyTo":"b491d954-b1a4-4000-95fb-fc83bf815edc@app.fastmail.com","subject":"Re: [PATCH] rebase: Fix documentation about used shell in -x","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-17T22:35:01Z","receivedAt":"2024-01-17T22:35:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n> Hi\n>\n> Some nitpicks since it seems like there will be another round (v2):\n>\n>> rebase: Fix documentation about used shell in -x\n>\n> Lower-case “fix” is more conventional.[1]\n>\n>> SHELL_PATH constant at build time. This erroneous statement in the\n>> documentation sent me on a 10 minute wild goose chase wondering why my\n>> $SHELL was pointing to /bin/bash and my /bin/sh to dash and git was\n>> using dash and not bash.\n>\n> I think anecdotes are not kept in the commit message, usually? Often they\n> are put after the three-hyphen/three-dash line.\n> ...\n> The shell used when using the -x option is the one pointed to by the\n> SHELL_PATH constant at build time.\n>\n> Signed-off-by: Nikolay Borisov \n> ---\n> This erroneous statement in the documentation sent me on a 10 minute\n> wild goose chase wondering why my $SHELL was pointing to /bin/bash and\n> my /bin/sh to dash and git was using dash and not bash.\n>\n> Documentation/git-rebase.txt | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nYup, that looks better. Here is what I will queue tentatively, with\nthe improvement suggested by Peff.\n\n----- >8 -----\nSubject: [PATCH] rebase: fix documentation about used shell in -x\n\nThe shell used when using the -x option is the one pointed to by the\nSHELL_PATH constant at build time, not $SHELL environment variable.\n\nWe could leave the parenthetical explanation about what shell is\nused, but it depends on the build and platform (Windows do not even\nuse SHELL_PATH build-time knob). Because Git executes lots of\nthings using a shell, and it always uses the default shell, it\nprobably is better to just stop at saying \"launches the command in a\nshell\" without going into more details.\n\nSigned-off-by: Nikolay Borisov \nHelped-by: Jeff King \nHelped-by: Kristoffer Haugsbakk \nSigned-off-by: Junio C Hamano \n---\n\n * We could say things like\n\n - it is a possibility for the future to add how the default\n shell is decided (including the use of SHELL_PATH) in a more\n central part of the doucmentation like git(1)\n\n - at least not giving a wrong information would prevent the\n future developers wasting time on experimenting various\n settings of the $SHELL variable\n\n in the log message, if we want.\n\n Documentation/git-rebase.txt | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex b4526ca246..51489ea686 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -957,8 +957,7 @@ The interactive rebase will stop when a command fails (i.e. exits with\n non-0 status) to give you an opportunity to fix the problem. You can\n continue with `git rebase --continue`.\n \n-The \"exec\" command launches the command in a shell (the one specified\n-in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n+The \"exec\" command launches the command in a shell, so you can\n use shell features (like \"cd\", \">\", \";\" ...). The command is run from\n the root of the working tree.\n \n"}]}