sequencer: start running post-commit hook again #388

phillipwood · 2019-10-10T13:55:48Z

When I converted the sequencer to avoid forking git commit i forgot about the post-commit hook. These patches are based on pw/rebase-i-show-HEAD-to-reword, otherwise the new test fails as that branch changes the number of commits we make.

Thanks to Dscho & Junio for their comments on V1. I've made the
following changes:

Patches 1-3
These are new patches in response to Dscho's request to set $EDITOR
in a subshell. There were ~80 other tests that weren't doing that
and they are fixed in these patches. Patch 2 contains the main
action, unfortunately due to a combination of having to remove the
trailing ' &&' from the last line moved into the subshell and
re-wrapping some lines due to the increased indentation
--color-moved and --color-moved-ws are of limited use when viewing
this patch.

Patch 4 (was 1)
Unchanged

Patch 5 (was 2)
I've moved the function definition to commit.c rather than
sequencer.c as suggested. I've also removed an unused struct argv_array from
run_prepare_commit_msg_hook() (There wasn't a compiler warning as it
was still calling argv_array_clear() at the end of the function) and
reworded the commit message.

Patch 6 (was 3)
I've tided up the test and removed the wrapper function for running
the post-commit hook as suggested.

Cc: Johannes Schindelin [email protected]

phillipwood · 2019-10-10T18:35:44Z

/submit

gitgitgadget · 2019-10-10T18:37:14Z

Submitted as [email protected]

WARNING: phillipwood has no public email address set on GitHub

gitgitgadget · 2019-10-10T21:17:27Z

builtin/commit.c

@@ -1443,28 +1443,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	return git_status_config(k, v, s);


On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Phillip, On Thu, 10 Oct 2019, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood <[email protected]> > > This simplifies the implementation of run_prepare_commit_msg_hook() and > will be used in the next commit. > > Signed-off-by: Phillip Wood <[email protected]> > --- > builtin/commit.c | 22 ---------------------- > commit.h | 3 --- > sequencer.c | 45 ++++++++++++++++++++++++++++++++++----------- > sequencer.h | 2 ++ > 4 files changed, 36 insertions(+), 36 deletions(-) Hmm. I would have thought that `commit.c` would be a more logical home for that function (and that the declaration could remain in `commit.h`)? The general thrust is good, of course: `commit.h` is supposedly a `libgit.a` header, but `builtin/commit.o` is _not_ included in `libgit.a`... Thanks, Dscho > > diff --git a/builtin/commit.c b/builtin/commit.c > index 1921401117..d898a57f5d 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1443,28 +1443,6 @@ static int git_commit_config(const char *k, const= char *v, void *cb) > return git_status_config(k, v, s); > } > > -int run_commit_hook(int editor_is_used, const char *index_file, const c= har *name, ...) > -{ > - struct argv_array hook_env =3D ARGV_ARRAY_INIT; > - va_list args; > - int ret; > - > - argv_array_pushf(&hook_env, "GIT_INDEX_FILE=3D%s", index_file); > - > - /* > - * Let the hook know that no editor will be launched. > - */ > - if (!editor_is_used) > - argv_array_push(&hook_env, "GIT_EDITOR=3D:"); > - > - va_start(args, name); > - ret =3D run_hook_ve(hook_env.argv,name, args); > - va_end(args); > - argv_array_clear(&hook_env); > - > - return ret; > -} > - > int cmd_commit(int argc, const char **argv, const char *prefix) > { > const char *argv_gc_auto[] =3D {"gc", "--auto", NULL}; > diff --git a/commit.h b/commit.h > index f5295ca7f3..37684a35f0 100644 > --- a/commit.h > +++ b/commit.h > @@ -389,7 +389,4 @@ void verify_merge_signature(struct commit *commit, i= nt verbose); > int compare_commits_by_commit_date(const void *a_, const void *b_, void= *unused); > int compare_commits_by_gen_then_commit_date(const void *a_, const void = *b_, void *unused); > > -LAST_ARG_MUST_BE_NULL > -int run_commit_hook(int editor_is_used, const char *index_file, const c= har *name, ...); > - > #endif /* COMMIT_H */ > diff --git a/sequencer.c b/sequencer.c > index 2adcf5a639..3ce578c40b 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1123,28 +1123,51 @@ void commit_post_rewrite(struct repository *r, > run_rewrite_hook(&old_head->object.oid, new_head); > } > > +int run_commit_hook(int editor_is_used, const char *index_file, > + const char *name, ...) > +{ > + struct argv_array hook_env =3D ARGV_ARRAY_INIT; > + va_list args; > + int ret; > + > + argv_array_pushf(&hook_env, "GIT_INDEX_FILE=3D%s", index_file); > + > + /* > + * Let the hook know that no editor will be launched. > + */ > + if (!editor_is_used) > + argv_array_push(&hook_env, "GIT_EDITOR=3D:"); > + > + va_start(args, name); > + ret =3D run_hook_ve(hook_env.argv,name, args); > + va_end(args); > + argv_array_clear(&hook_env); > + > + return ret; > +} > + > static int run_prepare_commit_msg_hook(struct repository *r, > struct strbuf *msg, > const char *commit) > { > struct argv_array hook_env =3D ARGV_ARRAY_INIT; > - int ret; > - const char *name; > + int ret =3D 0; > + const char *name, *arg1 =3D NULL, *arg2 =3D NULL; > > name =3D git_path_commit_editmsg(); > if (write_message(msg->buf, msg->len, name, 0)) > return -1; > > - argv_array_pushf(&hook_env, "GIT_INDEX_FILE=3D%s", r->index_file); > - argv_array_push(&hook_env, "GIT_EDITOR=3D:"); > - if (commit) > - ret =3D run_hook_le(hook_env.argv, "prepare-commit-msg", name, > - "commit", commit, NULL); > - else > - ret =3D run_hook_le(hook_env.argv, "prepare-commit-msg", name, > - "message", NULL); > - if (ret) > + if (commit) { > + arg1 =3D "commit"; > + arg2 =3D commit; > + } else { > + arg1 =3D "message"; > + } > + if (run_commit_hook(0, r->index_file, "prepare-commit-msg", name, > + arg1, arg2, NULL)) > ret =3D error(_("'prepare-commit-msg' hook failed")); > + > argv_array_clear(&hook_env); > > return ret; > diff --git a/sequencer.h b/sequencer.h > index ac66892d71..b0419d6ddb 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -201,4 +201,6 @@ int write_basic_state(struct replay_opts *opts, cons= t char *head_name, > void sequencer_post_commit_cleanup(struct repository *r); > int sequencer_get_last_command(struct repository* r, > enum replay_action *action); > +LAST_ARG_MUST_BE_NULL > +int run_commit_hook(int editor_is_used, const char *index_file, const c= har *name, ...); > #endif /* SEQUENCER_H */ > -- > gitgitgadget > >

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Schindelin <[email protected]> writes: >> builtin/commit.c | 22 ---------------------- >> commit.h | 3 --- >> sequencer.c | 45 ++++++++++++++++++++++++++++++++++----------- >> sequencer.h | 2 ++ >> 4 files changed, 36 insertions(+), 36 deletions(-) > > Hmm. I would have thought that `commit.c` would be a more logical home > for that function (and that the declaration could remain in `commit.h`)? Good correction. Thanks.

On the Git mailing list, Phillip Wood wrote (reply to this):

Hi Dscho & Junio On 11/10/2019 05:24, Junio C Hamano wrote: > Johannes Schindelin <[email protected]> writes: > >>> builtin/commit.c | 22 ---------------------- >>> commit.h | 3 --- >>> sequencer.c | 45 ++++++++++++++++++++++++++++++++++----------- >>> sequencer.h | 2 ++ >>> 4 files changed, 36 insertions(+), 36 deletions(-) >> >> Hmm. I would have thought that `commit.c` would be a more logical home >> for that function (and that the declaration could remain in `commit.h`)? > > Good correction. There are some other public commit related functions in sequencer.c - print_commit_summary(), commit_post_rewrite(), rest_is_empty(), cleanup_message(), message_is_empty(), template_untouched(), update_head_with_reflog() . Would you like to see them moved to commit.c (probably as a separate series)? Best Wishes Phillip > > Thanks. >

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Phillip, On Mon, 14 Oct 2019, Phillip Wood wrote: > Hi Dscho & Junio > > On 11/10/2019 05:24, Junio C Hamano wrote: > > Johannes Schindelin <[email protected]> writes: > > > >>> builtin/commit.c | 22 ---------------------- > >>> commit.h | 3 --- > >>> sequencer.c | 45 ++++++++++++++++++++++++++++++++++----------- > >>> sequencer.h | 2 ++ > >>> 4 files changed, 36 insertions(+), 36 deletions(-) > >> > >> Hmm. I would have thought that `commit.c` would be a more logical hom= e > >> for that function (and that the declaration could remain in `commit.h= `)? > > > > Good correction. > > There are some other public commit related functions in sequencer.c - > print_commit_summary(), commit_post_rewrite(), rest_is_empty(), > cleanup_message(), message_is_empty(), template_untouched(), > update_head_with_reflog() . Would you like to see them moved to commit.c > (probably as a separate series)? I don't think that it is necessary to move any of those functions out of their existing habitat just yet. While I haven't looked more closely which of these functions are specific to the sequencer and which are more generic, I would argue that moving any of them is outside of the goals of your patch series. Thanks, Dscho

gitgitgadget · 2019-10-10T21:35:25Z

builtin/commit.c

@@ -1443,28 +1443,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	return git_status_config(k, v, s);


On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Phillip, On Thu, 10 Oct 2019, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood <[email protected]> > > Prior to commit 356ee4659b ("sequencer: try to commit without forking > 'git commit'", 2017-11-24) the sequencer would always run the > post-commit hook after each pick or revert as it forked `git commit` to > create the commit. The conversion to committing without forking `git > commit` omitted to call the post-commit hook after creating the commit. > > Signed-off-by: Phillip Wood <[email protected]> Makes sense. > --- > builtin/commit.c | 2 +- > sequencer.c | 5 +++++ > sequencer.h | 1 + > t/t3404-rebase-interactive.sh | 17 +++++++++++++++++ > 4 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/builtin/commit.c b/builtin/commit.c > index d898a57f5d..adb8c89c60 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1653,7 +1653,7 @@ int cmd_commit(int argc, const char **argv, const = char *prefix) > > repo_rerere(the_repository, 0); > run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); > - run_commit_hook(use_editor, get_index_file(), "post-commit", NULL); > + run_post_commit_hook(use_editor, get_index_file()); Does it really make sense to abstract the hook name away? It adds a lot of churn for just two callers... > if (amend && !no_post_rewrite) { > commit_post_rewrite(the_repository, current_head, &oid); > } > diff --git a/sequencer.c b/sequencer.c > index 3ce578c40b..b4947f6969 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1173,6 +1173,10 @@ static int run_prepare_commit_msg_hook(struct rep= ository *r, > return ret; > } > > +void run_post_commit_hook(int editor_is_used, const char *index_file) { > + run_commit_hook(editor_is_used, index_file, "post-commit", NULL); > +} > + If we must have a separate `run_post_commit_hook()`, then it should be an `inline` function, defined in the header. Or even a macro to begin with. > static const char implicit_ident_advice_noconfig[] =3D > N_("Your name and email address were configured automatically based\n" > "on your username and hostname. Please check that they are accurate.\n" > @@ -1427,6 +1431,7 @@ static int try_to_commit(struct repository *r, > goto out; > } > > + run_post_commit_hook(0, r->index_file); So this is the _actual_ change of this patch. > if (flags & AMEND_MSG) > commit_post_rewrite(r, current_head, oid); > > diff --git a/sequencer.h b/sequencer.h > index b0419d6ddb..e3e73c5635 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -203,4 +203,5 @@ int sequencer_get_last_command(struct repository* r, > enum replay_action *action); > LAST_ARG_MUST_BE_NULL > int run_commit_hook(int editor_is_used, const char *index_file, const c= har *name, ...); > +void run_post_commit_hook(int editor_is_used, const char *index_file); > #endif /* SEQUENCER_H */ > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.= sh > index d2f1d5bd23..d9217235b6 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -1467,4 +1467,21 @@ test_expect_success 'valid author header when aut= hor contains single quote' ' > test_cmp expected actual > ' > > +test_expect_success 'post-commit hook is called' ' > + test_when_finished "rm -f .git/hooks/post-commit commits" && > + mkdir -p .git/hooks && > + write_script .git/hooks/post-commit <<-\EOS && > + git rev-parse HEAD >>commits Should `commits` be initialized before this script is written, e.g. using >commits && ? > + EOS > + set_fake_editor && The `set_fake_editor` function sets a global environment variable, and therefore needs to be run in a subshell. Therefore, this line (as well as the next one) need to be enclosed in `( ... )`. > + FAKE_LINES=3D"edit 4 1 reword 2 fixup 3" git rebase -i A E && > + echo x>file3 && We usually leave no space after the `>`, but we _do_ leave a space _before_ the `>`. > + git add file3 && > + FAKE_COMMIT_MESSAGE=3Dedited git rebase --continue && > + # rev-list does not support -g --reverse > + git rev-list --no-walk=3Dunsorted HEAD@{5} HEAD@{4} HEAD@{3} HEAD@{2} = \ > + HEAD@{1} HEAD >expected && Wouldn't this be better as: git rev-parse HEAD@{5} HEAD@{4} HEAD@{3} HEAD@{2} HEAD@{1} HEAD \ >expect && > + test_cmp expected commits We usually use the name `expect` instead of `expected` in the test suite. Thanks, Dscho > +' > + > test_done > -- > gitgitgadget >

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Schindelin <[email protected]> writes: >> diff --git a/builtin/commit.c b/builtin/commit.c >> index d898a57f5d..adb8c89c60 100644 >> --- a/builtin/commit.c >> +++ b/builtin/commit.c >> @@ -1653,7 +1653,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) >> >> repo_rerere(the_repository, 0); >> run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); >> - run_commit_hook(use_editor, get_index_file(), "post-commit", NULL); >> + run_post_commit_hook(use_editor, get_index_file()); > > Does it really make sense to abstract the hook name away? It adds a lot > of churn for just two callers... After looking at the three patches, I do not think so. >> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh >> index d2f1d5bd23..d9217235b6 100755 >> --- a/t/t3404-rebase-interactive.sh >> +++ b/t/t3404-rebase-interactive.sh >> @@ -1467,4 +1467,21 @@ test_expect_success 'valid author header when author contains single quote' ' >> test_cmp expected actual >> ' >> >> +test_expect_success 'post-commit hook is called' ' >> + test_when_finished "rm -f .git/hooks/post-commit commits" && >> + mkdir -p .git/hooks && >> + write_script .git/hooks/post-commit <<-\EOS && >> + git rev-parse HEAD >>commits > > Should `commits` be initialized before this script is written, e.g. > using > > >commits && Yes. >> + git rev-list --no-walk=unsorted HEAD@{5} HEAD@{4} HEAD@{3} HEAD@{2} \ >> + HEAD@{1} HEAD >expected && > > Wouldn't this be better as: > > git rev-parse HEAD@{5} HEAD@{4} HEAD@{3} HEAD@{2} HEAD@{1} HEAD \ > >expect && > Yes. >> + test_cmp expected commits > > We usually use the name `expect` instead of `expected` in the test > suite. And the actual output file is called 'actual'. Thanks.

On the Git mailing list, Phillip Wood wrote (reply to this):

Hi Dscho On 10/10/2019 22:31, Johannes Schindelin wrote: > Hi Phillip, > > On Thu, 10 Oct 2019, Phillip Wood via GitGitGadget wrote: > >> From: Phillip Wood <[email protected]> >> >> Prior to commit 356ee4659b ("sequencer: try to commit without forking >> 'git commit'", 2017-11-24) the sequencer would always run the >> post-commit hook after each pick or revert as it forked `git commit` to >> create the commit. The conversion to committing without forking `git >> commit` omitted to call the post-commit hook after creating the commit. >> >> Signed-off-by: Phillip Wood <[email protected]> > > Makes sense. > >> --- >> builtin/commit.c | 2 +- >> sequencer.c | 5 +++++ >> sequencer.h | 1 + >> t/t3404-rebase-interactive.sh | 17 +++++++++++++++++ >> 4 files changed, 24 insertions(+), 1 deletion(-) >> >> diff --git a/builtin/commit.c b/builtin/commit.c >> index d898a57f5d..adb8c89c60 100644 >> --- a/builtin/commit.c >> +++ b/builtin/commit.c >> @@ -1653,7 +1653,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) >> >> repo_rerere(the_repository, 0); >> run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); >> - run_commit_hook(use_editor, get_index_file(), "post-commit", NULL); >> + run_post_commit_hook(use_editor, get_index_file()); > > Does it really make sense to abstract the hook name away? It adds a lot > of churn for just two callers... I'll drop the new function in the reroll >> if (amend && !no_post_rewrite) { >> commit_post_rewrite(the_repository, current_head, &oid); >> } >> diff --git a/sequencer.c b/sequencer.c >> index 3ce578c40b..b4947f6969 100644 >> --- a/sequencer.c >> +++ b/sequencer.c >> @@ -1173,6 +1173,10 @@ static int run_prepare_commit_msg_hook(struct repository *r, >> return ret; >> } >> >> +void run_post_commit_hook(int editor_is_used, const char *index_file) { >> + run_commit_hook(editor_is_used, index_file, "post-commit", NULL); >> +} >> + > > If we must have a separate `run_post_commit_hook()`, then it should be > an `inline` function, defined in the header. Or even a macro to begin > with. > >> static const char implicit_ident_advice_noconfig[] = >> N_("Your name and email address were configured automatically based\n" >> "on your username and hostname. Please check that they are accurate.\n" >> @@ -1427,6 +1431,7 @@ static int try_to_commit(struct repository *r, >> goto out; >> } >> >> + run_post_commit_hook(0, r->index_file); > > So this is the _actual_ change of this patch. > >> if (flags & AMEND_MSG) >> commit_post_rewrite(r, current_head, oid); >> >> diff --git a/sequencer.h b/sequencer.h >> index b0419d6ddb..e3e73c5635 100644 >> --- a/sequencer.h >> +++ b/sequencer.h >> @@ -203,4 +203,5 @@ int sequencer_get_last_command(struct repository* r, >> enum replay_action *action); >> LAST_ARG_MUST_BE_NULL >> int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...); >> +void run_post_commit_hook(int editor_is_used, const char *index_file); >> #endif /* SEQUENCER_H */ >> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh >> index d2f1d5bd23..d9217235b6 100755 >> --- a/t/t3404-rebase-interactive.sh >> +++ b/t/t3404-rebase-interactive.sh >> @@ -1467,4 +1467,21 @@ test_expect_success 'valid author header when author contains single quote' ' >> test_cmp expected actual >> ' >> >> +test_expect_success 'post-commit hook is called' ' >> + test_when_finished "rm -f .git/hooks/post-commit commits" && >> + mkdir -p .git/hooks && >> + write_script .git/hooks/post-commit <<-\EOS && >> + git rev-parse HEAD >>commits > > Should `commits` be initialized before this script is written, e.g. > using > > >commits && Good point, especially if it is renamed to actual as Junio suggests >> + EOS >> + set_fake_editor && > > The `set_fake_editor` function sets a global environment variable, and > therefore needs to be run in a subshell. Therefore, this line (as well > as the next one) need to be enclosed in `( ... )`. There are ~80 instances of set_fake_editor/test_set_editor/set_cat_todo_editor in that file that are not in subshells. I've converted them in a preparatory patch (that was fun), removing about 20 that can now safely rely on EDITOR=: (hopefully that will ameliorate the performance hit of ~60 extra subshells a little) >> + FAKE_LINES="edit 4 1 reword 2 fixup 3" git rebase -i A E && >> + echo x>file3 && > > We usually leave no space after the `>`, but we _do_ leave a space > _before_ the `>`. > >> + git add file3 && >> + FAKE_COMMIT_MESSAGE=edited git rebase --continue && >> + # rev-list does not support -g --reverse >> + git rev-list --no-walk=unsorted HEAD@{5} HEAD@{4} HEAD@{3} HEAD@{2} \ >> + HEAD@{1} HEAD >expected && > > Wouldn't this be better as: > > git rev-parse HEAD@{5} HEAD@{4} HEAD@{3} HEAD@{2} HEAD@{1} HEAD \ > >expect && Good point >> + test_cmp expected commits > > We usually use the name `expect` instead of `expected` in the test > suite. OK Thanks for looking at this series Phillip > Thanks, > Dscho > >> +' >> + >> test_done >> -- >> gitgitgadget >>

Neither of the commands executed in the subshell change any shell variables or the current directory so there is no need for them to be executed in a subshell. Signed-off-by: Phillip Wood <[email protected]>

As $EDITOR is exported setting it in one test affects all subsequent tests. Avoid this by always setting it in a subshell. This commit leaves 20 calls to set_fake_editor that are not in subshells as they can safely be removed in the next commit once all the other editor setting is done inside subshells. I have moved the call to set_fake_editor in some tests so it comes immediately before the call to 'git rebase' to avoid moving unrelated commands into the subshell. In one case ('rebase -ix with --autosquash') the call to set_fake_editor is moved past an invocation of 'git rebase'. This is safe as that invocation of 'git rebase' requires EDITOR=: or EDITOR=fake-editor.sh without FAKE_LINES being set which will be the case as the preceding tests either set their editor in a subshell or call set_fake_editor without setting FAKE_LINES. In a one test ('auto-amend only edited commits after "edit"') a call to test_tick are now in a subshell. I think this is OK as it is there to set the date for the next commit which is executed in the same subshell rather than updating GIT_COMMITTER_DATE for later tests (the next test calls test_tick before doing anything else). Signed-off-by: Phillip Wood <[email protected]>

Some tests were calling set_fake_editor to ensure they had a sane no-op editor set. Now that all the editor setting is done in subshells these tests can rely on EDITOR=: and so do not need to call set_fake_editor. Also add a test at the end to detect any future additions messing with the exported value of $EDITOR. Signed-off-by: Phillip Wood <[email protected]>

Commit 6585068 ("rebase -i: rewrite write_basic_state() in C", 2018-08-28) accidentially added new function declarations after the #endif at the end of the include guard. Signed-off-by: Phillip Wood <[email protected]>

dscho · 2019-10-14T10:54:23Z

@phillipwood please note that I restarted one PR build job, as I encountered this type of problem in the past (it does not happen too often, and might be caused by some latent background job or something):

rm: cannot remove 'submodule_update/.git/objects/b6': Directory not empty

In other words, I think this is a transient failure that is not caused by the changes in this PR.

This function was declared in commit.h but was implemented in builtin/commit.c so was not part of libgit. Move it to libgit so we can use it in the sequencer. This simplifies the implementation of run_prepare_commit_msg_hook() and will be used in the next commit. Signed-off-by: Phillip Wood <[email protected]>

Prior to commit 356ee46 ("sequencer: try to commit without forking 'git commit'", 2017-11-24) the sequencer would always run the post-commit hook after each pick or revert as it forked `git commit` to create the commit. The conversion to committing without forking `git commit` omitted to call the post-commit hook after creating the commit. Signed-off-by: Phillip Wood <[email protected]>

dscho · 2019-10-14T15:13:37Z

/azp run

azure-pipelines · 2019-10-14T15:13:46Z

Azure Pipelines successfully started running 2 pipeline(s).

dscho · 2019-10-14T15:14:17Z

I edited the build definition to account for that Homebrew issue described in #391, then restarted the build.

phillipwood · 2019-10-15T10:23:07Z

I edited the build definition to account for that Homebrew issue described in #391, then restarted the build.

@dscho - Thanks for that

phillipwood · 2019-10-15T10:24:35Z

/submit

gitgitgadget · 2019-10-15T10:25:55Z

Submitted as [email protected]

WARNING: phillipwood has no public email address set on GitHub

gitgitgadget · 2019-10-16T01:32:42Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Schindelin <[email protected]> writes:

> Hi Phillip,
>
> On Tue, 15 Oct 2019, Phillip Wood via GitGitGadget wrote:
>
>> When I converted the sequencer to avoid forking git commit i forgot about
>> the post-commit hook. These patches are based on
>> pw/rebase-i-show-HEAD-to-reword, otherwise the new test fails as that branch
>> changes the number of commits we make.
>> ...
>
> I had a look over the patches, too, and all looks good to me.

Me three; thanks, both.

gitgitgadget · 2019-10-16T05:22:26Z

This branch is now known as pw/post-commit-from-sequencer.

gitgitgadget · 2019-10-16T05:22:27Z

This patch series was integrated into pu via git@0f7a08c.

gitgitgadget · 2019-10-18T02:33:32Z

This patch series was integrated into pu via git@bc3eb3d.

gitgitgadget · 2019-10-18T11:48:45Z

This patch series was integrated into pu via git@c57110b.

gitgitgadget · 2019-10-18T11:48:46Z

This patch series was integrated into next via git@15b41a0.

gitgitgadget · 2019-10-22T11:35:03Z

This patch series was integrated into pu via git@af496b5.

gitgitgadget · 2019-10-23T06:00:05Z

This patch series was integrated into pu via git@6fc3f8c.

gitgitgadget · 2019-10-23T12:00:37Z

This patch series was integrated into pu via git@b7a1f5c.

gitgitgadget · 2019-10-24T05:53:33Z

This patch series was integrated into pu via git@faefe86.

gitgitgadget · 2019-10-24T07:25:10Z

This patch series was integrated into pu via git@71b456d.

gitgitgadget · 2019-10-28T05:37:10Z

This patch series was integrated into pu via git@ac704c0.

gitgitgadget · 2019-10-30T07:47:34Z

This patch series was integrated into pu via git@99f491c.

gitgitgadget · 2019-11-02T10:37:37Z

This patch series was integrated into pu via git@ed6e111.

gitgitgadget · 2019-11-04T05:33:39Z

This patch series was integrated into pu via git@2ad8fc9.

gitgitgadget · 2019-11-06T13:37:56Z

This patch series was integrated into pu via git@ef5914a.

gitgitgadget · 2019-11-07T11:14:54Z

This patch series was integrated into pu via git@84cac1f.

gitgitgadget · 2019-11-08T08:48:16Z

This patch series was integrated into pu via git@b9e7864.

gitgitgadget · 2019-11-10T08:59:02Z

This patch series was integrated into pu via git@e8bc188.

gitgitgadget · 2019-11-11T04:50:51Z

This patch series was integrated into pu via git@5c8c0a0.

gitgitgadget · 2019-11-11T04:50:53Z

This patch series was integrated into master via git@5c8c0a0.

gitgitgadget · 2019-11-11T04:50:56Z

Closed via 5c8c0a0.

phillipwood force-pushed the wip/rebase-commit-hooks branch 2 times, most recently from febc50e to acaa086 Compare October 10, 2019 18:33

gitgitgadget bot reviewed Oct 10, 2019

View reviewed changes

phillipwood force-pushed the wip/rebase-commit-hooks branch 2 times, most recently from c872bdd to 1ebf780 Compare October 12, 2019 14:00

t3404: remove unnecessary subshell

b9689e8

Neither of the commands executed in the subshell change any shell variables or the current directory so there is no need for them to be executed in a subshell. Signed-off-by: Phillip Wood <[email protected]>

weekly-digest bot mentioned this pull request Oct 13, 2019

Weekly Digest (6 October, 2019 - 13 October, 2019) #393

Closed

phillipwood force-pushed the wip/rebase-commit-hooks branch from 1ebf780 to 3199063 Compare October 13, 2019 13:08

phillipwood added 3 commits October 14, 2019 10:35

sequencer.h fix placement of #endif

0eac3ed

Commit 6585068 ("rebase -i: rewrite write_basic_state() in C", 2018-08-28) accidentially added new function declarations after the #endif at the end of the include guard. Signed-off-by: Phillip Wood <[email protected]>

phillipwood force-pushed the wip/rebase-commit-hooks branch from 3199063 to dfaca45 Compare October 14, 2019 09:45

phillipwood added 2 commits October 14, 2019 14:12

phillipwood force-pushed the wip/rebase-commit-hooks branch from dfaca45 to 67a7117 Compare October 14, 2019 13:15

This comment has been minimized.

Sign in to view

gitgitgadget bot added the pu label Oct 16, 2019

gitgitgadget bot added the next label Oct 18, 2019

weekly-digest bot mentioned this pull request Oct 20, 2019

Weekly Digest (13 October, 2019 - 20 October, 2019) #409

Closed

gitgitgadget bot added the master label Nov 11, 2019

gitgitgadget bot closed this Nov 11, 2019

phillipwood deleted the wip/rebase-commit-hooks branch August 12, 2021 13:35

phillipwood restored the wip/rebase-commit-hooks branch August 15, 2021 15:21

phillipwood deleted the wip/rebase-commit-hooks branch August 1, 2023 15:46

phillipwood restored the wip/rebase-commit-hooks branch August 1, 2023 17:00

		@@ -1443,28 +1443,6 @@ static int git_commit_config(const char k, const char v, void *cb)
		return git_status_config(k, v, s);

sequencer: start running post-commit hook again #388

sequencer: start running post-commit hook again #388

Conversation

phillipwood commented Oct 10, 2019 • edited Loading

phillipwood commented Oct 10, 2019

gitgitgadget bot commented Oct 10, 2019

gitgitgadget bot Oct 10, 2019

Choose a reason for hiding this comment

gitgitgadget bot Oct 11, 2019

Choose a reason for hiding this comment

gitgitgadget bot Oct 14, 2019

Choose a reason for hiding this comment

gitgitgadget bot Oct 14, 2019

Choose a reason for hiding this comment

gitgitgadget bot Oct 10, 2019

Choose a reason for hiding this comment

gitgitgadget bot Oct 11, 2019

Choose a reason for hiding this comment

gitgitgadget bot Oct 11, 2019

Choose a reason for hiding this comment

dscho commented Oct 14, 2019

dscho commented Oct 14, 2019

azure-pipelines bot commented Oct 14, 2019

dscho commented Oct 14, 2019

phillipwood commented Oct 15, 2019

phillipwood commented Oct 15, 2019

gitgitgadget bot commented Oct 15, 2019

This comment has been minimized.

gitgitgadget bot commented Oct 16, 2019

gitgitgadget bot commented Oct 16, 2019

gitgitgadget bot commented Oct 16, 2019

gitgitgadget bot commented Oct 18, 2019

gitgitgadget bot commented Oct 18, 2019

gitgitgadget bot commented Oct 18, 2019

gitgitgadget bot commented Oct 22, 2019

gitgitgadget bot commented Oct 23, 2019

gitgitgadget bot commented Oct 23, 2019

gitgitgadget bot commented Oct 24, 2019

gitgitgadget bot commented Oct 24, 2019

gitgitgadget bot commented Oct 28, 2019

gitgitgadget bot commented Oct 30, 2019

gitgitgadget bot commented Nov 2, 2019

gitgitgadget bot commented Nov 4, 2019

gitgitgadget bot commented Nov 6, 2019

gitgitgadget bot commented Nov 7, 2019

gitgitgadget bot commented Nov 8, 2019

gitgitgadget bot commented Nov 10, 2019

gitgitgadget bot commented Nov 11, 2019

gitgitgadget bot commented Nov 11, 2019

gitgitgadget bot commented Nov 11, 2019

phillipwood commented Oct 10, 2019 •

edited

Loading