Re: [PATCH v4 1/1] macOS: ls-files path fails if path of workdir is NFD

[Junio C Hamano <gitster@pobox.com>]

tboegi@web.de writes:

> The problem is the_repository->config->hash_initialized
> is set to 1 before the_repository->commondir is set to ".git".
> Due to this, .git/config is never read, and precomposed_unicode
> is never set to 1 (remains -1).

The "is never read" part is a bit confusing and misleading.  If it
were

    At the point of code flow where we would want to learn the value
    of precompose configuration, the local configuration has not
    been read.

then I would understand it, though.

Is the analysis telling us that we need to rethink the order of
things setup_git_directory() does?  Or is this inherently unsolvable
because we need to discover the .git/ directory and path to it
before we can read configuration to learn from it, but we need the
value of precompose setting to compute the "path to it"?

Presumably chdir() done in setup_discovered_git_dir() can be done
with either NFC or NFD if the filesystem is squashing the
differences between them, so perhaps doing the repo_set_worktree()
done in setup_discovered_git_dir() is wrong, and we could delay
populating the .worktree member until much later?  For reading the
local config, it does matter we know where the git-dir and
common-dir are, but the location of worktree is immaterial.

Anyway, thanks for a patch.  Will queue.

> run_builtin() {
>     setup_git_directory() {
>         strbuf_getcwd() {   # setup.c:1542
>             precompose_{strbuf,string}_if_needed() {
>                 # precomposed_unicode is still -1
>                 git_congig_get_bool("core.precomposeunicode") {
>                     git_config_check_init() {
>                         repo_read_config() {
>                             git_config_init() {
>                                 # !!!
>                                 the_repository->config->hash_initialized=1
>                                 # !!!
>                             }
>                             # does not read .git/config since
>                             # the_repository->commondir is still NULL
>                         }
>                     }
>                 }
>                 returns without converting to NFC
>             }
>             returns cwd in NFD
>         }
>
>         setup_discovered_git_dir() {
>             set_git_work_tree(".") {
>                 repo_set_worktree() {
>                     # this function indirectly calls strbuf_getcwd()
>                     # --> precompose_{strbuf,string}_if_needed() -->
>                     # {git,repo}_config_get_bool("core.precomposeunicode"),
>                     # but does not try to read .git/config since
>                     # the_repository->config->hash_initialized
>                     # is already set to 1 above. And it will not read
>                     # .git/config even if hash_initialized is 0
>                     # since the_repository->commondir is still NULL.
>
>                     the_repository->worktree = NFD
>                 }
>             }
>         }
>
>         setup_git_env() {
>             repo_setup_gitdir() {
>                 repo_set_commondir() {
>                     # finally commondir is set here
>                     the_repository->commondir = ".git"
>                 }
>             }
>         }
>
>     } // END setup_git_directory
>
> Reported-by: Jun T <takimoto-j@kba.biglobe.ne.jp>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  setup.c                      |  2 +-
>  t/t3910-mac-os-precompose.sh | 39 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 2e607632db..61f61496ec 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -48,7 +48,7 @@ static int abspath_part_inside_repo(char *path)
>  	size_t wtlen;
>  	char *path0;
>  	int off;
> -	const char *work_tree = get_git_work_tree();
> +	const char *work_tree = precompose_string_if_needed(get_git_work_tree());
>  	struct strbuf realpath = STRBUF_INIT;
>
>  	if (!work_tree)
> diff --git a/t/t3910-mac-os-precompose.sh b/t/t3910-mac-os-precompose.sh
> index 898267a6bd..6d5918c8fe 100755
> --- a/t/t3910-mac-os-precompose.sh
> +++ b/t/t3910-mac-os-precompose.sh
> @@ -37,6 +37,27 @@ Alongc=$Alongc$Alongc$Alongc$Alongc$Alongc           #50 Byte
>  Alongc=$Alongc$Alongc$Alongc$Alongc$Alongc           #250 Byte
>  Alongc=$Alongc$AEligatu$AEligatu                     #254 Byte
>
> +
> +ls_files_nfc_nfd () {
> +	test_when_finished "git config --global --unset core.precomposeunicode" &&
> +	prglbl=$1
> +	prlocl=$2
> +	aumlcreat=$3
> +	aumllist=$4
> +	git config --global core.precomposeunicode $prglbl &&
> +	(
> +		rm -rf .git &&
> +		mkdir -p "somewhere/$prglbl/$prlocl/$aumlcreat" &&
> +		mypwd=$PWD &&
> +		cd "somewhere/$prglbl/$prlocl/$aumlcreat" &&
> +		git init &&
> +		git config core.precomposeunicode $prlocl &&
> +		git --literal-pathspecs ls-files "$mypwd/somewhere/$prglbl/$prlocl/$aumllist" 2>err &&
> +		>expected &&
> +		test_cmp expected err
> +	)
> +}
> +
>  test_expect_success "detect if nfd needed" '
>  	precomposeunicode=$(git config core.precomposeunicode) &&
>  	test "$precomposeunicode" = true &&
> @@ -211,8 +232,8 @@ test_expect_success "unicode decomposed: git restore -p . " '
>  '
>
>  # Test if the global core.precomposeunicode stops autosensing
> -# Must be the last test case
>  test_expect_success "respect git config --global core.precomposeunicode" '
> +	test_when_finished "git config --global --unset core.precomposeunicode" &&
>  	git config --global core.precomposeunicode true &&
>  	rm -rf .git &&
>  	git init &&
> @@ -220,4 +241,20 @@ test_expect_success "respect git config --global core.precomposeunicode" '
>  	test "$precomposeunicode" = "true"
>  '
>
> +test_expect_success "ls-files false false nfd nfd" '
> +	ls_files_nfc_nfd false false $Adiarnfd $Adiarnfd
> +'
> +
> +test_expect_success "ls-files false true nfd nfd" '
> +	ls_files_nfc_nfd false true $Adiarnfd $Adiarnfd
> +'
> +
> +test_expect_success "ls-files true false nfd nfd" '
> +	ls_files_nfc_nfd true false $Adiarnfd $Adiarnfd
> +'
> +
> +test_expect_success "ls-files true true nfd nfd" '
> +	ls_files_nfc_nfd true true $Adiarnfd $Adiarnfd
> +'
> +
>  test_done
> --
> 2.41.0.394.ge43f4fd0bd