Re: [PATCH v2 1/4] odb: decouple source path comparisons from `the_repository`

[Patrick Steinhardt <ps@pks.im>]

On Fri, Aug 14, 2026 at 01:17:24PM -0400, Jeff King wrote:
> On Wed, Aug 12, 2026 at 11:13:57AM +0200, Patrick Steinhardt wrote:
> 
> > When registering alternates we deduplicate object database sources by
> > their path so that the same source won't be added twice. Ever since
> > cf2dc1c238 (speed up alt_odb_usable() with many alternates, 2021-07-07)
> > this duplicate check is backed by a map keyed by the source's path,
> > using `fspathhash()` and `fspatheq()` as hash and equality functions,
> > respectively.
> > 
> > These functions are problematic in this context for two reasons:
> > 
> >   - They implicitly depend on `the_repository` instead of the
> >     repository that owns the object database.
> 
> I'm not even sure that using core.ignorecase here is strictly correct.
> It is a property of the containing repository, and the filesystem in
> which it's stored. But there is no guarantee that the alternate
> directories are in the same repository, or even the same filesystem!
> 
> So it is really just a best guess proxy for "this system tends to use or
> not use case insensitive filesystems[1]". It can be wrong in both
> directions (failing to suppress duplicates, and suppressing them when
> they are not actually duplicates).
> 
> I wonder how bad it would be if we just always did case-sensitive
> comparisons and made it the caller's responsibility to spell things
> consistently.  I guess some names ultimately come from things like
> "--reference" command-line arguments, so that would depend on user
> spelling. But having duplicates at all is kind of unlikely (you can't
> get it from one --reference clone, but rather a complex tree of
> interwoven repos with shared roots).
> 
> How bad is a duplicate alternate? It's a minor performance issue, I'd
> think. We would add its packs to the list (though hardly ever look
> through them, as the "first" copy would satisfy most requests, and the
> unused second copies end up at the back of the MRU list). You'd only pay
> the extra lookup cost for an object which we fail to find entirely,
> which is rare-ish (mostly speculative lookups for fetches).

A performance regression is definitely the most likely change in
behaviour we might see because of this. One other part I am a bit
worried about is housekeeping, but I think we should be fine there as we
only consider the primary source as special.

I also had the feeling that case insensitivity is quite a bit lacking,
too. What we're really after is whether two directories are actually the
exact same path. And whether the path is case-insensitive is only one
part of that equation, so it's an imperfect metric by itself already.

Ideally, we should probably use realpath(3p) to at least also resolve
symlinks. Unfortunately, it's not guaranteed that this function also
knows to canonicalize casing.

> And it would fix the unlikely-but-possible opposite case of suppressing
> a non-duplicate. If you have a repo on a case-insensitive filesystem
> with two alternates on a case-sensitive system that differ only in case,
> we erroneously suppress one of them, and commands may fail to find
> objects we should have. Of course that's super unlikely, which is why
> nobody has run into it before.
> 
> So I kind of wonder if we could just do away with considering case
> insensitivity here at all. We'd err on the side of correctness in the
> ambiguous cases, and this code complexity can just go away.

You will of course be able to craft edge cases where that would be a
significant regression. But if your alternates file looks like this you
may be holding it wrong:

    /path/to/alternate
    /PATH/TO/ALTERNATE
    /pAtH/tO/aLtErNaTe
    /PaTh/To/AlTeRnAtE

> Alternatively, I think we could probably make the check more thorough in
> a similar way. Always consider a pair of case-insensitive matches as
> possible duplicates, and then for each possible duplicate use stat() to
> check their st_dev and st_ino values. That keeps things cheap for normal
> cases, and we pay only the stat() before de-duping. It's correct and
> doesn't rely on the repo, though it is a bit more somewhat complicated
> code.

Hm. Weren't there filesystems where `st_ino` and `st_dev` aren't set at
all? I think that's the case on Windows, which is unfortunately also the
one where we see case insensitive filesystems by default. So that makes
it way less effective, as it only works on systems where we typically
aren't case-insensitive in the first place (except macOS maybe).

So if we want to go down this path I'm inclined to just unconditionally
use case sensitive matching and not introduce any secondary machinery.

Patrick