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

Re: git complains packed-refs is not a directory when used with GIT_DIR and GIT_WORK_TREE envvars.

From
Duy Nguyen <pclouds@gmail.com>
Date
Mar 27, 2018, 14:56 UTC
Message-ID
<CACsJy8DP53Og1crS1bLoJf6w8cJhjGKy=ggfbsqzJ6AU4eNhPA@mail.gmail.com>
In-Reply-To
<20180327063137.GA24044@sigill.intra.peff.net>
On Tue, Mar 27, 2018 at 8:31 AM, Jeff King <peff@peff.net> wrote:
>...
>
> But that really feels like we're papering over the problem.

It is papering over the problem in my opinion. setup_work_tree() is involved here and when it moves cwd around, it re-init git-dir (and all other paths). Before my patch we call git_path() only when we need it and git-dir is likely already updated by setup_work_tree(). After this patch, the path is set in stone before setup_work_tree() kicks in. Once it moves cwd, relative paths all become invalid.

The way setup_work_tree() does it is just bad to me. When it calls set_git_dir() again, the assumption is the new path is exactly the same as the old one. The only difference is relative vs absolute. But this is super hard to see from set_git_dir implementation. The new struct repository design also inherits this (i.e. allowing to call set_git_dir multiple times, which really does not make sense), but this won't fly long term. When cwd moves, all cached relative paths must be adjusted, we need a better mechanism to tell everybody that, not just do it for $GIT_DIR and related paths.

I am planning to fix this. This is part of the "setup cleanup" effort to keep repository.c design less messy and hopefully unify how the main repo and submodule repo are created/set up. But frankly that may take a long while before I could submit anything substantial (I also have the "report multiple worktree's HEADs correctly and make fsck count all HEADs" task, which is not small and to me have higher priority).

So I would not mind papering over it right now (with an understanding that absolute path pays some more overhead in path walking, which was th reason we tried to avoid it in setup code). A slightly better patch is trigger this path absolutization from setup_work_tree(), near set_git_dir(). But then you face some ugliness there: how to find out all ref stores to update, or just update the main ref store alone.

Show 6 quoted lines
> It's not
> clear to me exactly what f57f37e2 is trying to accomplish, and whether
> it would work for it to look call get_git_dir() whenever it needed the
> path.
>
> -Peff
-- 
Duy
Previous: Jeff KingNext: Jeff King
Message 4 of 54 in “git complains packed-refs is not a directory when used with GIT_DIR and GIT_WORK_TREE envvars.”
  1. Rafael AscensaoMar 26, 2018
  2. Ævar Arnfjörð BjarmasonMar 26, 2018
  3. Jeff KingMar 27, 2018
  4. Duy NguyenMar 27, 2018
  5. Jeff KingMar 27, 2018
  6. Duy NguyenMar 27, 2018
  7. Duy NguyenMar 27, 2018
  8. Jeff KingMar 28, 2018
  9. Duy NguyenMar 28, 2018
  10. Jeff KingMar 28, 2018
  11. 1/4 set_git_dir: die when setenv() failsJeff King, Mar 28, 2018
  12. 2/4 add chdir-notify APIJeff King, Mar 28, 2018
  13. Eric SunshineMar 28, 2018
  14. Jeff KingMar 28, 2018
  15. Duy NguyenMar 29, 2018
  16. Jeff KingMar 29, 2018
  17. Duy NguyenMar 29, 2018
  18. 3/4 set_work_tree: use chdir_notifyJeff King, Mar 28, 2018
  19. Duy NguyenMar 29, 2018
  20. Duy NguyenMar 29, 2018
  21. Jeff KingMar 29, 2018
  22. Jeff KingMar 29, 2018
  23. Duy NguyenMar 29, 2018
  24. Jeff KingMar 30, 2018
  25. 4/4 refs: use chdir_notify to update cached relative pathsJeff King, Mar 28, 2018
  26. 0/5 re-parenting relative directories after chdirJeff King, Mar 30, 2018
  27. 1/5 set_git_dir: die when setenv() failsJeff King, Mar 30, 2018
  28. 2/5 trace.c: export trace_setup_keyJeff King, Mar 30, 2018
  29. Junio C HamanoMar 30, 2018
  30. Jeff KingMar 30, 2018
  31. Junio C HamanoMar 30, 2018
  32. Jeff KingMar 30, 2018
  33. 3/5 add chdir-notify APIJeff King, Mar 30, 2018
  34. 4/5 set_work_tree: use chdir_notifyJeff King, Mar 30, 2018
  35. 5/5 refs: use chdir_notify to update cached relative pathsJeff King, Mar 30, 2018
  36. Duy NguyenMar 30, 2018
  37. Jeff KingMar 28, 2018
  38. 0/8 Re: git complains packed-refs is not a directory when used with GIT_DIR and GIT_WORK_TREE envvars.Nguyễn Thái Ngọc Duy, Mar 28, 2018
  39. 1/8 strbuf.c: add strbuf_ensure_trailing_dr_sep()Nguyễn Thái Ngọc Duy, Mar 28, 2018
  40. 2/8 strbuf.c: reintroduce get_pwd_cwd() (with strbuf_ prefix)Nguyễn Thái Ngọc Duy, Mar 28, 2018
  41. Stefan BellerMar 28, 2018
  42. Duy NguyenMar 28, 2018
  43. 3/8 trace.c: export trace_setup_keyNguyễn Thái Ngọc Duy, Mar 28, 2018
  44. 4/8 setup.c: introduce setup_adjust_path()Nguyễn Thái Ngọc Duy, Mar 28, 2018
  45. 5/8 setup.c: allow other code to be notified when $CWD movesNguyễn Thái Ngọc Duy, Mar 28, 2018
  46. 6/8 environment.c: adjust env containing relpath when $CWD is movedNguyễn Thái Ngọc Duy, Mar 28, 2018
  47. Jeff KingMar 28, 2018
  48. Duy NguyenMar 28, 2018
  49. 8/8 refs: adjust main repo paths when $CWD movesNguyễn Thái Ngọc Duy, Mar 28, 2018
  50. 7/8 repository: adjust repo paths when $CWD movesNguyễn Thái Ngọc Duy, Mar 28, 2018
  51. Jeff KingMar 28, 2018
  52. Duy NguyenMar 29, 2018
  53. Jeff KingMar 30, 2018
  54. Junio C HamanoMar 28, 2018

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.