Re: [PATCH v6 4/6] refs: move out stub modification to generic layer
- From
Toon Claes <toon@iotcl.com>
- Date
- Feb 18, 2026, 14:21 UTC
- Message-ID
- <87o6lmceup.fsf@iotcl.com>
- In-Reply-To
- <CAOLa=ZQwrOGpZfVtfTfPFhnkJ_qnEhv8mxO3Ot7nQXusbkJkYw@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 21 quoted lines
> 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.
Show 10 quoted lines
>> 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
Show 31 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.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.
Show 9 quoted lines
>> 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