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

[Adrian Ratiu <adrian.ratiu@collabora.com>]

On Mon, 10 Nov 2025, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes: 
> 
>> I tried the mkdir approach. Unfortunately it does not work 
>> because  submodule_name_to_gitdir() is called on all submodule 
>> paths: both  existing or new, valid or non-valid. 
>> 
>> So if a dir already exists, we do not know if there is a 
>> conflict. It might just be a normal valid path verification of 
>> an  existing module. 
> 
> Puzzled.  Wouldn't that only mean that submodule-name-to-gitdir 
> is a wrong place to see if the name of a directory you are 
> planning to use for a newly added submodule is available?  IOW, 
> don't you need a more specific new helper function and perform 
> the check in there? 

I thought long and hard about this in each version since v1 and 
keep coming to the same answer: we need to do it here and can't 
avoid it.

The reason is that submodule_name_to_gitdir() is the unified API 
where submodule gitdirs get computed consistently and validation 
goes hand in hand with computing the valid gitdirs. If you look at 
all the places where validate_submodule_git_dir() is currently 
called, it's always immediately after submodule_name_to_gitdir(), 
on its result.

It even makes sense to remove the validation calls outside 
submodule_name_to_gitdir() and do it inside it for each path 
attempt, like we do for the encoding cases, so API users don't 
have to manually validate the result each time. 

This reminds me: we should also validate the gitdir path we get 
via the submodule.%s.gitdir config. Will fix this as well in v5.

Another related question I've been pondering is whether to create 
two separate validation functions, one for legacy and one for 
encoding.  It doesn't win us much (currently we return early if 
extension/encoding is enabled in the unified function), but we can 
do this in v5 as well.

To recap, what I intend to do in v5 (up to now) is:
- Address Aaron's case-fold corner-case of creating Foo then foo.
- Address Junio's case-fold corner-case of creating foo + Foo + 
  %46oo.
- (Important) Also validate the submodule.%s.gitdir config value.
- Move validation checks inside submodule_name_to_gitdir() to 
ensure they run each time, instead of having API users validate 
the gitdir result each time.  - (Undecided) Split the validation 
function in two for the legacy vs encoding/extension cases.
- (Undecided) If all our path attempts fail, try hashing the name 
and see if that works before erroring out.

Especially if we implement the last point with hashing, the 
chances for conflicts are close to 0.

Here's an interesting idea:

Maybe it's easier to just hash everything when the extension is 
enabled since users now always have the submodule.%s.gitdir 
config, which is also accesible via the submodule--helper?

Hashing + config would eliminate all this corner-case 
complexity. :)