Re: [PATCH v2] notes: handle multiple worktrees

From: David Turner <dturner@twopensource.com>
Date: Jul 31, 2015, 21:44 UTC
Message-ID: <1438379086.4735.25.camel@twopensource.com>
In-Reply-To: <xmqq8u9w5fnd.fsf@gitster.dls.corp.google.com>

On Fri, 2015-07-31 at 11:46 -0700, Junio C Hamano wrote:

Show 24 quoted lines

> David Turner <dturner@twopensource.com> writes:
> 
> > Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using
> > die_if_shared_symref.  This prevents simultaneous merges to the same
> > notes branch from different worktrees.
> >
> > Signed-off-by: David Turner <dturner@twopensource.com>
> > ---
> >
> > This version addresses Eric Sunshine's critiques of v1.  It breaks out
> > the symref-checking functionality into die_if_shared_symref, leaving
> > die_if_checked_out to handle HEAD.
> 
> >
> > ---
> >  builtin/notes.c                  |  2 ++
> >  t/t3320-notes-merge-worktrees.sh | 71 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 73 insertions(+)
> >  create mode 100755 t/t3320-notes-merge-worktrees.sh
> 
> Hmm, is this the whole thing?
> 
> Or is a prerequisite patch to add that "symref check" infrastructure
> missing on the receiving end?

Oops, resent with first patch included -- I forgot I had split it.

> This is a tangent, but there is a bigger spanner in "bisect".  It
> uses refs/bisect/* to keep track of the current bisection state, but
> that state is tied to a particular worktree, and it should not be
> shared across other worktrees.

Good point.

> We need to reserve a part of refs/* hierarchy (e.g. refs/private/*)
> for per worktree private stuff and handle them just like we do
> per-worktree pseudorefs, or something like that.

In the pseudorefs series, is_per_worktree_ref is a separate function precisely so that we can make this sort of change. To bikeshed a bit, I propose worktree_refs/*, since:

(a) "private" can have many meanings, and (b) common_list in path.c doesn't do exceptions ("refs/ except refs/worktree/"). We could of course do a manual exception, but that's not pretty.

I further propose to add some patches (to go on top of the pseudorefs series):

(a) include worktree-refs/ in per_worktree_refs and include worktree-refs/ in per_worktree_refs in common_list, and (b) include changes to bisect which switches it over to use worktree_refs/*. To make this change compatible with bisections started with old git versions, we should just unconditionally rename refs/bisect to worktree-refs/bisect, and, in the event of EISDIR (meaning that both refs/bisect and worktree-refs/bisect exist), die with a message explaining the situation.