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:

quoted

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