Re: [PATCH v4 4/4] submodule: fix case-folding gitdir filesystem colisions

[Junio C Hamano <gitster@pobox.com>]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> On Sat, 08 Nov 2025, Aaron Schrab <aaron@schrab.com> wrote:
>> At 17:05 +0200 07 Nov 2025, Adrian Ratiu 
>> <adrian.ratiu@collabora.com> wrote: 
>>>Add a new check in validate_submodule_git_dir() to detect and 
>>>prevent case-folding filesystem colisions. When this new check 
>>>is triggered, a stricter casefolding aware URI encoding is used 
>>>to percent-encode uppercase characters, e.g. Foo becomes %46oo. 
>>>By using this check/retry mechanism the uppercase encoding is 
>>>only applied when necessary, so case-sensitive filesystems are 
>>>not affected. 

The .gitdir name munging is a local thing, so it makes sense to do
the casefold mitigation only the filesystem is case folding one,

Your code seems to compare directory names textually, and downcasing
the proposed name for some reason, but I am not sure why we need any
of these complexity.  Wouldn't it be the matter of actually trying
to mkdir(2) the name presented (either "foo" or "Foo") and see if
that fails?  If it fails (most likely with EEXIST if case folding is
getting in the way, but for any reason), the name is unusable and we
need to "tweak" the name to a usable one at that point by retrying.
Once we find a usable name, we can remember the fact that we already
created a directory for it and reuse that empty directory in the
code where we used to do mkdir(2), no?

> Maybe we could derive a new path automatically (eg foo2 or foo_, 
> suggestions welcome) and use it if valid. This way, there is no 
> user intervention.
>
> Do you have any preference?

If adding 'foo' and then an attempt to add 'Foo' will automatically
assign a name that does not conflict with 'foo' to the newly added
submodule, then the users would expect the same to happen if the
order to add them are swapped, wouldn't they?

IOW, I do not see why the code wants to treat uppercase and
lowercase letters any differently, and suspect that it might be the
source of additional complication.  Also, if there is an existing
module with a funny path "%46oo", you cannot just encode "Foo" into
"%46oo" to avoid crashes with 'foo' and be done anyway, so it feels
like we are inviting more bugs by special casing certain paths (and
not encoding or checking others).  Don't we have an issue similar to
"case folding" in macOS wrt UTF-8 canonicalization, too?  An
identical Unicode string may be canonicalized in two ways, so in a
presence of a submodule named one way, the other submodule named in
the other canonicalization, while their names may be with different
byte sequences, cannot co-exist in the same directory next to each
other.  "Try to mkdir(2) the new name, and see if it succeeds, and
if so use the resulting empty directory" approach would cover that
case with the same mechanism as you need to use for case folding
filesystems, I would imagine.