Re: [PATCH 1/4] refs: move to using the '.optimize' functions
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Oct 16, 2025, 12:11 UTC
- Message-ID
- <aPDhDG1kfja8dWXv@pks.im>
- In-Reply-To
- <CAOLa=ZQGQ_Qan9PtE-akUpWwGg8j6y5xcKpyOpztN7bZp+rMvw@mail.gmail.com>
On Thu, Oct 16, 2025 at 07:07:33AM -0500, Karthik Nayak wrote:
Show 33 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> > On Wed, Oct 15, 2025 at 05:05:46PM -0500, Justin Tobler wrote:
> >> On 25/10/15 11:25PM, Karthik Nayak wrote:
> >> > @@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = {
> >> > .transaction_finish = debug_transaction_finish,
> >> > .transaction_abort = debug_transaction_abort,
> >> >
> >> > - .pack_refs = debug_pack_refs,
> >> > + .optimize = debug_optimize,
> >>
> >> question: Was the debug backend not using either of these callbacks?
> >> From the commit message, it sounds like all the backends were using the
> >> optimize callback.
> >
> > Doesn't look like it. Overall I kind of doubt the value that this
> > backend has. I have never had even a single use case for it, and I have
> > been working with references extensively over the last two or three
> > years by now.
> >
> > Maybe we should just drop it eventually?
> >
> > Patrick
>
> I'd be happy to drop it too as a Git developer. But I can merit in
> keeping it. It does wrap around all reference subsystem calls and if
> tracing is enabled it would log these reference calls. This can be used
> in bug reports.
>
> However, a counter argument would be that these only track the surface
> level APIs to each backend, but not really the internal details.
>
> Considering this and also the fact that we need to update the backend
> for every change made, I'm for dropping it too.Digging a bit deeper: I think this actually _is_ exposed via our tracing API. So if you run Git with GIT_TRACE_REFS= set it will indeed use that debug backend. TIL.
So with that I'll reverse my stance and say that this probably makes sense to keep.
Patrick