Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-30 Thread Junio C Hamano 
Johan Herland  writes:

>> Actually, the name "linked worktree" is probably a misnomer.
>> ...
> Makes sense, although currently, IINM, those multiple $GIT_DIRs must
> be associated with strictly different branches, which is completely
> unrelated to the desired notes-merge restriction (which applies to
> notes refs - not branches). But this has been discussed to death,
> already.

Yeah, when we enhance "linked worktree" to make it easier to create
"bare worktree", we'd need to make sure it is easy to make them in
the detached HEAD state, i.e. not tied to any particular branch.

>> The user could attempt to start different notes merges in her
>> multiple $GIT_DIRs.  The question is to what degree we want to
>> support her.
>>
>> Is it sufficient to have these per $GIT_DIR, when the user has two
>> $GIT_DIRs connected to the same repository and wants to do two
>> "notes merge" acting on different ref in refs/notes/*?  Or are there
>> some other states in the shared part kept, which would be stomped on
>> by simultaneously running "notes merge" instances in different
>> $GIT_DIRs, that make this not to work?  Any other problems in the
>> remainder of the current implementation of "notes merge"?
>
> Still, I believe the answer is "No".

Good.  So the following would be a viable way forward.

>> But if there isn't, "[PATCH] notes: handle multiple worktrees" is
>> the absolute minimum thing we could
>> do to make "notes merge" usable by making sure that the user does
>> not attempt merging the same refs/notes/commits in two different
>> places.
>
> Sure. There's no point in delaying a patch that works well in practice
> just because I have a delusion of a theoretically cleaner solution
> that won't make any difference in practice.

I think the part of that patch that refactors the mechanism to
notice and disallow symrefs pointing at the same thing (with various
tweaks suggested during the review) is a good idea in the longer
term.  If we were to switch to a "you can do multiple notes merges
inside a single repository without any additional linked worktree"
paradigm in the future, we would not want (or need) to use that
"avoid pointing this symref to the same thing" code for notes merge
codepath, but that is not something we need to decide now.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-29 Thread Johan Herland 
On Wed, Jul 29, 2015 at 6:37 PM, Junio C Hamano  wrote:
> Johan Herland  writes:
>> On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano  wrote:
>>> Johan Herland  writes:
>>>
 I believe it is a bad compromise. It complicates the code, and it
 provides a concurrent notes merges that is unnecessarily tied to (and
 dependent on) worktrees. For example, if I wanted to perform N
 concurrent notes merges in a project that happens to have a huge
 worktree, I would now have to create N copies of the huge worktree...
>>>
>>> Who said worktree has to be populated?  You should be able to have
>>> an absolutely empty checkout just like "clone --no-checkout".
>>
>> IMHO that's an insane workaround that only serves to highlight the
>> conceptual problems of binding notes merges (as they are implemented
>> today) to worktrees.
>
> Actually, the name "linked worktree" is probably a misnomer.
>
> There is nothing fundamental in the mechanism or in the concept that
> says that these multiple $GIT_DIR's must not be a bare one.  The
> main thing the separation between $GIT_DIR and $GIT_COMMON_DIR
> affords you is that you can have some things shared across them
> (e.g. refs/*, objects) while making others per $GIT_DIR (e.g. HEAD,
> index, etc.).
>
> With that in mind, it is not an insane workaround but a very natural
> mechanism suited exactly for what you want to do: using a feature
> (e.g. "notes merge") that currently can have at most one instance
> running at a time because it stores its state inside $GIT_DIR, and
> you want to have N concurrent one going.  You keep that "state per
> running instance" inside $GIT_DIR (i.e. not shared) and use the
> "linked worktree" mechanism to have multiple $GIT_DIR, connected
> to the same $GIT_COMMON_DIR.

Makes sense, although currently, IINM, those multiple $GIT_DIRs must
be associated with strictly different branches, which is completely
unrelated to the desired notes-merge restriction (which applies to
notes refs - not branches). But this has been discussed to death,
already.

>> But, whatever. This is unrelated to David's current effort, and I
>> don't want to hold that up, so please move along, nothing to see here.
>
> I need this part from an earlier message answered to unblock David's
> topic:
>
> 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?

I believe the answer to both questions is "No".

> If we do not change anything (not even applying the "[PATCH] notes: handle 
> multiple worktrees" patch
> we are discussing), all these things prefixed with NOTES_ will
> become per $GIT_DIR with linked worktrees.
>
> NOTES_EDITMSG, NOTES_MERGE_REF, NOTES_MERGE_PARTIAL,
> NOTES_MERGE_WORKTREE
>
> The user could attempt to start different notes merges in her
> multiple $GIT_DIRs.  The question is to what degree we want to
> support her.
>
> Is it sufficient to have these per $GIT_DIR, when the user has two
> $GIT_DIRs connected to the same repository and wants to do two
> "notes merge" acting on different ref in refs/notes/*?  Or are there
> some other states in the shared part kept, which would be stomped on
> by simultaneously running "notes merge" instances in different
> $GIT_DIRs, that make this not to work?  Any other problems in the
> remainder of the current implementation of "notes merge"?

Still, I believe the answer is "No".

> If there are reasons/limitations that make simultaneous "notes
> merge" of different notes in different $GIT_DIRs impossible, then I
> agree we shouldn't bother with "[PATCH] notes: handle multiple worktrees" 
> patch.  We should just
> declare "do not do it, it does not (yet) work".
>
> But if there isn't, "[PATCH] notes: handle multiple worktrees" is the 
> absolute minimum thing we could
> do to make "notes merge" usable by making sure that the user does
> not attempt merging the same refs/notes/commits in two different
> places.

Sure. There's no point in delaying a patch that works well in practice
just because I have a delusion of a theoretically cleaner solution
that won't make any difference in practice.


...Johan

-- 
Johan Herland, 
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-29 Thread Junio C Hamano 
Junio C Hamano  writes:

> If we do not change anything (not even applying the [v3 2/6] patch
> we are discussing),...

This one and...

> If there are reasons/limitations that make simultaneous "notes
> merge" of different notes in different $GIT_DIRs impossible, then I
> agree we shouldn't bother with [v3 2/6] patch.  We should just

... this one and ...

> declare "do not do it, it does not (yet) work".
>
> But if there isn't, [v3 2/6] is the absolute minimum thing we could

... this one.  All of these reference to [v3 2/6] are wrong.

The patch I meant was "[PATCH] notes: handle multiple worktrees",
http://thread.gmane.org/gmane.comp.version-control.git/274803/focus=274852

Sorry for the confusion.


> do to make "notes merge" usable by making sure that the user does
> not attempt merging the same refs/notes/commits in two different
> places.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-29 Thread Junio C Hamano 
Johan Herland  writes:

> On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano  wrote:
>> Johan Herland  writes:
>>
>>> I believe it is a bad compromise. It complicates the code, and it
>>> provides a concurrent notes merges that is unnecessarily tied to (and
>>> dependent on) worktrees. For example, if I wanted to perform N
>>> concurrent notes merges in a project that happens to have a huge
>>> worktree, I would now have to create N copies of the huge worktree...
>>
>> Who said worktree has to be populated?  You should be able to have
>> an absolutely empty checkout just like "clone --no-checkout".
>
> IMHO that's an insane workaround that only serves to highlight the
> conceptual problems of binding notes merges (as they are implemented
> today) to worktrees.

Actually, the name "linked worktree" is probably a misnomer.

There is nothing fundamental in the mechanism or in the concept that
says that these multiple $GIT_DIR's must not be a bare one.  The
main thing the separation between $GIT_DIR and $GIT_COMMON_DIR
affords you is that you can have some things shared across them
(e.g. refs/*, objects) while making others per $GIT_DIR (e.g. HEAD,
index, etc.).

With that in mind, it is not an insane workaround but a very natural
mechanism suited exactly for what you want to do: using a feature
(e.g. "notes merge") that currently can have at most one instance
running at a time because it stores its state inside $GIT_DIR, and
you want to have N concurrent one going.  You keep that "state per
running instance" inside $GIT_DIR (i.e. not shared) and use the
"linked worktree" mechanism to have multiple $GIT_DIR, connected
to the same $GIT_COMMON_DIR.

> But, whatever. This is unrelated to David's current effort, and I
> don't want to hold that up, so please move along, nothing to see here.

I need this part from an earlier message answered to unblock David's
topic:

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?

If we do not change anything (not even applying the [v3 2/6] patch
we are discussing), all these things prefixed with NOTES_ will
become per $GIT_DIR with linked worktrees.

NOTES_EDITMSG, NOTES_MERGE_REF, NOTES_MERGE_PARTIAL,
NOTES_MERGE_WORKTREE

The user could attempt to start different notes merges in her
multiple $GIT_DIRs.  The question is to what degree we want to
support her.

Is it sufficient to have these per $GIT_DIR, when the user has two
$GIT_DIRs connected to the same repository and wants to do two
"notes merge" acting on different ref in refs/notes/*?  Or are there
some other states in the shared part kept, which would be stomped on
by simultaneously running "notes merge" instances in different
$GIT_DIRs, that make this not to work?  Any other problems in the
remainder of the current implementation of "notes merge"?

If there are reasons/limitations that make simultaneous "notes
merge" of different notes in different $GIT_DIRs impossible, then I
agree we shouldn't bother with [v3 2/6] patch.  We should just
declare "do not do it, it does not (yet) work".

But if there isn't, [v3 2/6] is the absolute minimum thing we could
do to make "notes merge" usable by making sure that the user does
not attempt merging the same refs/notes/commits in two different
places.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-29 Thread Johan Herland 
On Wed, Jul 29, 2015 at 7:01 AM, Junio C Hamano  wrote:
> Johan Herland  writes:
>
>> I believe it is a bad compromise. It complicates the code, and it
>> provides a concurrent notes merges that is unnecessarily tied to (and
>> dependent on) worktrees. For example, if I wanted to perform N
>> concurrent notes merges in a project that happens to have a huge
>> worktree, I would now have to create N copies of the huge worktree...
>
> Who said worktree has to be populated?  You should be able to have
> an absolutely empty checkout just like "clone --no-checkout".

IMHO that's an insane workaround that only serves to highlight the
conceptual problems of binding notes merges (as they are implemented
today) to worktrees.

I'm much more excited about Michael's idea of formalising the concept
of a notes tree checkout (although as already discussed, checking out
a notes tree is different from checking out a "regular" tree). Then, a
notes merge (as you already suggested) should be implemented by
creating a linked worktree whose HEAD is the notes ref being merged
into. I believe there are potential complications here as well (where
a notes-merge might behave differently from a regular merge), but
those should be solvable.

But, whatever. This is unrelated to David's current effort, and I
don't want to hold that up, so please move along, nothing to see here.


...Johan

-- 
Johan Herland, 
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
Johan Herland  writes:

> I believe it is a bad compromise. It complicates the code, and it
> provides a concurrent notes merges that is unnecessarily tied to (and
> dependent on) worktrees. For example, if I wanted to perform N
> concurrent notes merges in a project that happens to have a huge
> worktree, I would now have to create N copies of the huge worktree...

Who said worktree has to be populated?  You should be able to have
an absolutely empty checkout just like "clone --no-checkout".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
Johan Herland  writes:

> On Wed, Jul 29, 2015 at 4:00 AM, Junio C Hamano  wrote:
>
>> So doing the absolute minimum, leaving the "now what can we do to
>> improve notes-merge process?" outside the scope of the topic.
>
> That's exactly what I was also trying to do: David's topic should not
> have to deal with NOTES_MERGE_* at all. Simply leave it all as
> something that belongs in $GIT_COMMON_DIR, and let's solve concurrent
> unrelated notes merges as a wholly independent, separate topic.

Here, it perhaps is showing that you are unfamiliar with the linked
worktree topic.

Things directly under .git/, like HEAD, MERGE_HEAD, etc., are per
worktree (i.e. not shared across working trees).  You have to work
to make them shared, i.e. per repo.  David's earlier one downcased
them while still keeing them directly under .git/ but I think it is
ugly and in general a wrong way to mark what is shared and what is
per worktree.  E.g. index is not shared, even though the name is all
lowercase.

You will be updating that mechanism when you truly make the
notes-merge a per refs/notes/* ref thing (not per repo, not per
worktree).  Perhaps you will use refs/notes-merge/$foo that is
shared across worktrees as the replacement for NOTES_MERGE_REF that
currently is used to merge into refs/notes/$foo, or something like
that, to pursue your "per ref" goal.  At that point, NOTES_MERGE_REF
based design needs to be scrapped anyway; it should not matter if
NOTES_MERGE_REF is per worktree (not shared) or per repo (shared) in
the meantime.

Doing absolute minimum means that $GIT_DIR/NOTES_MERGE_REF should be
left per worktree, like MERGE_HEAD, etc., without doing anything
like downcasing or moving it under refs/notes-merge/ (which I think
is a better way to make it shared, if we wanted to), which is
additional special casing that will be a wasted effort that would
not matter in the end result.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Johan Herland 
On Wed, Jul 29, 2015 at 4:00 AM, Junio C Hamano  wrote:
> Michael Haggerty  writes:
>> It sounds like what a notes merge really wants is a new linked worktree
>> that has branch refs/notes/foo checked out:
>>
>> * This would allow multiple notes merges to take place at the same time
>> provided they target different merge references.
>>
>> * This would prevent multiple notes merges to the same notes reference
>> at the same time by the same mechanism that prevents the same branch
>> from being checked out in two linked worktrees at the same time.
>>
>> It's just a thought; I have no idea whether it is practical...
>
> That was certainly one of the possibilities that crossed my mind.
>
> In any case, the primary thing I am interested in at this point is
> to unblock David's "prepare things so that we can put primary refs
> in a different ref backends more easily" topic, and I've already
> made my point a few messages ago upstream:
>
> I think it is OK for us to admit that the "notes" subsystem is
> not quite ready to work well with multiple working tree world
> yet [*1*], and move this series forward without worrying about
> them.
>
> So doing the absolute minimum, leaving the "now what can we do to
> improve notes-merge process?" outside the scope of the topic.

That's exactly what I was also trying to do: David's topic should not
have to deal with NOTES_MERGE_* at all. Simply leave it all as
something that belongs in $GIT_COMMON_DIR, and let's solve concurrent
unrelated notes merges as a wholly independent, separate topic.

...Johan

-- 
Johan Herland, 
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Johan Herland 
On Wed, Jul 29, 2015 at 4:17 AM, Junio C Hamano  wrote:
> Perhaps you meant by "per repo" to mean "per $GIT_DIR" in this
> message, and if that is the case, then I think we are in agreement.

No, by per repo I mean per $GIT_COMMON_DIR (I haven't followed the
linked worktree effort, so this language is new to me. My apologies).

...Johan

-- 
Johan Herland, 
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Johan Herland 
On Wed, Jul 29, 2015 at 2:33 AM, Junio C Hamano  wrote:
> Johan Herland  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.

I do not argue that it is sensible to forbid concurrent unrelated
notes merges. I argue that using linked worktrees is a poor solution
for concurrent unrelated notes merges.

A better solution does not concern itself with worktrees at all, and
does not need to add nonsensical conditions like:

die_if_checked_out("NOTES_MERGE_REF", default_notes_ref());

A better solution does not need to add complexity to the branch or
linked worktree code to deal with notes merges. Instead, it simply
organizes the notes merge worktrees in such a manner that the correct
semantics naturally emerge.

Again, let's compare the two approaches (against the current situation):

Current situation:
 - A single $GIT_COMMON_DIR/NOTES_MERGE_*
 - Concurrent (unrelated or not) notes merges are simply not supported

Proposal A (please correct me where I have misunderstood what's proposed):
 - Each worktree has its own $GIT_DIR/NOTES_MERGE_*
 - Concurrent unrelated notes merges are supported, provided that you
create an additional linked worktree to "host" each concurrent notes
merge.
 - Logic must be added to ensure unrelated-ness, i.e. make sure that
the NOTES_MERGE_REF in worktree X is different from all other
worktrees' NOTES_MERGE_REF.

Proposal B:
 - The repo has a $GIT_COMMON_DIR/notes-merge/$ref/* hierarchy for
organizing concurrent notes merges
 - Concurrent unrelated notes merges are supported, independently of
whether you have zero, one, or several worktrees.
 - The notes merge code must be adjusted to work with the above
hierarchy, and must naturally fail if the user attempts to start a new
notes merge that would clobber an in-progress notes merge (only notes
merges to the same notes ref will clobber).

I obviously feel proposal B is superior to A, so I wonder what I have
missed about A that makes it preferable.

>> 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.

I believe it is a bad compromise. It complicates the code, and it
provides a concurrent notes merges that is unnecessarily tied to (and
dependent on) worktrees. For example, if I wanted to perform N
concurrent notes merges in a project that happens to have a huge
worktree, I would now have to create N copies of the huge worktree...


...Johan

-- 
Johan Herland, 
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
Johan Herland  writes:

> Yes, almost. There are some complications with the concept of
> "checking out" a notes tree:
>
>  - The notes tree fanout must be flattened (so that when merging two
> note trees with different fanout, conflicting notes (e.g. deadbeef...
> and de/adbeef) are turned into a file-level conflict in the notes
> merge worktree (i.e. contents with conflict markers in
> .git/NOTES_MERGE_WORKTREE/deadbeef...).

True.  I however think Michael was envisioning further into the
future, where the tree-level merges would take care of that detail
and populate the index to express conflicts using "canonicalised"
paths, removing these fan-out slashes, before externalising the
conflicted paths that need user's attention on the filesystem.

>  - Notes trees may be very large (e.g. one note per commit for the
> entire project history), so we want to avoid checking out as many
> notes as possible. Currently we only checkout the notes that actually
> do conflict, and keep the rest referenced from NOTES_MERGE_PARTIAL.

This is a valid point, but nobody forces us to do a full checkout to
perform a merge.  From day one, our merge machinery is prepared to
merge in an empty working tree, only checking out paths that need to
be modified by the merge.

>> * This would allow multiple notes merges to take place at the same time
>> provided they target different merge references.
>>
>> * This would prevent multiple notes merges to the same notes reference
>> at the same time by the same mechanism that prevents the same branch
>> from being checked out in two linked worktrees at the same time.
>>
>> It's just a thought; I have no idea whether it is practical...
>
> I'm not sure whether it's worth trying to reuse the same linked
> worktree mechanism for notes trees, when (a) the concept of "checking
> out" a notes tree is so different, as explained above, and (b)
> currently the only use case for a notes worktree is during a notes
> merge.

I have a very similar feeling except that I'd replace "linked
worktree mechanism" with "checkout mechanism".  If we were to
introduce such a new checkout mechanism that flattens a fan-out
paths, with "sparse checkout"-like behaviour, the current "checkout
mechanism" would not be reused, but the resulting system would
benefit from "linked worktree mechanism" just as much as the normal
multiple worktree layout benefits from it.  You'd want to make sure
only one such worktree has the checkout of one refs/notes/* ref,
everything in refs/* is shared across the repository and its
worktrees, and certain things like MERGE_HEAD and the index inside
$GIT_DIR/ are not shared, which are what the linked worktree
mechanism gives us.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
Johan Herland  writes:

> In fact, you can easily do a notes merge in a _bare_ repo...

You keep repeating that but I do not think it is relevant at all.

If you have a bare repository, you either

 (1) do not have any worktree associated with it; or

 (2) have worktrees associated with it elsewhere, but its parent
 directory is *not* the root of any worktree.

If (1), what are found outside GIT_COMMON_DIR (e.g. HEAD) is found
inside GIT_DIR, so if we leave NOTES_MERGE_REF and friends outside
GIT_COMMON_DIR and create the NOTES_MERGE_WORKTREE in GIT_DIR, that
would work just as fine as it currently does.

If (2), what are found outside GIT_COMMON_DIR (e.g. HEAD) is still
found inside GIT_DIR (that is now per worktree, but for your bare
repository, that is the repository directory itself).  Again, if we
leave NOTES_MERGE_REF and friends outside GIT_COMMON_DIR and create
the NOTES_MERGE_WORKTREE in GIT_DIR, that would work just as fine as
it currently does.

And as long as NOTES_MERGE_REF is made per $GIT_DIR ("per worktree"
is the phrase I am refraining deliberately here, because all the
worktrees have their own private area, and in addition, your bare
repository has one, too) that is protected against "multiple
checkout", all worktrees and your bare repository can perform
independent notes-merges.

Perhaps you meant by "per repo" to mean "per $GIT_DIR" in this
message, and if that is the case, then I think we are in agreement.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
Michael Haggerty  writes:

> It sounds like what a notes merge really wants is a new linked worktree
> that has branch refs/notes/foo checked out:
>
> * This would allow multiple notes merges to take place at the same time
> provided they target different merge references.
>
> * This would prevent multiple notes merges to the same notes reference
> at the same time by the same mechanism that prevents the same branch
> from being checked out in two linked worktrees at the same time.
>
> It's just a thought; I have no idea whether it is practical...

That was certainly one of the possibilities that crossed my mind.

In any case, the primary thing I am interested in at this point is
to unblock David's "prepare things so that we can put primary refs
in a different ref backends more easily" topic, and I've already
made my point a few messages ago upstream:

I think it is OK for us to admit that the "notes" subsystem is
not quite ready to work well with multiple working tree world
yet [*1*], and move this series forward without worrying about
them.

So doing the absolute minimum, leaving the "now what can we do to
improve notes-merge process?" outside the scope of the topic.

Improving the notes-merge process is also an interesting topic, but
it is clear that people started thinking about it today ;-), so it
can wait without blocking David's work.  The refs/notes/* hierarchy
will be handled exactly the same way as regular branches in the
pluggable ref-backends, and how the ephemeral REF_NOTES_REF and
friends are represented (some are refs, some may be pseudo-refs,
some may be just a filesystem entity) would need to be revamped if
we really do the "improving notes-merge" thing anyway, so David's
topic shouldn't be blocked by the "possible future tweak" of the
current design that hasn't happened yet.  Instead, improving
notes-merge can be done on top, potentially undoing and redoing
Davi'd topic.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Johan Herland 
On Wed, Jul 29, 2015 at 2:56 AM, Michael Haggerty  wrote:
> Johan Herland  writes:
>> Here is where we start to differ. I would say that starting a notes
>> merge is completely unrelated to your worktree. Consider this:
>
> It sounds like what a notes merge really wants is a new linked worktree
> that has branch refs/notes/foo checked out:

Yes, almost. There are some complications with the concept of
"checking out" a notes tree:

 - The notes tree fanout must be flattened (so that when merging two
note trees with different fanout, conflicting notes (e.g. deadbeef...
and de/adbeef) are turned into a file-level conflict in the notes
merge worktree (i.e. contents with conflict markers in
.git/NOTES_MERGE_WORKTREE/deadbeef...).

 - Notes trees may be very large (e.g. one note per commit for the
entire project history), so we want to avoid checking out as many
notes as possible. Currently we only checkout the notes that actually
do conflict, and keep the rest referenced from NOTES_MERGE_PARTIAL.

> * This would allow multiple notes merges to take place at the same time
> provided they target different merge references.
>
> * This would prevent multiple notes merges to the same notes reference
> at the same time by the same mechanism that prevents the same branch
> from being checked out in two linked worktrees at the same time.
>
> It's just a thought; I have no idea whether it is practical...

I'm not sure whether it's worth trying to reuse the same linked
worktree mechanism for notes trees, when (a) the concept of "checking
out" a notes tree is so different, as explained above, and (b)
currently the only use case for a notes worktree is during a notes
merge.


...Johan

> Michael
>
> --
> Michael Haggerty
> mhag...@alum.mit.edu
>

-- 
Johan Herland, 
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Jacob Keller 
On Tue, Jul 28, 2015 at 5:56 PM, Michael Haggerty  wrote:
> Johan Herland  writes:
>> Here is where we start to differ. I would say that starting a notes
>> merge is completely unrelated to your worktree. Consider this:
>
> It sounds like what a notes merge really wants is a new linked worktree
> that has branch refs/notes/foo checked out:
>
> * This would allow multiple notes merges to take place at the same time
> provided they target different merge references.
>
> * This would prevent multiple notes merges to the same notes reference
> at the same time by the same mechanism that prevents the same branch
> from being checked out in two linked worktrees at the same time.
>
> It's just a thought; I have no idea whether it is practical...
>
> Michael
>

That seems far more flexible and robust to me.

Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Michael Haggerty 
Johan Herland  writes:
> Here is where we start to differ. I would say that starting a notes
> merge is completely unrelated to your worktree. Consider this:

It sounds like what a notes merge really wants is a new linked worktree
that has branch refs/notes/foo checked out:

* This would allow multiple notes merges to take place at the same time
provided they target different merge references.

* This would prevent multiple notes merges to the same notes reference
at the same time by the same mechanism that prevents the same branch
from being checked out in two linked worktrees at the same time.

It's just a thought; I have no idea whether it is practical...

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
Johan Herland  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 s

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Johan Herland 
On Wed, Jul 29, 2015 at 12:52 AM, Junio C Hamano  wrote:
> Johan Herland  writes:
>> However, in any case, notes merges are always per _repo_ and never per
>> _worktree_, so this is all unrelated to the current patch/discussion
>> AFAICS.
>
> Thanks for chiming in, but I actually think you are confused.
>
> "git merge" is always per _repo_ in the sense that the result of a
> merge of a topic to the 'master' is recorded in the 'master' which
> is per-repo.  In the multi-worktree world order, that does not
> change.  What changes is that you could have different worktrees
> that check out different branches.  Worktree A may have 'master'
> checked out and do the merge there to update the tip of 'master'.
> But while worktree A is doing that, worktree B may have 'next'
> checked out and do an unrelated merge there.  Once worktree A leaves
> 'master' by checking out another branch, worktree B is free to check
> out 'master' and do further merges there.  Merging into 'master' is
> per _repo_, but the act of merging is per worktree.

Agreed thus far.

> I think merges of refs/notes/commits and refs/notes/someotherthing
> works exactly the same way.  In worktree A, you may decide to merge
> a notes tree refs/notes/commits with somebody else's.

Here is where we start to differ. I would say that starting a notes
merge is completely unrelated to your worktree. Consider this:

When you start a "regular" (non-notes) merge in worktree A, that merge
will "occupy" worktree A for the purpose of completing the merge, e.g.
conflicting files will be checked out inside worktree A, and it is
obvious that you cannot do other/unrelated things in worktree A until
you have resolved the conflicts and completed the merge. As such, a
regular merge is inextricably bound to a specific worktree.

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.

>  It may
> conflict and you may need to "lock" refs/notes/commits from being
> touched by other worktrees while resolving that, but that does not
> mean other worktrees cannot do a merge of refs/notes/someotherthing
> at all.

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.

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.

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

But this doesn't really have anything to do with worktree B. I can
just as easily say:

 - From worktree A I should NOT be able to start another notes-merge
into refs/notes/commits.

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

My conclusion is therefore that binding a notes merge to a specific
worktree does not make any sense. Preventing simultaneous notes merges
into the same notes ref is something that must be solved per _repo_
(and not per worktree), and since the worktree plays no part in the
resolution/completion of the notes merge, it makes more sense to place
all the notes-merge-related refs/files inside the _repo_, and not
inside the worktree.

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. 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).

>  The temporary area you use for merging notes, i.e. the
> working tree as far as notes merge is concerned, is private to
> worktree A and does not need to be seen by other worktrees.

Disagree. The private area used to resolve notes merges should be per
_repo_, not per worktree. That is IMHO the only sane way to (in the
future) prevent simultaneous notes merges going into the same notes
ref.

> So while you are working on merging and resolving, that intermediate
> state is *NOT* per _repo_ at all.  It is at most per worktree (Yes
> you could extend and have one notes_merge_ref per each refs/notes/*
> ref to make it even finer grained to allow more than one notes merge
> going on inside a single worktree, but I do not think it is worth
> it).

As stated above, my position is that while you are resolving a n

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
Johan Herland  writes:

Johan Herland  writes:

> However, in any case, notes merges are always per _repo_ and never per
> _worktree_, so this is all unrelated to the current patch/discussion
> AFAICS.

Thanks for chiming in, but I actually think you are confused.

"git merge" is always per _repo_ in the sense that the result of a
merge of a topic to the 'master' is recorded in the 'master' which
is per-repo.  In the multi-worktree world order, that does not
change.  What changes is that you could have different worktrees
that check out different branches.  Worktree A may have 'master'
checked out and do the merge there to update the tip of 'master'.
But while worktree A is doing that, worktree B may have 'next'
checked out and do an unrelated merge there.  Once worktree A leaves
'master' by checking out another branch, worktree B is free to check
out 'master' and do further merges there.  Merging into 'master' is
per _repo_, but the act of merging is per worktree.

I think merges of refs/notes/commits and refs/notes/someotherthing
works exactly the same way.  In worktree A, you may decide to merge
a notes tree refs/notes/commits with somebody else's.  It may
conflict and you may need to "lock" refs/notes/commits from being
touched by other worktrees while resolving that, but that does not
mean other worktrees cannot do a merge of refs/notes/someotherthing
at all.  The temporary area you use for merging notes, i.e. the
working tree as far as notes merge is concerned, is private to
worktree A and does not need to be seen by other worktrees.

So while you are working on merging and resolving, that intermediate
state is *NOT* per _repo_ at all.  It is at most per worktree (Yes
you could extend and have one notes_merge_ref per each refs/notes/*
ref to make it even finer grained to allow more than one notes merge
going on inside a single worktree, but I do not think it is worth
it).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Johan Herland 
On Tue, Jul 28, 2015 at 9:44 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>> David Turner  writes:
>>> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
>>> per-worktree.  We don't want multiple notes merges happening at once
>>> (in different worktrees), so we want to make these refs true refs.
>>> ...
>> The two "rev-parse --verify" looks semi-sensible [*1*];
>> notes_merge_partial is all lowercase and it refers to
>> $GIT_DIR/notes_merge_partial, because they are shared across working
>> tree.
>>
>> But then why are $GIT_DIR/notes_merge_worktree/* still checked with
>> "test -f"?  If they are not refs or ref-like things, why should they
>> be downcased?  If they are, why not "rev-parse --verify"?
>>
>> [Footnote]
>>
>> *1* I say "semi-" sensible, because it looks ugly.  All ref-like
>> things immediately below $GIT_DIR/ are all-caps by convention.
>> Perhaps it is a better idea to move it under refs/; "everything
>> under refs/ is shared across working trees" is probably a much
>> better rule than "all caps but HEAD is special".
>
> I am not sure if "we don't want multiple notes merges at once" is a
> valid cause for this inconsistency in the first place.  When you try
> to start merging a notes tree in one place (leaving NOTES_MERGE_REF
> and friends in the ref storage shared across worktrees), should you
> be prevented from merging a different notes tree in another?  Isn't
> the whole point of having refs/notes/ hierarchy to allow different
> notes to hang underneath there, and isn't NOTES_MERGE_REF symref
> about keeping track of which one of them is being worked on in this
> working tree?

I believe the current M.O. for notes merge is to allow only one
simultaneous notes merge per _repo_. AFAIR, the notes merge code is
completely unrelated to the work tree (you could probably even do a
notes merge in a _bare_ repo), so IINM, the NOTES_MERGE_* refs should
be bound to the _repo_, NOT to the worktree.

So if I understand the pseudoref concept correctly, that should make
NOTES_MERGE_* "regular" refs, and not pseudorefs.

> We don't want multiple usual merges at once in different worktrees,
> either; rather, we don't want conflicting updates to the same branch,
> be it done by a merge or a single-parent commit, from different
> worktrees.  And the way we protect ourselves is by forbidding two
> checkout of the same branch by controlling what HEAD can point at.
>
> Making NOTES_MERGE_REF shared across working trees feels like
> solving that "no multiple checkouts" problem by making HEAD shared
> across working trees, ensuring that only one of them can have usable
> HEAD.  Instead, shouldn't we be solving the issue by allowing
> NOTES_MERGE_REF in multiple working trees point at different refs
> but ensuring that there is at most one working tree that updates one
> particular ref in refs/notes/ hierarchy?  Can't we learn from (and
> even better, reuse) the logic that controls HEAD for this purpose?
>
> I think it is OK for us to admit that the "notes" subsystem is not
> quite ready to work well with multiple working tree world yet [*1*],
> and move this series forward without worrying about them.
>
>
> [Footnote]
>
> *1* I am not saying that the notes subsystem can forever stay to be
> second class citizen that does not know about multiple worktrees
> or pluggable ref backends.  But my brief scan of builtin/notes.c
> seems to indicate that there are quite a lot of work to be done
> to prepare it for these two big changes.  My gut feeling is
> that:
>
> - NOTES_MERGE_REF symref is like HEAD, that is per working tree
>   but is controlled across working trees---no two working trees
>   can "checkout" the same refs/notes/* tree for conflict
>   resolution at the same time.

This sentence does not make sense to me. IMHO, a notes merge has
nothing to do with the worktree at all. Instead, the notes merge
creates its _own_ worktree (under .git/NOTES_MERGE_WORKTREE) when
needed (i.e. when there are notes conflicts to be resolved).

>   It should be all-caps and live
>   directly under $GIT_DIR/.

It should definitely live under $GIT_DIR (and not inside each
worktree's .git/). Whether or not it's all-caps is not that important
to me.

>
> - NOTES_MERGE_PARTIAL is like MERGE_HEAD or the index in that it
>   is merely a state of in-progress merge that is private to a
>   single working tree.  $GIT_DIR/NOTES_MERGE_PARTIAL is just
>   fine, and we do not have to change it.

Agreed.

>
> - NOTES_MERGE_WORKTREE is a temporary area in the filesystem
>   that houses bunch of blob files (i.e. notes contents).  It
>   should be per-working tree but it is not even refs.
>   $GIT_DIR/NOTES_MERGE_WORKTREE is just fine, and we do not have
>   to change it.

>From the notes merge perspective, NOTES_MERGE_WORKTREE _is_ the
worktree. There is no other worktree that is relevant to the notes
merge code. So I'm not qu

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
Junio C Hamano  writes:

> David Turner  writes:
>
>> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
>> per-worktree.  We don't want multiple notes merges happening at once
>> (in different worktrees), so we want to make these refs true refs.
>> ...
> The two "rev-parse --verify" looks semi-sensible [*1*];
> notes_merge_partial is all lowercase and it refers to
> $GIT_DIR/notes_merge_partial, because they are shared across working
> tree. 
>
> But then why are $GIT_DIR/notes_merge_worktree/* still checked with
> "test -f"?  If they are not refs or ref-like things, why should they
> be downcased?  If they are, why not "rev-parse --verify"?
>
> [Footnote]
>
> *1* I say "semi-" sensible, because it looks ugly.  All ref-like
> things immediately below $GIT_DIR/ are all-caps by convention.
> Perhaps it is a better idea to move it under refs/; "everything
> under refs/ is shared across working trees" is probably a much
> better rule than "all caps but HEAD is special".

I am not sure if "we don't want multiple notes merges at once" is a
valid cause for this inconsistency in the first place.  When you try
to start merging a notes tree in one place (leaving NOTES_MERGE_REF
and friends in the ref storage shared across worktrees), should you
be prevented from merging a different notes tree in another?  Isn't
the whole point of having refs/notes/ hierarchy to allow different
notes to hang underneath there, and isn't NOTES_MERGE_REF symref
about keeping track of which one of them is being worked on in this
working tree?

We don't want multiple usual merges at once in different worktrees,
either; rather, we don't want conflicting updates to the same branch,
be it done by a merge or a single-parent commit, from different
worktrees.  And the way we protect ourselves is by forbidding two
checkout of the same branch by controlling what HEAD can point at.

Making NOTES_MERGE_REF shared across working trees feels like
solving that "no multiple checkouts" problem by making HEAD shared
across working trees, ensuring that only one of them can have usable
HEAD.  Instead, shouldn't we be solving the issue by allowing
NOTES_MERGE_REF in multiple working trees point at different refs
but ensuring that there is at most one working tree that updates one
particular ref in refs/notes/ hierarchy?  Can't we learn from (and
even better, reuse) the logic that controls HEAD for this purpose?

I think it is OK for us to admit that the "notes" subsystem is not
quite ready to work well with multiple working tree world yet [*1*],
and move this series forward without worrying about them.


[Footnote]

*1* I am not saying that the notes subsystem can forever stay to be
second class citizen that does not know about multiple worktrees
or pluggable ref backends.  But my brief scan of builtin/notes.c
seems to indicate that there are quite a lot of work to be done
to prepare it for these two big changes.  My gut feeling is
that:

- NOTES_MERGE_REF symref is like HEAD, that is per working tree
  but is controlled across working trees---no two working trees
  can "checkout" the same refs/notes/* tree for conflict
  resolution at the same time.  It should be all-caps and live
  directly under $GIT_DIR/.

- NOTES_MERGE_PARTIAL is like MERGE_HEAD or the index in that it
  is merely a state of in-progress merge that is private to a
  single working tree.  $GIT_DIR/NOTES_MERGE_PARTIAL is just
  fine, and we do not have to change it.

- NOTES_MERGE_WORKTREE is a temporary area in the filesystem
  that houses bunch of blob files (i.e. notes contents).  It
  should be per-working tree but it is not even refs.
  $GIT_DIR/NOTES_MERGE_WORKTREE is just fine, and we do not have
  to change it.

And as to adjusting to the "ref backend with pseudo ref" world,
I think the code is more-or-less ready, especially because your
recent round of this series teaches the "pseudorefs private to
working tree" to update_ref() and delete_ref(), relieving the
ref API users like notes subsystem from having to worry about
them.

So the only potential issue in the notes subsystem is to ensure
NOTES_MERGE_REF from multiple working trees do not point at the
same thing at the same time, with a similar protection we have
for HEAD, I think.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread David Turner 
On Tue, 2015-07-28 at 12:00 -0700, Junio C Hamano wrote:
> David Turner  writes:
> 
> > All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
> > per-worktree.  We don't want multiple notes merges happening at once
> > (in different worktrees), so we want to make these refs true refs.
> >
> > So, we lowercase NOTES_MERGE_REF and friends.  That way, backends
> > that distinguish between pseudorefs and real refs can correctly
> > handle notes merges.
> >
> > This will also enable us to prevent pseudorefs from being updated by
> > the ref update machinery e.g. git update-ref.
> >
> > Signed-off-by: David Turner 
> > ---
> 
> This seems to do a bit more than what it claims to do.  As this kind
> of changes are very error-prone to review, I did a bulk replace of
> the three all-caps NOTES_*THING* and compared the result with what
> this patch gives to spot this:
> 
> > # Fail to finalize merge
> > test_must_fail git notes merge --commit >output 2>&1 &&
> > -   # .git/NOTES_MERGE_* must remain
> > -   test -f .git/NOTES_MERGE_PARTIAL &&
> > -   test -f .git/NOTES_MERGE_REF &&
> > -   test -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&
> > -   test -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&
> > -   test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
> > -   test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
> > +   # .git/notes_merge_* must remain
> > +   git rev-parse --verify notes_merge_partial &&
> > +   git rev-parse --verify notes_merge_ref &&
> > +   test -f .git/notes_merge_worktree/$commit_sha1 &&
> > +   test -f .git/notes_merge_worktree/$commit_sha2 &&
> > +   test -f .git/notes_merge_worktree/$commit_sha3 &&
> > +   test -f .git/notes_merge_worktree/$commit_sha4 &&
> 
> The two "rev-parse --verify" looks semi-sensible [*1*];
> notes_merge_partial is all lowercase and it refers to
> $GIT_DIR/notes_merge_partial, because they are shared across working
> tree. 
> 
> But then why are $GIT_DIR/notes_merge_worktree/* still checked with
> "test -f"?  If they are not refs or ref-like things, why should they
> be downcased?  If they are, why not "rev-parse --verify"?

They are downcased for consistency with the other notes_merge_* stuff.

> [Footnote]
> 
> *1* I say "semi-" sensible, because it looks ugly.  All ref-like
> things immediately below $GIT_DIR/ are all-caps by convention.
> Perhaps it is a better idea to move it under refs/; "everything
> under refs/ is shared across working trees" is probably a much
> better rule than "all caps but HEAD is special".

Do you mean move all current pseudorefs?  Or just the notes stuff? 

Moving MERGE_HEAD means that if someone upgrades in the middle of a
merge, they'll end up confused.  The same is true of the notes stuff,
but I imagine that many fewer people use notes than use git merge, so I
wasn't going to worry about that.  What do you think?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread Junio C Hamano 
David Turner  writes:

> All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
> per-worktree.  We don't want multiple notes merges happening at once
> (in different worktrees), so we want to make these refs true refs.
>
> So, we lowercase NOTES_MERGE_REF and friends.  That way, backends
> that distinguish between pseudorefs and real refs can correctly
> handle notes merges.
>
> This will also enable us to prevent pseudorefs from being updated by
> the ref update machinery e.g. git update-ref.
>
> Signed-off-by: David Turner 
> ---

This seems to do a bit more than what it claims to do.  As this kind
of changes are very error-prone to review, I did a bulk replace of
the three all-caps NOTES_*THING* and compared the result with what
this patch gives to spot this:

>   # Fail to finalize merge
>   test_must_fail git notes merge --commit >output 2>&1 &&
> - # .git/NOTES_MERGE_* must remain
> - test -f .git/NOTES_MERGE_PARTIAL &&
> - test -f .git/NOTES_MERGE_REF &&
> - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha1 &&
> - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha2 &&
> - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
> - test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
> + # .git/notes_merge_* must remain
> + git rev-parse --verify notes_merge_partial &&
> + git rev-parse --verify notes_merge_ref &&
> + test -f .git/notes_merge_worktree/$commit_sha1 &&
> + test -f .git/notes_merge_worktree/$commit_sha2 &&
> + test -f .git/notes_merge_worktree/$commit_sha3 &&
> + test -f .git/notes_merge_worktree/$commit_sha4 &&

The two "rev-parse --verify" looks semi-sensible [*1*];
notes_merge_partial is all lowercase and it refers to
$GIT_DIR/notes_merge_partial, because they are shared across working
tree. 

But then why are $GIT_DIR/notes_merge_worktree/* still checked with
"test -f"?  If they are not refs or ref-like things, why should they
be downcased?  If they are, why not "rev-parse --verify"?

[Footnote]

*1* I say "semi-" sensible, because it looks ugly.  All ref-like
things immediately below $GIT_DIR/ are all-caps by convention.
Perhaps it is a better idea to move it under refs/; "everything
under refs/ is shared across working trees" is probably a much
better rule than "all caps but HEAD is special".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/6] notes: replace pseudorefs with real refs

2015-07-28 Thread David Turner 
All-caps files like NOTES_MERGE_REF are pseudorefs, and thus are
per-worktree.  We don't want multiple notes merges happening at once
(in different worktrees), so we want to make these refs true refs.

So, we lowercase NOTES_MERGE_REF and friends.  That way, backends
that distinguish between pseudorefs and real refs can correctly
handle notes merges.

This will also enable us to prevent pseudorefs from being updated by
the ref update machinery e.g. git update-ref.

Signed-off-by: David Turner 
---
 Documentation/git-notes.txt   | 12 ++---
 builtin/notes.c   | 38 
 notes-merge.c | 10 ++--
 notes-merge.h |  8 ++--
 t/t3310-notes-merge-manual-resolve.sh | 86 +--
 t/t3311-notes-merge-fanout.sh |  6 +--
 6 files changed, 80 insertions(+), 80 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 851518d..82fa3fd 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -103,7 +103,7 @@ merge::
 If conflicts arise and a strategy for automatically resolving
 conflicting notes (see the -s/--strategy option) is not given,
 the "manual" resolver is used. This resolver checks out the
-conflicting notes in a special worktree (`.git/NOTES_MERGE_WORKTREE`),
+conflicting notes in a special worktree (`.git/notes_merge_worktree`),
 and instructs the user to manually resolve the conflicts there.
 When done, the user can either finalize the merge with
 'git notes merge --commit', or abort the merge with
@@ -189,11 +189,11 @@ OPTIONS
 --commit::
Finalize an in-progress 'git notes merge'. Use this option
when you have resolved the conflicts that 'git notes merge'
-   stored in .git/NOTES_MERGE_WORKTREE. This amends the partial
+   stored in .git/notes_merge_worktree. This amends the partial
merge commit created by 'git notes merge' (stored in
-   .git/NOTES_MERGE_PARTIAL) by adding the notes in
-   .git/NOTES_MERGE_WORKTREE. The notes ref stored in the
-   .git/NOTES_MERGE_REF symref is updated to the resulting commit.
+   .git/notes_merge_partial) by adding the notes in
+   .git/notes_merge_worktree. The notes ref stored in the
+   .git/notes_merge_ref symref is updated to the resulting commit.
 
 --abort::
Abort/reset a in-progress 'git notes merge', i.e. a notes merge
@@ -241,7 +241,7 @@ NOTES MERGE STRATEGIES
 
 The default notes merge strategy is "manual", which checks out
 conflicting notes in a special work tree for resolving notes conflicts
-(`.git/NOTES_MERGE_WORKTREE`), and instructs the user to resolve the
+(`.git/notes_merge_worktree`), and instructs the user to resolve the
 conflicts in that work tree.
 When done, the user can either finalize the merge with
 'git notes merge --commit', or abort the merge with
diff --git a/builtin/notes.c b/builtin/notes.c
index 63f95fc..e0b8a02 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -670,14 +670,14 @@ static int merge_abort(struct notes_merge_options *o)
int ret = 0;
 
/*
-* Remove .git/NOTES_MERGE_PARTIAL and .git/NOTES_MERGE_REF, and call
-* notes_merge_abort() to remove .git/NOTES_MERGE_WORKTREE.
+* Remove .git/notes_merge_partial and .git/notes_merge_ref, and call
+* notes_merge_abort() to remove .git/notes_merge_worktree.
 */
 
-   if (delete_ref("NOTES_MERGE_PARTIAL", NULL, 0))
-   ret += error("Failed to delete ref NOTES_MERGE_PARTIAL");
-   if (delete_ref("NOTES_MERGE_REF", NULL, REF_NODEREF))
-   ret += error("Failed to delete ref NOTES_MERGE_REF");
+   if (delete_ref("notes_merge_partial", NULL, 0))
+   ret += error("Failed to delete ref notes_merge_partial");
+   if (delete_ref("notes_merge_ref", NULL, REF_NODEREF))
+   ret += error("Failed to delete ref notes_merge_ref");
if (notes_merge_abort(o))
ret += error("Failed to remove 'git notes merge' worktree");
return ret;
@@ -694,16 +694,16 @@ static int merge_commit(struct notes_merge_options *o)
int ret;
 
/*
-* Read partial merge result from .git/NOTES_MERGE_PARTIAL,
-* and target notes ref from .git/NOTES_MERGE_REF.
+* Read partial merge result from .git/notes_merge_partial,
+* and target notes ref from .git/notes_merge_ref.
 */
 
-   if (get_sha1("NOTES_MERGE_PARTIAL", sha1))
-   die("Failed to read ref NOTES_MERGE_PARTIAL");
+   if (get_sha1("notes_merge_partial", sha1))
+   die("Failed to read ref notes_merge_partial");
else if (!(partial = lookup_commit_reference(sha1)))
-   die("Could not find commit from NOTES_MERGE_PARTIAL.");
+   die("Could not find commit from notes_merge_partial.");
else if (parse_commit(partial))
-   die("Could not parse commit from NOTES_MER