Re: [PATCH v6 4/6] refs: move out stub modification to generic layer

From: Karthik Nayak <karthik.188@gmail.com>
Date: Feb 17, 2026, 09:29 UTC
Message-ID: <CAOLa=ZQwrOGpZfVtfTfPFhnkJ_qnEhv8mxO3Ot7nQXusbkJkYw@mail.gmail.com>
In-Reply-To: <aZQXpb7RMW83eGxe@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 17 quoted lines

> On Sat, Feb 14, 2026 at 11:34:17PM +0100, Karthik Nayak wrote:
>> When creating the reftable reference backend on disk, we create stubs to
>> ensure that the directory can be recognized as a Git repository. This is
>> done by calling `refs_create_refdir_stubs()`. Move this to the generic
>> layer as this is needed for all backends excluding from the files
>> backends. In an upcoming commit, we'll also need to extend this logic to
>> create stubs when using alternate reference directories.
>>
>> Similarly, move the logic for deletion of stubs to the generic layer.
>> The files backend recursively calls the remove function of the
>> 'packed-backend', here skip calling the generic function since that
>> would try to delete stubs.
>
> Tiniest nit: it might make sense to reorder patches a bit so that the
> creation of `refs_create_refdir_stubs()` and this patch here sit next to
> each other.
>

I think that would be nice, let me do that.

> What's missing a bit in the commit message is the motivation. What does
> this step enable us to do that we couldn't do before?
>

I did add a line

  In an upcoming commit, we'll also need to extend this logic to create
  stubs when using alternate reference directories.

I'll expand a little on that.

Show 27 quoted lines

>> diff --git a/refs.c b/refs.c
>> index 11d028232b..a24602c9bf 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2190,12 +2190,59 @@ void refs_create_refdir_stubs(struct repository *repo, const char *refdir,
>>  /* backend functions */
>>  int ref_store_create_on_disk(struct ref_store *refs, int flags, struct strbuf *err)
>>  {
>> -	return refs->be->create_on_disk(refs, flags, err);
>> +	int ret = refs->be->create_on_disk(refs, flags, err);
>> +
>> +	if (!ret &&
>> +	    ref_storage_format_by_name(refs->be->name) != REF_STORAGE_FORMAT_FILES) {
>> +		struct strbuf msg = STRBUF_INIT;
>> +
>> +		strbuf_addf(&msg, "this repository uses the %s format", refs->be->name);
>> +		refs_create_refdir_stubs(refs->repo, refs->gitdir, msg.buf);
>> +		strbuf_release(&msg);
>> +	}
>> +
>> +	return ret;
>>  }
>
> This makes me wonder: if we called `refs_create_refdir_stubs()` before
> we call `->create_on_disk()`, could we even do it for the "files"
> backend? Just a thought though.
>

Well, there is some nuance there

1. 'refs/heads', 'refs/tags' is not created for linked worktrees.
2. 'HEAD' is only created lazily, not in `create_on_disk()`.

Also the intent is totally different, the stubs are for backward compatibility. So I think its better to let that logic stay within the files-backend.

Show 18 quoted lines

>>  int ref_store_remove_on_disk(struct ref_store *refs, struct strbuf *err)
>>  {
>> -	return refs->be->remove_on_disk(refs, err);
>> +	int ret = refs->be->remove_on_disk(refs, err);
>> +
>> +	if (!ret) {
>> +		enum ref_storage_format format = ref_storage_format_by_name(refs->be->name);
>> +		struct strbuf sb = STRBUF_INIT;
>> +
>> +		/* Backends apart from the files backend create stubs. */
>> +		if (format == REF_STORAGE_FORMAT_FILES)
>> +			return ret;
>
> For symmetry it would be nice to not have an early return here, but also
> format the condition for this block in the same way as we have it for
> `ref_store_create_on_disk()`.
>
> Patrick

Yeah sure, we can do that here, in the last commit, we'll have to modify that anyway back to something like this. But it definitely would be easier to review this commit. Will add.