Re: [PATCH v4 1/1] macOS: ls-files path fails if path of workdir is NFD
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 1, 2024, 15:55 UTC
- Message-ID
- <xmqqh6ecbqug.fsf@gitster.g>
- In-Reply-To
- <20240531193156.28046-1-tboegi@web.de>
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.
Show 137 quoted lines
> 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