git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johan Herland <johan@herland.net>
Cc: David Turner <dturner@twopensource.com>,
	Git mailing list <git@vger.kernel.org>,
	Michael Haggerty <mhagger@alum.mit.edu>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Philip Oakley <philipoakley@iee.org>
Subject: Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs
Date: Tue, 28 Jul 2015 17:33:23 -0700	[thread overview]
Message-ID: <xmqqlhdzby5o.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CALKQrgfxc83-yjrCWZqC+pyPhbQDgYbrpCDSaBk78YypO=BXOg@mail.gmail.com> (Johan Herland's message of "Wed, 29 Jul 2015 01:43:28 +0200")

Johan Herland <johan@herland.net> writes:

> Here is where we start to differ. I would say that starting a notes
> merge is completely unrelated to your worktree. Consider this:
> ...
> This is not the case for notes merges. If I start a notes merge from
> worktree A, there is no "occupation" of that worktree. Before the
> notes merge is resolved, I can do whatever I want in worktree A
> (including checking out a different branch, performing a rebase,
> whatever...). Instead, the notes merge creates its own worktree (that
> is "occupied" until the notes merge is completed), which is completely
> unrelated to worktree A.

That does not mean the notes merge that you started when you were
sitting in worktree A has to be shared with worktree B, by say doing
"vi .git/NOTES_MERGE_WORKTREE/$commit && git notes merge --commit".

Also the above does not explain why it is sensible for you to forbid
worktree B from doing an unrelated notes merge of a different ref
under refs/notes/* while your worktree A is doing a notes merge.

> In principle, I agree that an ongoing notes-merge into
> refs/notes/someotherthing should be able to coexist with an ongoing
> notes-merge into refs/notes/commits. However, it does not make sense
> to bind those notes-merges to a specific worktree.

The thing is, the choice is between per worktree or per repository.
Taking the latter would mean you can only be doing one notes merge
at a time, even though you prepared multiple worktrees so that you
can work on different things at a time.  It is true that there is
nothing inherent that ties a note merge to a worktree (a worktree is
tied to a branch that is checed out, and there is no tie between a
branch and a notes tree), but "not inherantly tied to" does not
automatically mean "has to be one per repository".  You'd ideally
want to allow N workspaces for N refs/notes/* refs.

But people work in worktrees, and that is their unit of working
space.  From that point of view, unless you are proposing a
completely different design where the primary worktree can be used
only for manipulating notes (hence, you can have worktrees for
resolving refs/notes/A and refs/notes/B, in addition to the other
worktrees you use to advance branches), treating NOTES_MERGE_REF as
a per-worktree entity just like HEAD and the index is, would be the
most sensible comporise.

> Let's say I have two worktrees, A and B, and from worktree A, I have
> started a notes-merge into refs/notes/commits. Now:
>
>  - From worktree B I should NOT be able to start another notes-merge
> into refs/notes/commits.

Correct.

>  - From worktree B I SHOULD be able to start another notes-merge into
> refs/notes/someotherthing

Correct.

> But this doesn't really have anything to do with worktree B. 

Correct. There is nothing inherent that ties someotherthing to B and
commits to A.  The only reason why I think "per worktree" is vastly
superiour than "only one per repository" is because the end user did
set up two worktrees so that he can do two things at once
independently.

> I can
> just as easily say:
>
>  - From worktree A I should NOT be able to start another notes-merge
> into refs/notes/commits.

Correct.

>  - From worktree A I SHOULD be able to start another notes-merge into
> refs/notes/someotherthing

Irrelevant and moving the goalpost.  We are not talking about
extending capability of the system that does not use multi-worktree.
We do not do two regular merges in a single worktree at a time, and
I do not think it is sensible to claim "I SHOULD be able to".

> Now, we do not yet support simultaneous notes merges at all, but my
> follow-on point is that the addition of such support is wholly
> independent of the multi-worktree support.

Now we are getting somewhere.  So is there more that is needed than
separating NOTES_MERGE_REF per worktree to make this work (remember,
multiple notes-merge in a single worktree is a non-goal, just like
multiple merge in a single worktree is not supported today and will
not be)?  Is there some other state that is not captured by
NOTES_MERGE_REF and friends that you would end up recording a wrong
merge result, if two worktrees that have NOTES_MERGE_REF pointing at
a different ref in refs/notes/* try to do the notes-merge at the
same time?

> For now, it would make more
> sense to only allow a single notes-merge across all worktrees. I.e.
> when starting a notes-merge from ANY worktree, it should simply fail
> if there is an existing unresolved notes merge (no matter which
> worktree started that unresolved notes merge).

I do not understand why we need to have such a restriction.  It is
like saying you may have two worktrees but you cannot merge into
master in worktree A if you are doing a merge into next in worktree
B.  I'd need a clear answer to the question in the previous
paragraph to say something like "ah, ok, that limitation (either
imposed by the current code design, or perhaps reliance of the
filesystem, or whatever) I didn't think about, and now what you say
makes sense to me".

> Disagree. The private area used to resolve notes merges should be per
> _repo_, not per worktree.

I can buy "the private area for resolving refs/notes/common must be
a single place per repo that is differnet from the private area used
for refs/notes/somethingelse, which would also have its own single
place per repo".

But then what you are talkinng about is per _ref_, not per _repo_.

And I'd very much love to see this per _ref_; that is fantastic.

Because the suggestion is to allow a single notes ref refs/notes/*
to be "checked out" only in a single worktree at a time, that per
_ref_ thing falls out naturally from per _worktree_ arrangement.

And that is how people work, by creating a separate worktree, they
gain one additional workspace.

  reply	other threads:[~2015-07-29  0:33 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-28 18:12 [PATCH v3 0/6] pseudorefs David Turner
2015-07-28 18:12 ` [PATCH v3 1/6] refs: Introduce pseudoref and per-worktree ref concepts David Turner
2015-07-28 18:12 ` [PATCH v3 2/6] notes: replace pseudorefs with real refs David Turner
2015-07-28 19:00   ` Junio C Hamano
2015-07-28 19:24     ` David Turner
2015-07-28 19:44     ` Junio C Hamano
2015-07-28 21:23       ` [PATCH] notes: handle multiple worktrees David Turner
2015-07-28 21:42         ` David Turner
2015-07-28 22:00           ` Junio C Hamano
2015-07-28 22:12         ` Junio C Hamano
2015-07-28 22:50           ` Johan Herland
2015-08-03 13:27             ` Duy Nguyen
2015-07-28 22:17         ` Eric Sunshine
2015-07-28 22:38       ` [PATCH v3 2/6] notes: replace pseudorefs with real refs Johan Herland
2015-07-28 22:52         ` Junio C Hamano
2015-07-28 23:43           ` Johan Herland
2015-07-29  0:33             ` Junio C Hamano [this message]
2015-07-29  0:56               ` Michael Haggerty
2015-07-29  1:23                 ` Jacob Keller
2015-07-29  1:24                 ` Johan Herland
2015-07-29  2:25                   ` Junio C Hamano
2015-07-29  2:00                 ` Junio C Hamano
2015-07-29  2:53                   ` Johan Herland
2015-07-29  5:00                     ` Junio C Hamano
2015-07-29  2:34               ` Johan Herland
2015-07-29  5:01                 ` Junio C Hamano
2015-07-29 13:19                   ` Johan Herland
2015-07-29 16:37                     ` Junio C Hamano
2015-07-29 16:58                       ` Junio C Hamano
2015-07-30  6:05                       ` Johan Herland
2015-07-30 16:24                         ` Junio C Hamano
2015-07-29  2:17             ` Junio C Hamano
2015-07-29  2:37               ` Johan Herland
2015-07-28 18:12 ` [PATCH v3 3/6] refs: add ref_type function David Turner
2015-07-29  6:32   ` Eric Sunshine
2015-07-28 18:12 ` [PATCH v3 4/6] pseudorefs: create and use pseudoref update and delete functions David Turner
2015-07-29  6:38   ` Eric Sunshine
2015-07-28 18:12 ` [PATCH v3 5/6] rebase: use update_ref David Turner
2015-07-28 18:12 ` [PATCH v3 6/6] sequencer: replace write_cherry_pick_head with update_ref David Turner
2015-07-28 19:01 ` [PATCH v3 0/6] pseudorefs Junio C Hamano
2015-07-28 19:07   ` David Turner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqlhdzby5o.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=johan@herland.net \
    --cc=mhagger@alum.mit.edu \
    --cc=philipoakley@iee.org \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).