Subject: Re: [PATCH v2 0/6] refs: excessive hook execution with packed refs

From: Junio C Hamano <[email-masked]>
Date: Fri, 07 Jan 2022 14:17:21 -0800

Patrick Steinhardt <[email-masked]> writes:

> Hi,
>
> this is a resend of version 1 of this patch series to hopefully entice
> some reviews. The only change is that v2 is rebased onto the current
> main branch at commit e83ba647f7 (The seventh batch, 2022-01-05). The
> following was from the orignial cover letter:

I'll add Ævar, who has been making a lot of changes to the refs
subsystem, and Han-Wen, whose work to add a new ref backend may need
to interact with this change, as possible stake-holders to the CC list.

> As reported by Waleed in [1], the reference-transaction hook is being
> executed when packing refs. Given that the hook ideally ought to track
> logical updates to refs instead of leaking low-level implementation
> details of how the files backend works, this is understandably leading
> to some confusion.
>
> This patch series aims to fix that by improving how the tandom of loose
> and packed refs backends interact such that we skip executing the hook
> when the loose backend:
>
>     - repacks references.
>     - needs to delete packed refs when deleting a loose ref would
>       uncover that packed ref.
>
> Patrick
>
> [1]: <[email-masked]>
>
> Patrick Steinhardt (6):
>   refs: open-code deletion of packed refs
>   refs: allow passing flags when beginning transactions
>   refs: allow skipping the reference-transaction hook
>   refs: demonstrate excessive execution of the reference-transaction
>     hook
>   refs: do not execute reference-transaction hook on packing refs
>   refs: skip hooks when deleting uncovered packed refs
>
>  refs.c                           | 11 +++++--
>  refs.h                           |  8 ++++-
>  refs/files-backend.c             | 25 +++++++++++-----
>  refs/packed-backend.c            | 30 ++++++++++++++-----
>  refs/packed-backend.h            |  6 ++++
>  refs/refs-internal.h             |  1 +
>  sequencer.c                      |  2 +-
>  t/t1416-ref-transaction-hooks.sh | 50 ++++++++++++++++++++++++++++++++
>  8 files changed, 113 insertions(+), 20 deletions(-)