Re: [PATCH] Fix "inside work tree" detection on case-insensitive filesystems

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Dec 29, 2015, 14:47 UTC
Message-ID: <56829D08.1060107@alum.mit.edu>
In-Reply-To: <ac9733a6b922572ec10f09f89e07cde37ba43f13.1443456630.git.johannes.schindelin@gmx.de>

On 09/28/2015 06:12 PM, Johannes Schindelin wrote:

Show 13 quoted lines

> Git has a config variable to indicate that it is operating on a file
> system that is case-insensitive: core.ignoreCase. But the
> `dir_inside_of()` function did not respect that. As a result, if Git's
> idea of the current working directory disagreed in its upper/lower case
> with the `GIT_WORK_TREE` variable (e.g. `C:\test` vs `c:\test`) the
> user would be greeted by the error message
> 
> 	fatal: git-am cannot be used without a working tree.
> 
> when trying to run a rebase.
> 
> This fixes https://github.com/git-for-windows/git/issues/402 (reported by
> Daniel Harding).

I was just going through the 2.7 release notes when I saw this patch. My understanding was that many of the case-insensitive filesystems also support Unicode. Is the byte-oriented code in this patch adequate? I would have thought it necessary to use a Unicode-aware algorithm here, that knows:

* that bytes != characters
* how to do a case-insensitive comparison of strings that include
non-ASCII characters
* (possibly) insensitivity to NFC vs. NFD vs. non-normalized forms

I suppose that such OSs have built-in functions for deciding whether two paths are equivalent. Possibly these could be used?

Michael

Show 35 quoted lines

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  dir.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/dir.c b/dir.c
> index b90484a..fba938b 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2107,6 +2107,15 @@ int file_exists(const char *f)
>  	return lstat(f, &sb) == 0;
>  }
>  
> +static int cmp_icase(char a, char b)
> +{
> +	if (a == b)
> +		return 0;
> +	if (ignore_case)
> +		return toupper(a) - toupper(b);
> +	return a - b;
> +}
> +
>  /*
>   * Given two normalized paths (a trailing slash is ok), if subdir is
>   * outside dir, return -1.  Otherwise return the offset in subdir that
> @@ -2118,7 +2127,7 @@ int dir_inside_of(const char *subdir, const char *dir)
>  
>  	assert(dir && subdir && *dir && *subdir);
>  
> -	while (*dir && *subdir && *dir == *subdir) {
> +	while (*dir && *subdir && !cmp_icase(*dir, *subdir)) {
>  		dir++;
>  		subdir++;
>  		offset++;
>

-- 
Michael Haggerty
mhagger@alum.mit.edu