Re: [PATCH v2 00/13] hook.[ch]: new library to run hooks + simple hook conversion

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 16, 2021, 05:47 UTC
Message-ID: <874k9hecro.fsf@evledraar.gmail.com>
In-Reply-To: <xmqqbl3p4wgz.fsf@gitster.g>

On Fri, Oct 15 2021, Junio C Hamano wrote:

Show 17 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Part 2 of the greater configurable hook saga, starting by converting
>> some existing simple hooks to the new hook.[ch] library and "git hook
>> run" utility.
>>
>> See the v1 [1] CL for more context. I've dropped the git-p4 folks from
>> the CC list, your feedback on git-p4 would still be very appreciated,
>> but I don't think I need to re-spam you for every re-roll of this.
>
>
> This, when merged to 'seen', seems to fail so many tests to make me
> question my sanity.  Something is not playing with it well.
>
> Queued on top of a merge of ab/config-based-hooks-1 into 'master';
> when tested alone, without any other topics in 'seen', it seems to
> fare much better.  Only t2400-worktree-add.sh fails at 58-59 and 62.

I haven't looked into worktree-add.sh failure (and from what Eric says it seems unrelated), but all the rest is due to a mismerge of reset.c. The diff here at the end on top of "seen" fixes it[1].

I'm able to reproduce it with:

    $ git reference 978c287ca92 # just a "show" alias...
    978c287ca92 (hooks: convert 'post-checkout' hook to hook library, 2021-10-15)
    $ git checkout origin/seen^1; git -c rerere.enabled=false cherry-pick 978c287ca92

Which has a big conflict, including:
    
    @@ -158,10 -90,54 +159,58 @@@
                    goto leave_reset_head;
            }
      
    ++<<<<<<< HEAD
     +      if (oid != &head_oid || update_orig_head || switch_to_branch)
     +              ret = update_refs(opts, oid);
    ++=======
    + reset_head_refs:
    +       reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
    +       strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : default_reflog_action);

I.e. the oid check there went away, but needs to be kept as well.

1.

diff --git a/reset.c b/reset.c
index 6441b22eebc..602cba75c1c 100644
--- a/reset.c
+++ b/reset.c
@@ -165,6 +165,8 @@ int reset_head(struct repository *r, const struct reset_head_opts *opts)
                goto leave_reset_head;
        }
 
+       if (oid != &head_oid || update_orig_head || switch_to_branch)
+               ret = update_refs(opts, oid);
 
 leave_reset_head:
        rollback_lock_file(&lock);