Re: [PATCH v2 0/5] re-parenting relative directories after chdir

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 30, 2018, 19:36 UTC
Message-ID: <20180330193637.GA1620@duynguyen.home>
In-Reply-To: <20180330183425.GA30575@sigill.intra.peff.net>

On Fri, Mar 30, 2018 at 02:34:25PM -0400, Jeff King wrote:

Show 25 quoted lines

> 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