From: Kristofer Karlsson Date: Tue, 03 Nov 2026 12:05:29 GMT Subject: Re: [PATCH v2] odb: do not list our own refs as alternate refs in the quarantine Message-ID: In-Reply-To: On Tue, Nov 03, 2026 at 12:00 PM Patrick Steinhardt 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 /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