git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Torsten Bögershausen <tboegi@web.de>
Date
May 21, 2024, 20:57 UTC
Message-ID
<20240521205749.GA8165@tb-raspi4>
In-Reply-To
<xmqqttir9hr2.fsf@gitster.g>
On Tue, May 21, 2024 at 10:50:25AM -0700, Junio C Hamano wrote:
Show 45 quoted lines
> tboegi@web.de 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.

Previous: Junio C HamanoNext: Junio C Hamano
Message 17 of 24 in “strbuf_getcwd() needs precompse_strbuf_if_needed()”
  1. 2/2 strbuf_getcwd() needs precompse_strbuf_if_needed()tboegi@web.de, May 7, 2024
  2. Junio C HamanoMay 7, 2024
  3. Junio C HamanoMay 9, 2024
  4. Torsten BögershausenMay 9, 2024
  5. Junio C HamanoMay 7, 2024
  6. brian m. carlsonMay 8, 2024
  7. 1/2 t0050: ls-files path fails if path of workdir is NFDtboegi@web.de, May 7, 2024
  8. Junio C HamanoMay 7, 2024
  9. 1/1 macOS: ls-files path fails if path of workdir is NFDtboegi@web.de, May 9, 2024
  10. Junio C HamanoMay 9, 2024
  11. Jun. TMay 19, 2024
  12. Torsten BögershausenMay 20, 2024
  13. Junio C HamanoMay 20, 2024
  14. Torsten BögershausenMay 20, 2024
  15. 1/1 macOS: ls-files path fails if path of workdir is NFDtboegi@web.de, May 21, 2024
  16. Junio C HamanoMay 21, 2024
  17. Torsten BögershausenMay 21, 2024
  18. Junio C HamanoMay 21, 2024
  19. Jun. TMay 23, 2024
  20. Torsten BögershausenMay 25, 2024
  21. 1/1 macOS: ls-files path fails if path of workdir is NFDtboegi@web.de, May 31, 2024
  22. Junio C HamanoJun 1, 2024
  23. Torsten BögershausenJun 2, 2024
  24. Jun TJun 4, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.