On Tue, May 21, 2024 at 10:50:25AM -0700, Junio C Hamano wrote: > tboegi@xxxxxx writes: > > > Add a missing call to precompose_string_if_needed() to this code > > in setup.c : > > `work_tree = precompose_string_if_needed(get_git_work_tree());` > > This is new in this iteration, I presume? The old one did the > precompose only in strbuf_getcwd(). We now precompose also the > result of get_git_work_tree(). > > Two questions. > > * It is unclear to me why this makes a difference only when the > precompuse configuration is set only in the local configuration. > > * As the leading part of the value placed in get_git_work_tree() > comes from strbuf_getcwd() called by abspath.c:real_pathdup() > that is called by repository.c:repo_set_worktree(), doesn't this > potentially call precompse twice on the already precomposed early > parth of the get_git_work_tree() result? > > I suspect that with the arrangement in your test, the argument given > to set_git_work_tree() from setup.c:setup_discovered_git_dir() is > always ".", and that dot is passed to repository.c:repo_set_worktree() > which calls abspath.c:real_pathdup() to turn it into an absolute, > where it has a call to strbuf_getcwd(). > > So with the provided test, I suspect there is no difference between > the previous and this iteration in behaviour, as what is fed to > precompose should be identical? > > What this iteration does differently is that inside real_pathdup(), > if the string given to repo_set_worktree() is more than the trivial > ".", it is appended to the result of strbuf_getcwd(), and the new > code precomposes after such appending in real_pathdup() happens. It > will convert the leading part twice [*] and more importantly the > appended part is now converted, unlike the previous one? > > Side note: [*] hopefully precompose is idempotent? Relying > on that property somewhat feels yucky, though. > > Puzzled... > > Will replace and queue, but I couldn't figure out what is going on > with the help by the proposed log message, so... Acknowledge. The commit message deserves an update, for sure. My suggestion would be too keep it in seen, until I have managed to write a better commit message. At the same time, I would ask Jun-ichi Takimoto to do a re-test of the new version.
