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

Jeff King <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
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).

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.

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.

-Peff

[1] Even on a single filesystem I think case-sensitivity check is not
    completely sufficient either. We know that filesystems do more
    complicated one-way transformations than just case folding, like
    unicode normalization or even removing some funky code points.
    We'd miss those "equivalent" spellings.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.