From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 16 Oct 2025 12:11:56 GMT
Subject: Re: [PATCH 1/4] refs: move to using the '.optimize' functions
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:
> 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