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

Karthik Nayak [off-list ref] writes:

Patrick Steinhardt [off-list ref] writes:

quoted

On Sat, Feb 14, 2026 at 11:34:17PM +0100, Karthik Nayak wrote:

quoted

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.

quoted

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

quoted

quoted

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.

quoted

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