On Fri, Mar 30, 2018 at 02:34:25PM -0400, Jeff King wrote: > On Wed, Mar 28, 2018 at 01:36:56PM -0400, Jeff King wrote: > > > For those just joining us, this fixes a regression that was in v2.13 (so > > nothing we need to worry about as part of the 2.17-rc track). > > > > [1/4]: set_git_dir: die when setenv() fails > > [2/4]: add chdir-notify API > > [3/4]: set_work_tree: use chdir_notify > > [4/4]: refs: use chdir_notify to update cached relative paths > > Here's a re-roll based on the feedback I got, including: > > - fixes the memory leak and vague comment pointed out by Eric > > - adds in tracing code from Duy > > - I took Duy's suggestions regarding "least" surprise in some of the > functions (reparenting NULL is a noop, and registering a reparent > handler does so even for an absolute path). > > I punted on the "registering the same path twice" thing. That is a > potential way to misuse this API, but I don't think there's a good > solution. The "reparent" helper could figure this out for you, but in > the general case we actually install an arbitrary callback. So the > caller really has to handle it there. The series looks good to me. > > I think in the long run we'd want to outlaw calling set_git_dir() twice > anyway. Oh yeah. With my latest WIP changes, the bottom of setup_git_directory_gently() looks like this. Nowhere else in setup code calls these functions anymore (except the current setup_work_tree) -- 8< -- if (result.worktree) set_git_work_tree(result.worktree); if (result.gitdir) set_git_dir(result.gitdir); if (startup_info->have_repository) repo_set_hash_algo(the_repository, result.repo_fmt.hash_algo); ... return result.prefix; -- 8< -- >From here on, it's not hard to see how to turn set_git_work_tree() into setup_work_tree_gently() (without doing any set_git_dir) and the last two calls into "repo_init_gitdir(gitdir, hash_algo)", which should be where we allocate a new repository object and initialize related object store, ref store... But I still have a couple setup corner cases to deal with first :( -- Duy
