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

On Tue, Nov 03, 2026 at 12:00 PM Patrick Steinhardt [off-list ref] 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