Re: [PATCH v2 0/5] branch: fix branch operations with multiple working trees

From: Kazuki Yamaguchi <k@rhe.jp>
Date: Mar 27, 2016, 07:29 UTC
Message-ID: <20160327072942.GA13750@chikuwa.rhe.jp>
In-Reply-To: <xmqqzitm1ct8.fsf@gitster.mtv.corp.google.com>

On Fri, Mar 25, 2016 at 02:13:07PM -0700, Junio C Hamano wrote:

Show 16 quoted lines

> Kazuki Yamaguchi <k@rhe.jp> writes:
> 
> > [1/5]
> > Adds RESOLVE_REF_COMMON_DIR to resolve_ref_unsafe(). The second - fourth
> > patch depend on this. At the same time, this allows us to remove
> > reimplementation of resolve_ref_unsafe() in worktree.c: parse_ref().
> >
> > [2/5]
> > Adds REF_COMMON_DIR flag to lock_ref_sha1_basic().
> 
> While the code reduction in 1/5 is a very good and welcome change,
> these new flags make me wonder if they can be easily misused in a
> way that contradicts with what other parts of the system (e.g.
> path.c::common_list[]) tell us.  Am I worried too much without a
> good reason?
>

Umm... Certainly I can't come up with other use cases of there flags, especially the latter one. I'll reconsider.

Show 12 quoted lines

> > [3/5]
> > Adds create_symref_common_dir(). Same as create_symref() except it
> > doesn't consider $GIT_DIR. create_symref_common_dir("HEAD", some) always
> > updates .git/HEAD. The next patch uses this.
> 
> Similarly, would this allow updating "refs/remotes/origin/HEAD"
> (which is also a symbolic ref) to different values for different
> worktrees?  In other words, I am wondering if this new function
> should be narrower in scope--e.g. used only to update "HEAD" and
> no other symbolic refs.  Or will the additional flexibility be
> useful?
>

For non-per-worktree ref names, create_symref_common_dir() and create_symref() should work in the same way. But yes, as far as I know, there is no other chance to update other worktree's per-worktree symrefs.

How about like this (will update per-worktree-gitdir/HEAD): set_worktree_head_symref("per-worktree-gitdir", "refs/heads/master")

Thanks,

Show 13 quoted lines

> > [4/5]
> > Fixes the issue of git branch -m.
> > The behavior when one failed has changed from v1: print an error and
> > continue.
> >
> >   % git branch -m oldname newname
> >   error: Unable to create '/path/to/.git/worktrees/wt/HEAD.lock': Permission denied
> >   error: HEAD of working tree /path/to/wt is not updated.
> >   error: Unable to create '/path/to/.git/worktrees/wt2/HEAD.lock': Permission denied
> >   error: HEAD of working tree /path/to/wt2 is not updated.
> >   fatal: Branch renamed to newname, but HEAD is not updated!
> 
> Sounds like a good goal.