From: Adrian Ratiu <adrian.ratiu@collabora.com>
Date: Wed, 12 Nov 2025 15:28:10 GMT
Subject: Re: [PATCH v4 4/4] submodule: fix case-folding gitdir filesystem colisions
Message-ID: <871pm3jmnp.fsf@collabora.com>
In-Reply-To: <20251107150547.3272180-5-adrian.ratiu@collabora.com>

On Fri, 07 Nov 2025, Adrian Ratiu <adrian.ratiu@collabora.com> 
wrote:
> diff --git a/submodule.c b/submodule.c index 
> ceaff0c1aa..ecbffac2c6 100644 --- a/submodule.c +++ 
> b/submodule.c @@ -2280,7 +2280,7 @@ int 
> validate_submodule_git_dir(char *git_dir, const char 
> *submodule_name) 
>  	size_t len = strlen(git_dir), suffix_len = 
>  strlen(submodule_name); char *p = git_dir + len - suffix_len; 
>  bool suffixes_match = !strcmp(p, submodule_name); 
> -	int ret = 0; +	int ret = 0, config_ignorecase = 0; 
>   /* * We prevent the contents of sibling submodules' git 
>  directories to 
> @@ -2318,6 +2318,42 @@ int validate_submodule_git_dir(char 
> *git_dir, const char *submodule_name) 
>  	if (p && strchr(p, '/') != NULL) return error("submodule 
>  gitdir name '%s' contains unexpected '/'", p);  
> +	/* Prevent conflicts on case-folding filesystems */ + 
> repo_config_get_bool(the_repository, "core.ignorecase", 
> &config_ignorecase); +	if (ignore_case || 
> config_ignorecase) { +		char *lower_gitdir = 
> xstrdup(git_dir); +		char *module_name = 
> find_last_submodule_name(lower_gitdir); + +		if 
> (module_name) { +			for (p = module_name; *p; 
> p++) +				*p = tolower(*p); + + 
> /* +			 * If lower path is different and already 
> exists, check for collision.  +			 * 
> Intentionally double-check to eliminate false-positives.  + 
> */ +			if (strcmp(lower_gitdir, git_dir) && 
> is_git_directory(lower_gitdir)) { + 
> char *canonical = real_pathdup(git_dir, 0); + 
> if (canonical) { +					struct 
> strbuf norm_git_dir = STRBUF_INIT; + 
> strbuf_addstr(&norm_git_dir, git_dir); + 
> strbuf_normalize_path(&norm_git_dir); + + 
> if (strcmp(canonical, norm_git_dir.buf)) + 
> ret = error(_("submodule git dir '%s' " + 
> "collides with '%s'"), + 
> canonical, norm_git_dir.buf); + + 
> strbuf_release(&norm_git_dir); + 
> FREE_AND_NULL(canonical); +				} + 
> } +		} + +		FREE_AND_NULL(lower_gitdir); + 
> return ret; +	} + 
>  	return 0; } 

I think I came up with a better case-folding conflict detection 
implementation.

In a nutshell, for the next iteration (v5), I intend to do:

DIR *dir = opendir(modules_dir);
...
/* Check for another directory under .git/modules that differs 
only in case. */ while ((de = readdir(dir)) != NULL) { if 
(!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))  continue; 
if (!strcasecmp(de->d_name, submodule_name) && strcmp(de->d_name, 
submodule_name)) { closedir(dir); return error(_("submodule name 
'%s' collides with '%s' " "on case-insensitive filesystem"), 
submodule_name, de->d_name); } }

We look at existing submodules and we have a conflict if both are 
true:
1. Names are equal ignoring the case (case insensitive equality).
2. Names are NOT equal considering the case (case sensitive 
inequality).

This is simpler, more robest and passes all my test cases.

Will leave v4 on the ML until next week in case there is more feedback,
then I'll send v5 using this check (I'll also add more tests).