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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 10, 2025, 19:10 UTC
Message-ID: <xmqqwm3xzots.fsf@gitster.g>
In-Reply-To: <87ecq5ke2m.fsf@gentoo.mail-host-address-is-not-set>

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

Show 10 quoted lines

> 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?

Show 5 quoted lines

> 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.