[BUG] 'ls-files path' fails if absolute path of workdir contains NFD (macOS)

[BUG] 'ls-files path' fails if absolute path of workdir contains NFD (macOS)

[Jun T]

On macOS, 'git ls-files path' does not work (gives an error)
if the absolute 'path' contains characters in NFD (decomposed).
I guess this is a (minor) bug of git.

[1] How to reproduce the problem

On macOS, git 2.39.3 or 2.45.0.219.gb9fe23f5ca(next branch),
file system can be APFS or HFS.

with zsh or bash:
% cd /somewhere         # some safe place, /tmp or ~/tmp etc.
% mkdir $'u\xcc\x88'    # ü in NFD
% cd ü                  # or cd $'u\xcc\x88' or cd $'\xc3\xbc'
% git init
% git ls-files $'/somewhere/u\xcc\x88'   # NFD
fatal: /somewhere/ü: '/somewhere/ü' is outside repository at '/somewhere/ü'
% git ls-files $'/somewhere/\xc3\xbc'    # NFC
(the same error as above)

In the 'fatal:' error message, there are three ü;
the 1st and 2nd are in NFC, the 3rd is in NFD.

[2] Some analysis

The path on the command line $'/somewhere/u\xcc\x88'
is converted to NFC by precompose_argv_prefix(),
called at git.c:451 in run_builtin().

But get_git_work_tree() (called at setup.c:50, in
abspath_part_inside_repo()) returns the work_tree in NFD,
and comparing it with the path in NFC (setup.c:92) fails.

I'm not familiar with git internals, but maybe
get_git_work_tree() should return NFC (on macoS)?

--
Jun (Jun-ichi Takimoto)

Re: [BUG] 'ls-files path' fails if absolute path of workdir contains NFD (macOS)

[Torsten Bögershausen]

On Tue, Apr 30, 2024 at 12:27:02PM +0900, Jun T wrote:
> On macOS, 'git ls-files path' does not work (gives an error)
> if the absolute 'path' contains characters in NFD (decomposed).
> I guess this is a (minor) bug of git.
>
> [1] How to reproduce the problem
>
> On macOS, git 2.39.3 or 2.45.0.219.gb9fe23f5ca(next branch),
> file system can be APFS or HFS.
>
> with zsh or bash:
> % cd /somewhere         # some safe place, /tmp or ~/tmp etc.
> % mkdir $'u\xcc\x88'    # ü in NFD
> % cd ü                  # or cd $'u\xcc\x88' or cd $'\xc3\xbc'
> % git init
> % git ls-files $'/somewhere/u\xcc\x88'   # NFD
> fatal: /somewhere/ü: '/somewhere/ü' is outside repository at '/somewhere/ü'
> % git ls-files $'/somewhere/\xc3\xbc'    # NFC
> (the same error as above)
>
> In the 'fatal:' error message, there are three ü;
> the 1st and 2nd are in NFC, the 3rd is in NFD.
>
> [2] Some analysis
>
> The path on the command line $'/somewhere/u\xcc\x88'
> is converted to NFC by precompose_argv_prefix(),
> called at git.c:451 in run_builtin().
>
> But get_git_work_tree() (called at setup.c:50, in
> abspath_part_inside_repo()) returns the work_tree in NFD,
> and comparing it with the path in NFC (setup.c:92) fails.
>
> I'm not familiar with git internals, but maybe
> get_git_work_tree() should return NFC (on macoS)?
>
> --
> Jun (Jun-ichi Takimoto)

Thanks for an excellent bug report and analyzes.
I am familar with the NFC/NFD stuff, but not with get_git_work_tree(),
at least not yet.

If you have a suggestion for a patch, would you like to share it ?
A 'git diff' may be a good start, I am happy to review things.

/Torsten

Re: [BUG] 'ls-files path' fails if absolute path of workdir contains NFD (macOS)

[Jun. T]

> 2024/04/30 16:12, Torsten Bögershausen <tboegi@web.de> wrote:
> 
> I am familar with the NFC/NFD stuff, but not with get_git_work_tree(),
> at least not yet.
> 
> If you have a suggestion for a patch, would you like to share it ?

Well, the only thing I can tell is the patch below _seems_ to fix
the _current_ problem. There may be other problems, it may introduce
new problem(s) (such as memory leak), etc.

But I don't know anything about the git internals and have no time
now to investigate further. Sorry.

get_git_work_tree() just returns the value of the_repository->worktree
but I have no idea where this variable is set.

get_git_work_tree() and the_repository->worktree are used in
many places, and I'm not sure changing the function/variable has
no bad side effects or not.

And how to convert into NFC? By precompose_string_if_needed()?
Does this function allocate a new memory for NFC? If so, do we
need to free it at some point?


diff --git a/setup.c b/setup.c
index f4b32f76e3..3f2f3ed016 100644
--- a/setup.c
+++ b/setup.c
@@ -47,7 +47,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)

Re: [BUG] 'ls-files path' fails if absolute path of workdir contains NFD (macOS)

[Torsten Bögershausen]

On Wed, May 01, 2024 at 12:52:38AM +0900, Jun. T wrote:
>
> > 2024/04/30 16:12, Torsten Bögershausen <tboegi@web.de> wrote:
> >
> > I am familar with the NFC/NFD stuff, but not with get_git_work_tree(),
> > at least not yet.
> >
> > If you have a suggestion for a patch, would you like to share it ?
>
> Well, the only thing I can tell is the patch below _seems_ to fix
> the _current_ problem. There may be other problems, it may introduce
> new problem(s) (such as memory leak), etc.
>
> But I don't know anything about the git internals and have no time
> now to investigate further. Sorry.
>
> get_git_work_tree() just returns the value of the_repository->worktree
> but I have no idea where this variable is set.
>
> get_git_work_tree() and the_repository->worktree are used in
> many places, and I'm not sure changing the function/variable has
> no bad side effects or not.
>
> And how to convert into NFC? By precompose_string_if_needed()?
> Does this function allocate a new memory for NFC? If so, do we
> need to free it at some point?
>
>
> diff --git a/setup.c b/setup.c
> index f4b32f76e3..3f2f3ed016 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -47,7 +47,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)
>

Thanks for digging - I have spend some time to find the cause,
but no success yet.
There is even a set_git_work_tree() in environment.c, and that may
need some treatment - or some other place.
To be continued - I will continue digging, where the NFD comes into Git.