Re: [PATCH v2] odb: do not list our own refs as alternate refs in the quarantine

From: Jeff King <peff@peff.net>
Date: Nov 4, 2026, 02:48 UTC
Message-ID: <20261104024806.GA1492379@coredump.intra.peff.net>
In-Reply-To: <pull.2285.v2.git.1793619933300.gitgitgadget@gmail.com>

On Mon, Nov 02, 2026 at 11:45:33AM +0000, Kristofer Karlsson via GitGitGadget wrote:

Show 16 quoted lines

> An alternate that is ourselves is not a source of alternate refs: its
> refs are our own refs, which the caller already lists as it sees fit.
> Mark each object directory added via alternates with whether it is
> borrowed from another repository, and list alternate refs only from
> those. Within one process this needs no guessing, as tmp-objdir
> links our own object directory in itself. But a child process rebuilds
> its object store from the environment, so when an alternate is added,
> recognize ourselves in two ways:
> 
>  - as our <commondir>/objects, which also covers an environment that
>    makes our own object directory an alternate by hand;
> 
>  - as the parent of GIT_QUARANTINE_PATH, as the quarantine is created
>    directly inside our object directory (see "QUARANTINE ENVIRONMENT"
>    in git-receive-pack(1)); this also works when the object directory
>    is not under the commondir.

So as I said before, I think this is a reasonable fix regardless of the how quarantine issues work. We should not be pulling in alternate ref sources that duplicate each other or our main repo.

But I was surprised to see special handling of GIT_QUARANTINE_PATH here. I think this is a defect in the alternate-ref code. Yes, the common case is triggering it with our tmp-objdir shenanigans, but if we fix the underlying issue, then the use of it here should fix the quarantine issue automatically.

I.e., I'd expect the code part of the patch to just be:

diff --git a/odb.c b/odb.c
index 2c93a9490c..f9ab5c8e7f 100644
--- a/odb.c
+++ b/odb.c
@@ -183,6 +183,7 @@ static int refs_from_alternate_cb(struct odb_files_dir *alternate,
 				  void *payload)
 {
 	struct strbuf path = STRBUF_INIT;
+	struct strbuf repo = STRBUF_INIT;
 	size_t base_len;
 	struct alternate_refs_data *cb = payload;
 
@@ -198,10 +199,16 @@ static int refs_from_alternate_cb(struct odb_files_dir *alternate,
 		goto out;
 	strbuf_setlen(&path, base_len);
 
+	/* XXX could realpath once in caller as a mild optimization */
+	if (strbuf_realpath(&repo, cb->repo->commondir, 0) &&
+	    !strcmp(path.buf, repo.buf))
+		goto out;
+
 	read_alternate_refs(cb->repo, path.buf, cb->fn, cb->payload);
 
 out:
 	strbuf_release(&path);
+	strbuf_release(&repo);
 	return 0;
 }
 

and keep it totally local to the path hackery in the alternate-refs
path.

Or am I missing something that requires more complication?

> This is safe: alternate refs only serve as negative tips that are
> assumed to be complete, so dropping some can only make the traversal
> walk further, never make it accept a missing object.

That's how _we_ use them in the internal callers, but it's a rev-list
option. So we can't say how they are used elsewhere. That said, I think
this is a totally reasonable tightening of the definition for general
use.

-Peff