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

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>>> 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.
>
> Thanks, I was thinking the same, but I wasn't going to comment on that.
> Happy to see you've agreed on this already.
>
>>> 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.
>
> <3
>
>>>> 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.
>
> I'm a little bit confused what you mean here? Would it be a problem if
> it *is* created?
>

>> 2. 'HEAD' is only created lazily, not in `create_on_disk()`.
>
> Okay, seems like a valid argument to me. You don't want to have
> `refs/HEAD` created with `ref: refs/heads/.invalid`?
>

>> 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.
>
> That's mainly because you named the function like this, but it doesn't
> have to be named like that.
>

>>> 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.
>
> :+1:
>
> --
> Cheers,
> Toon