{"thread":{"id":"53162","subject":"[PATCH] git-rebase.txt: add another hook to the hooks section, and explain more","startedAt":"2020-04-03T04:35:26Z","lastAt":"2020-04-05T00:00:23Z","messageCount":6,"participants":["Elijah Newren via GitGitGadget","Junio C Hamano","Elijah Newren","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"394638","messageId":"pull.749.git.git.1585888520572.gitgitgadget@gmail.com","threadId":"53162","inReplyTo":null,"subject":"[PATCH] git-rebase.txt: add another hook to the hooks section, and explain more","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-03T04:35:20Z","receivedAt":"2020-04-03T04:35:26Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nFor more discussion about these hooks, their history relative to rebase,\nand logical consistency between different types of operations, see\n https://lore.kernel.org/git/CABPp-BG0bFKUage5cN_2yr2DkmS04W2Z9Pg5WcROqHznV3XBdw@mail.gmail.com/\nand the links to some threads referenced therein.\n\nSigned-off-by: Elijah Newren \n---\n git-rebase.txt: add another hook to the hooks section, and explain more\n \n I'm particularly interested in opinions on the last sentence added to\n the manpage, which is based on the thread mentioned in the commit\n message.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-749%2Fnewren%2Frebase-and-hooks-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-749/newren/rebase-and-hooks-v1\nPull-Request: https://github.com/git/git/pull/749\n\n Documentation/git-rebase.txt | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex f7a6033607f..5c518ca0ccf 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -684,9 +684,15 @@ Hooks\n ~~~~~\n \n The apply backend has not traditionally called the post-commit hook,\n-while the merge backend has. However, this was by accident of\n-implementation rather than by design. Both backends should have the\n-same behavior, though it is not clear which one is correct.\n+while the merge backend has. In contrast, the apply backend has\n+traditionally called the post-checkout hook while the merge backend\n+has not. However, the calling of these hooks in both cases was by\n+accident of implementation rather than by design (both backends were\n+originally implemented as shell scripts and happened to invoke other\n+commands like 'git checkout' or 'git commit' that would call the\n+hooks). Both backends should have the same behavior, though it is not\n+entirely clear which, if any, is correct. We will likely remove both\n+of these hooks in the future.\n \n Interruptability\n ~~~~~~~~~~~~~~~~\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"394688","messageId":"xmqqk12wz7jv.fsf@gitster.c.googlers.com","threadId":"53162","inReplyTo":"pull.749.git.git.1585888520572.gitgitgadget@gmail.com","subject":"Re: [PATCH] git-rebase.txt: add another hook to the hooks section, and explain more","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-03T19:35:32Z","receivedAt":"2020-04-03T19:35:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> The apply backend has not traditionally called the post-commit hook,\n> +while the merge backend has. In contrast, the apply backend has\n> +traditionally called the post-checkout hook while the merge backend\n> +has not. However, the calling of these hooks in both cases was by\n> +accident of implementation rather than by design (both backends were\n> +originally implemented as shell scripts and happened to invoke other\n> +commands like 'git checkout' or 'git commit' that would call the\n> +hooks). Both backends should have the same behavior, though it is not\n> +entirely clear which, if any, is correct. We will likely remove both\n> +of these hooks in the future.\n\nI'd phrase, instead of \"remove\", \"stop calling from 'git rebase'\".\nThese hooks will still be called from their intended context ;-)\n"},{"id":"394693","messageId":"CABPp-BHQ9NSaocWEC9GGN4xD7sqi6MrtF98gDgZ5O5fhYYgwbw@mail.gmail.com","threadId":"53162","inReplyTo":"xmqqk12wz7jv.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] git-rebase.txt: add another hook to the hooks section, and explain more","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2020-04-03T19:52:58Z","receivedAt":"2020-04-03T19:53:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Apr 3, 2020 at 12:35 PM Junio C Hamano wrote:\n>\n> \"Elijah Newren via GitGitGadget\" writes:\n>\n> > The apply backend has not traditionally called the post-commit hook,\n> > +while the merge backend has. In contrast, the apply backend has\n> > +traditionally called the post-checkout hook while the merge backend\n> > +has not. However, the calling of these hooks in both cases was by\n> > +accident of implementation rather than by design (both backends were\n> > +originally implemented as shell scripts and happened to invoke other\n> > +commands like 'git checkout' or 'git commit' that would call the\n> > +hooks). Both backends should have the same behavior, though it is not\n> > +entirely clear which, if any, is correct. We will likely remove both\n> > +of these hooks in the future.\n>\n> I'd phrase, instead of \"remove\", \"stop calling from 'git rebase'\".\n> These hooks will still be called from their intended context ;-)\n\nMakes sense; I'll update it.\n"},{"id":"394760","messageId":"pull.749.v2.git.git.1585963816430.gitgitgadget@gmail.com","threadId":"53162","inReplyTo":"pull.749.git.git.1585888520572.gitgitgadget@gmail.com","subject":"[PATCH v2] git-rebase.txt: add another hook to the hooks section, and explain more","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-04T01:30:16Z","receivedAt":"2020-04-04T01:30:22Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nFor more discussion about these hooks, their history relative to rebase,\nand logical consistency between different types of operations, see\n https://lore.kernel.org/git/CABPp-BG0bFKUage5cN_2yr2DkmS04W2Z9Pg5WcROqHznV3XBdw@mail.gmail.com/\nand the links to some threads referenced therein.\n\nSigned-off-by: Elijah Newren \n---\n git-rebase.txt: add another hook to the hooks section, and explain more\n \n Changes since v1:\n \n * Updated the wording of the last sentence as per Junio's suggestion.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-749%2Fnewren%2Frebase-and-hooks-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-749/newren/rebase-and-hooks-v2\nPull-Request: https://github.com/git/git/pull/749\n\nRange-diff vs v1:\n\n 1: 29a1ff520ba ! 1: 45a5c1c1ff9 git-rebase.txt: add another hook to the hooks section, and explain more\n @@ Documentation/git-rebase.txt: Hooks\n +originally implemented as shell scripts and happened to invoke other\n +commands like 'git checkout' or 'git commit' that would call the\n +hooks). Both backends should have the same behavior, though it is not\n -+entirely clear which, if any, is correct. We will likely remove both\n -+of these hooks in the future.\n ++entirely clear which, if any, is correct. We will likely make rebase\n ++stop calling either of these hooks in the future.\n \n Interruptability\n ~~~~~~~~~~~~~~~~\n\n\n Documentation/git-rebase.txt | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex f7a6033607f..5e0fcd4e9bd 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -684,9 +684,15 @@ Hooks\n ~~~~~\n \n The apply backend has not traditionally called the post-commit hook,\n-while the merge backend has. However, this was by accident of\n-implementation rather than by design. Both backends should have the\n-same behavior, though it is not clear which one is correct.\n+while the merge backend has. In contrast, the apply backend has\n+traditionally called the post-checkout hook while the merge backend\n+has not. However, the calling of these hooks in both cases was by\n+accident of implementation rather than by design (both backends were\n+originally implemented as shell scripts and happened to invoke other\n+commands like 'git checkout' or 'git commit' that would call the\n+hooks). Both backends should have the same behavior, though it is not\n+entirely clear which, if any, is correct. We will likely make rebase\n+stop calling either of these hooks in the future.\n \n Interruptability\n ~~~~~~~~~~~~~~~~\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"},{"id":"394761","messageId":"7c1cf1dd-3f70-0c57-ad4f-44453ed565a6@gmail.com","threadId":"53162","inReplyTo":"pull.749.v2.git.git.1585963816430.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] git-rebase.txt: add another hook to the hooks section, and explain more","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2020-04-04T09:33:20Z","receivedAt":"2020-04-04T09:33:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Elijah\n\nOn 04/04/2020 02:30, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren \n> \n> For more discussion about these hooks, their history relative to rebase,\n> and logical consistency between different types of operations, see\n> https://lore.kernel.org/git/CABPp-BG0bFKUage5cN_2yr2DkmS04W2Z9Pg5WcROqHznV3XBdw@mail.gmail.com/\n> and the links to some threads referenced therein.\n> \n> Signed-off-by: Elijah Newren \n> ---\n> git-rebase.txt: add another hook to the hooks section, and explain more\n> \n> Changes since v1:\n> \n> * Updated the wording of the last sentence as per Junio's suggestion.\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-749%2Fnewren%2Frebase-and-hooks-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-749/newren/rebase-and-hooks-v2\n> Pull-Request: https://github.com/git/git/pull/749\n> \n> Range-diff vs v1:\n> \n> 1: 29a1ff520ba ! 1: 45a5c1c1ff9 git-rebase.txt: add another hook to the hooks section, and explain more\n> @@ Documentation/git-rebase.txt: Hooks\n> +originally implemented as shell scripts and happened to invoke other\n> +commands like 'git checkout' or 'git commit' that would call the\n> +hooks). Both backends should have the same behavior, though it is not\n> -+entirely clear which, if any, is correct. We will likely remove both\n> -+of these hooks in the future.\n> ++entirely clear which, if any, is correct. We will likely make rebase\n> ++stop calling either of these hooks in the future.\n> \n> Interruptability\n> ~~~~~~~~~~~~~~~~\n> \n> \n> Documentation/git-rebase.txt | 12 +++++++++---\n> 1 file changed, 9 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index f7a6033607f..5e0fcd4e9bd 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -684,9 +684,15 @@ Hooks\n> ~~~~~\n> \n> The apply backend has not traditionally called the post-commit hook,\n> -while the merge backend has. However, this was by accident of\n> -implementation rather than by design. Both backends should have the\n> -same behavior, though it is not clear which one is correct.\n> +while the merge backend has. In contrast, the apply backend has\n> +traditionally called the post-checkout hook while the merge backend\n> +has not. \n\nThis is not correct - both backends call the post-checkout hook, but the \nmerge backend suppresses any output from it - see \nhttps://public-inbox.org/git/6c413abc-f48d-5a02-ac8b-09a0fa80b98d@gmail.com/\n\nBest Wishes\n\nPhillip\n\n> However, the calling of these hooks in both cases was by\n> +accident of implementation rather than by design (both backends were\n> +originally implemented as shell scripts and happened to invoke other\n> +commands like 'git checkout' or 'git commit' that would call the\n> +hooks). Both backends should have the same behavior, though it is not\n> +entirely clear which, if any, is correct. We will likely make rebase\n> +stop calling either of these hooks in the future.\n> \n> Interruptability\n> ~~~~~~~~~~~~~~~~\n> \n> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n> \n"},{"id":"394785","messageId":"pull.749.v3.git.git.1586044818132.gitgitgadget@gmail.com","threadId":"53162","inReplyTo":"pull.749.v2.git.git.1585963816430.gitgitgadget@gmail.com","subject":"[PATCH v3] git-rebase.txt: add another hook to the hooks section, and explain more","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-05T00:00:17Z","receivedAt":"2020-04-05T00:00:23Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nFor more discussion about these hooks, their history relative to rebase,\nand logical consistency between different types of operations, see\n https://lore.kernel.org/git/CABPp-BG0bFKUage5cN_2yr2DkmS04W2Z9Pg5WcROqHznV3XBdw@mail.gmail.com/\nand the links to some threads referenced therein.\n\nSigned-off-by: Elijah Newren \n---\n git-rebase.txt: add another hook to the hooks section, and explain more\n \n Changes since v2:\n \n * Corrections to the text pointed out by Phillip.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-749%2Fnewren%2Frebase-and-hooks-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-749/newren/rebase-and-hooks-v3\nPull-Request: https://github.com/git/git/pull/749\n\nRange-diff vs v2:\n\n 1: 45a5c1c1ff9 ! 1: 22fb6ff7080 git-rebase.txt: add another hook to the hooks section, and explain more\n @@ Documentation/git-rebase.txt: Hooks\n -while the merge backend has. However, this was by accident of\n -implementation rather than by design. Both backends should have the\n -same behavior, though it is not clear which one is correct.\n -+while the merge backend has. In contrast, the apply backend has\n -+traditionally called the post-checkout hook while the merge backend\n -+has not. However, the calling of these hooks in both cases was by\n -+accident of implementation rather than by design (both backends were\n -+originally implemented as shell scripts and happened to invoke other\n -+commands like 'git checkout' or 'git commit' that would call the\n -+hooks). Both backends should have the same behavior, though it is not\n -+entirely clear which, if any, is correct. We will likely make rebase\n -+stop calling either of these hooks in the future.\n ++while the merge backend has. Both have called the post-checkout hook,\n ++though the merge backend has squelched its output. Further, both\n ++backends only call the post-checkout hook with the starting point\n ++commit of the rebase, not the intermediate commits nor the final\n ++commit. In each case, the calling of these hooks was by accident of\n ++implementation rather than by design (both backends were originally\n ++implemented as shell scripts and happened to invoke other commands\n ++like 'git checkout' or 'git commit' that would call the hooks). Both\n ++backends should have the same behavior, though it is not entirely\n ++clear which, if any, is correct. We will likely make rebase stop\n ++calling either of these hooks in the future.\n \n Interruptability\n ~~~~~~~~~~~~~~~~\n\n\n Documentation/git-rebase.txt | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex f7a6033607f..5a756b5b3a6 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -684,9 +684,17 @@ Hooks\n ~~~~~\n \n The apply backend has not traditionally called the post-commit hook,\n-while the merge backend has. However, this was by accident of\n-implementation rather than by design. Both backends should have the\n-same behavior, though it is not clear which one is correct.\n+while the merge backend has. Both have called the post-checkout hook,\n+though the merge backend has squelched its output. Further, both\n+backends only call the post-checkout hook with the starting point\n+commit of the rebase, not the intermediate commits nor the final\n+commit. In each case, the calling of these hooks was by accident of\n+implementation rather than by design (both backends were originally\n+implemented as shell scripts and happened to invoke other commands\n+like 'git checkout' or 'git commit' that would call the hooks). Both\n+backends should have the same behavior, though it is not entirely\n+clear which, if any, is correct. We will likely make rebase stop\n+calling either of these hooks in the future.\n \n Interruptability\n ~~~~~~~~~~~~~~~~\n\nbase-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925\n-- \ngitgitgadget\n"}]}