Re: [PATCH] notes: handle multiple worktrees

From: Duy Nguyen <pclouds@gmail.com>
Date: Aug 3, 2015, 13:27 UTC
Message-ID: <CACsJy8B-HV8NS_FCuoy3pvLRr4B7S4CGO4StNxXaB=WLt8=94w@mail.gmail.com>
In-Reply-To: <CALKQrgcX2aPc4cac2p-w8FE9K7X+A5tFWtNcS-N6EOC9gTvfZw@mail.gmail.com>

On Wed, Jul 29, 2015 at 5:50 AM, Johan Herland <johan@herland.net> wrote:

Show 29 quoted lines

> On Wed, Jul 29, 2015 at 12:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> David Turner <dturner@twopensource.com> writes:
>>> Prevent merges to the same notes branch from different worktrees.
>>> Before creating NOTES_MERGE_REF, check NOTES_MERGE_REF using the same
>>> code we use to check that two HEADs in different worktrees don't point
>>> to the same branch.  Modify that code, die_if_checked_out, to take a
>>> "head" ref to examine; previously, it just looked at HEAD.
>>>
>>> Reported-by: Junio C Hamano <gitster@pobox.com>
>>> Signed-off-by: David Turner <dturner@twopensource.com>
>>> ---
>>
>> Thanks for following through.  As I didn't report anything, I do not
>> deserve that label, but it's OK ;-)
>>
>> I know that it is a requirement to protect NOTES_MERGE_REF from
>> being used by multiple places for "notes merge" to play well with
>> the multi-worktree world order, but because I do not know if that is
>> sufficient, I'm asking a few people for further review.
>
> As just stated in a related thread, I don't think it makes sense to
> have NOTES_MERGE_REF per worktree, as the notes merge is always
> completely unrelated to the current worktree (or the current branch
> for that matter). AFAICS this patch is all about handling per-worktree
> NOTES_MERGE_REFs, and as such I'm NAK on this patch. Instead, there
> should only be one NOTES_MERGE_REF per _repo_, and although we might
> want to expand to allow multiple concurrent notes merges in the
> future, that is still a per-repo, and not a per-worktree thing, hence
> completely unrelated to David's current effort.

I agree. Luckily sharing NOTES_MERGE_REF is as short as

diff --git a/path.c b/path.c
index 10f4cbf..52d8ee4 100644
--- a/path.c
+++ b/path.c
@@ -94,7 +94,7 @@ static void replace_dir(struct strbuf *buf, int len,
const char *newdir)
 static const char *common_list[] = {
  "/branches", "/hooks", "/info", "!/logs", "/lost-found",
  "/objects", "/refs", "/remotes", "/worktrees", "/rr-cache", "/svn",
- "config", "!gc.pid", "packed-refs", "shallow",
+ "config", "!gc.pid", "packed-refs", "shallow", "NOTES_MERGE_REF",
  NULL
 };
 --
Duy