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

From: Kristofer Karlsson <krka@spotify.com>
Date: Nov 3, 2026, 12:05 UTC
Message-ID: <CAL71e4McZzVJqFHnmpoQkC6+hUkhz9_HtEy7dqC5rDyXdV6oaQ@mail.gmail.com>
In-Reply-To: <aum-34I8KZvTfDRT@pks.im>

On Tue, Nov 03, 2026 at 12:00 PM Patrick Steinhardt <ps@pks.im> wrote:

> So this here isn't fully correct as it does not account for all cases.
> We'd have to read the object storage format extension to figure out
> where the primary object directory lives.

Good point. I can rework the patch to make it easier to integrate with your ps/odb-object-storage-extension topic, by having the files source remember its default object directory in files->default_object_dir where it decides it, and comparing against that instead of hardcoding <commondir>/objects in the check. Then your topic only has to change how the default is decided, and the check follows. Something like this, if you want:

--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ static bool is_borrowed_object_dir(struct odb_source_files *files,
- strbuf_addf(&objdir, "%s/objects", files->base.odb->repo->commondir);
- if (strbuf_realpath(&self, objdir.buf, 0) &&
+ if (strbuf_realpath(&self, files->default_object_dir, 0) &&
     !fspathcmp(real.buf, self.buf)) {
@@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
+ default_object_dir = xstrfmt("%s/objects", odb->repo->commondir);
  if (!object_dir)
- object_dir = xstrfmt("%s/objects", odb->repo->commondir);
+ object_dir = xstrdup(default_object_dir);

  CALLOC_ARRAY(files, 1);
+ files->default_object_dir = default_object_dir;

Either way, the check should not make things worse where it is
incomplete.  A directory we fail to recognize as our own stays
borrowed, so --alternate-refs lists its refs just like today.  The
opposite mistake would need a quarantine inside another repository's
object directory, which seems unlikely, and even then it would only
cost some negative tips: the traversal walks further, but never
accepts a missing object.

> This makes me wonder whether the better fix would be to stop modifying
> the primary object directory and instead we simply append the quarantine
> directory to alternates.
[...]

That does sound like the cleaner fix.  Since you are already deep into
this area, I would be happy to drop the fix and wait for that instead.
I expect very few users to be affected in practice: mostly git hosts
with many hidden refs, which is the case bcec6780b2 was about.

In that case I could send just the tests, with the hidden refs case
marked as test_expect_failure, so that it is covered once a long-term
fix lands.  Or, if you would rather have a short-term fix in the
meantime, I can send a v3 with the change above.

Thanks,
Kristofer