Re: [PATCH/RFC] core.precomposeunicode is true by default

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 29, 2013, 17:20 UTC
Message-ID: <7vmwp5z3iu.fsf@alter.siamese.dyndns.org>
In-Reply-To: <201307270321.32398.tboegi@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

Show 8 quoted lines

> When core.precomposeunicode was introduced, it was set to false
> by default, to be compatible with older versions of Git.
>
> Whenever UTF-8 file names are used in a mixed environment,
> the Mac OS users need to find out that this configuration exist
> and set it to true manually.
>
> There is no measurable performance impact between false and true.

The real reason we default it to auto-sensing in the current code is for correctness, I think. the new precompose code could be buggy, and by auto-sensing, we hoped that we would enable it only on filesystems that the codepath matters.

> A smoother workflow can be achieved for new Git users,
> so change the default to true:
>
> - Remove the auto-sensing

Why?

> - Rename the internal variable into precompose_unicode,
>   and set it to 1 meaning true.

Why the rename?

> - Adjust and clean up test cases
>
> The configuration core.precomposeunicode is still supported.

Sorry, but I do not quite understand the change. Is this because the auto-sensing is not working, or after auto-sensing we do a wrong thing? If that is the case, perhaps that is what we should fix?

Show 17 quoted lines

> diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
> index 7980abd..5396b91 100644
> --- a/compat/precompose_utf8.c
> +++ b/compat/precompose_utf8.c
> @@ -36,30 +36,6 @@ static size_t has_non_ascii(const char *s, size_t maxlen, size_t *strlen_c)
>  }
>  
>  
> -void probe_utf8_pathname_composition(char *path, int len)
> -{
> -	static const char *auml_nfc = "\xc3\xa4";
> -	static const char *auml_nfd = "\x61\xcc\x88";
> -	int output_fd;
> -	if (precomposed_unicode != -1)
> -		return; /* We found it defined in the global config, respect it */
> -	strcpy(path + len, auml_nfc);
> -	output_fd = open(path, O_CREAT|O_EXCL|O_RDWR, 0600);

So we try to create a path under one name, and ...

Show 6 quoted lines

> -	if (output_fd >= 0) {
> -		close(output_fd);
> -		strcpy(path + len, auml_nfd);
> -		/* Indicate to the user, that we can configure it to true */
> -		if (!access(path, R_OK))
> -			git_config_set("core.precomposeunicode", "false");

... see if that path can be seen under its alias. Why do we set it to "false"? Isn't this the true culprit?

After all, this is not in the "reinit" codepath, so we know we are dealing with a repository that was created afresh.