Re: [PATCH 1/4] refs: move to using the '.optimize' functions

From: Karthik Nayak <karthik.188@gmail.com>
Date: Oct 16, 2025, 12:07 UTC
Message-ID: <CAOLa=ZQGQ_Qan9PtE-akUpWwGg8j6y5xcKpyOpztN7bZp+rMvw@mail.gmail.com>
In-Reply-To: <aPDKzqT2YaG2NmEu@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 40 quoted lines

> On Wed, Oct 15, 2025 at 05:05:46PM -0500, Justin Tobler wrote:
>> On 25/10/15 11:25PM, Karthik Nayak wrote:
>> > diff --git a/refs/debug.c b/refs/debug.c
>> > index 01499b9033..40cd1d9c15 100644
>> > --- a/refs/debug.c
>> > +++ b/refs/debug.c
>> > @@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs,
>> >  	return res;
>> >  }
>> >
>> > -static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)
>> > +static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)
>> >  {
>> >  	struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
>> > -	int res = drefs->refs->be->pack_refs(drefs->refs, opts);
>> > -	trace_printf_key(&trace_refs, "pack_refs: %d\n", res);
>> > +	int res = drefs->refs->be->optimize(drefs->refs, opts);
>> > +	trace_printf_key(&trace_refs, "optimize: %d\n", res);
>> >  	return res;
>> >  }
>> >
>> > @@ -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.

Karthik