Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-20 Thread Jacob Keller
On Fri, Apr 20, 2018 at 1:26 AM, Johannes Schindelin
 wrote:
> Hi Jake,
>
> On Thu, 19 Apr 2018, Jacob Keller wrote:
>
>> On Wed, Apr 18, 2018 at 9:24 PM, Sergey Organov  wrote:
>> > Johannes Schindelin  writes:
>> >
>> >> On Fri, 13 Apr 2018, Phillip Wood wrote:
>> >>
>> >>> On 12/04/18 23:02, Johannes Schindelin wrote:
>> >>> >
>> >>> > [...]
>> >>> >
>> >>> > So: the order of the 3-way merges does matter.
>> >>> >
>> >>> > [...]
>> >>>
>> >>> Those conflicts certainly look intimidating (and the ones in your later
>> >>> reply with the N way merge example still look quite complicated). One
>> >>> option would be just to stop and have the user resolve the conflicts
>> >>> after each conflicting 3-way merge rather than at the end of all the
>> >>> merges. There are some downsides: there would need to be a way to
>> >>> explain to the user that this is an intermediate step (and what that
>> >>> step was); the code would have to do some book keeping to know where it
>> >>> had got to; and it would stop and prompt the user to resolve conflicts
>> >>> more often which could be annoying but hopefully they'd be clearer to
>> >>> resolve because they weren't nested.
>> >>
>> >> I thought about that. But as I pointed out: the order of the merges *does*
>> >> matter. Otherwise we force the user to resolve conflicts that they
>> >> *already* resolved during this rebase...
>> >
>> > How it's relevant to what Phillip suggested? How the order of taking 2
>> > steps, A and B, affects an ability to stop after the first step? It's
>> > still either "A,stop,B" or "B,stop,A", depending on the chosen order.
>> >
>> > What's the _actual_ problem here, if any?
>> >
>> > -- Sergey
>>
>> I believe the order of the merges changes which ones cause conflicts,
>
> That is a correct interpretation of the example I showed.
>
>> but it's possible to generate pre-images (i.e. a set of parents to
>> merge) which cause conflicts regardless of which ordering we pick, so
>> I'm not sure there is a "best ordering".
>
> In general, there is no best ordering, you are right. There is no silver
> bullet.
>
> I am not satisfied with stating that and then leaving it at that.
>
> In the example I presented, you can see that there are common cases where
> there *is* a best ordering. In the wrong order, even if you would force
> the user to resolve the merge conflict in an intermediate merge (which
> would introduce a nightmare for the user interface, I am sure you see
> that), then the next merge would *again* show merge conflicts.
>
> And I, for one, am *really* certain what my decision would be when offered
> the two options 1) force the user to resolve merge conflicts *twice*, or
> 2) reorder the intermediate merges and present the user with exactly one
> set of merge conflicts.
>
> So it is irrelevant that there might not be a "best order" in the general
> case, when in the common cases quite frequently there is.
>
> It is just another example where theory disagrees with practice. Don't get
> me wrong: it is good to start with theory. And likewise it is simply
> necessary to continue from there, and put your theory to the test. And
> then you need to turn this into something practical.
>
> Ciao,
> Dscho

I recall you suggested an approach of "try one way, if there are
conflicts, check the other way and see if it had conflicts".

And I also agree that forcing the user to resolve conflicts in the
middle of the operation is a huge nightmare of a user interface,
probably worse than the issues with nested merge conflicts.

Thanks,
Jake


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-20 Thread Johannes Schindelin
Hi Jake,

On Thu, 19 Apr 2018, Jacob Keller wrote:

> On Wed, Apr 18, 2018 at 9:24 PM, Sergey Organov  wrote:
> > Johannes Schindelin  writes:
> >
> >> On Fri, 13 Apr 2018, Phillip Wood wrote:
> >>
> >>> On 12/04/18 23:02, Johannes Schindelin wrote:
> >>> >
> >>> > [...]
> >>> >
> >>> > So: the order of the 3-way merges does matter.
> >>> >
> >>> > [...]
> >>>
> >>> Those conflicts certainly look intimidating (and the ones in your later
> >>> reply with the N way merge example still look quite complicated). One
> >>> option would be just to stop and have the user resolve the conflicts
> >>> after each conflicting 3-way merge rather than at the end of all the
> >>> merges. There are some downsides: there would need to be a way to
> >>> explain to the user that this is an intermediate step (and what that
> >>> step was); the code would have to do some book keeping to know where it
> >>> had got to; and it would stop and prompt the user to resolve conflicts
> >>> more often which could be annoying but hopefully they'd be clearer to
> >>> resolve because they weren't nested.
> >>
> >> I thought about that. But as I pointed out: the order of the merges *does*
> >> matter. Otherwise we force the user to resolve conflicts that they
> >> *already* resolved during this rebase...
> >
> > How it's relevant to what Phillip suggested? How the order of taking 2
> > steps, A and B, affects an ability to stop after the first step? It's
> > still either "A,stop,B" or "B,stop,A", depending on the chosen order.
> >
> > What's the _actual_ problem here, if any?
> >
> > -- Sergey
> 
> I believe the order of the merges changes which ones cause conflicts,

That is a correct interpretation of the example I showed.

> but it's possible to generate pre-images (i.e. a set of parents to
> merge) which cause conflicts regardless of which ordering we pick, so
> I'm not sure there is a "best ordering".

In general, there is no best ordering, you are right. There is no silver
bullet.

I am not satisfied with stating that and then leaving it at that.

In the example I presented, you can see that there are common cases where
there *is* a best ordering. In the wrong order, even if you would force
the user to resolve the merge conflict in an intermediate merge (which
would introduce a nightmare for the user interface, I am sure you see
that), then the next merge would *again* show merge conflicts.

And I, for one, am *really* certain what my decision would be when offered
the two options 1) force the user to resolve merge conflicts *twice*, or
2) reorder the intermediate merges and present the user with exactly one
set of merge conflicts.

So it is irrelevant that there might not be a "best order" in the general
case, when in the common cases quite frequently there is.

It is just another example where theory disagrees with practice. Don't get
me wrong: it is good to start with theory. And likewise it is simply
necessary to continue from there, and put your theory to the test. And
then you need to turn this into something practical.

Ciao,
Dscho


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-19 Thread Sergey Organov
Hi Jacob,

Jacob Keller  writes:

> On Wed, Apr 18, 2018 at 9:24 PM, Sergey Organov  wrote:
>> Johannes Schindelin  writes:
>>
>>> Hi Phillip,
>>>
>>> On Fri, 13 Apr 2018, Phillip Wood wrote:
>>>
 On 12/04/18 23:02, Johannes Schindelin wrote:
 >
 > [...]
 >
 > So: the order of the 3-way merges does matter.
 >
 > [...]

 Those conflicts certainly look intimidating (and the ones in your later
 reply with the N way merge example still look quite complicated). One
 option would be just to stop and have the user resolve the conflicts
 after each conflicting 3-way merge rather than at the end of all the
 merges. There are some downsides: there would need to be a way to
 explain to the user that this is an intermediate step (and what that
 step was); the code would have to do some book keeping to know where it
 had got to; and it would stop and prompt the user to resolve conflicts
 more often which could be annoying but hopefully they'd be clearer to
 resolve because they weren't nested.
>>>
>>> I thought about that. But as I pointed out: the order of the merges *does*
>>> matter. Otherwise we force the user to resolve conflicts that they
>>> *already* resolved during this rebase...
>>
>> How it's relevant to what Phillip suggested? How the order of taking 2
>> steps, A and B, affects an ability to stop after the first step? It's
>> still either "A,stop,B" or "B,stop,A", depending on the chosen order.
>>
>> What's the _actual_ problem here, if any?
>>
>> -- Sergey
>
> I believe the order of the merges changes which ones cause conflicts,
> but it's possible to generate pre-images (i.e. a set of parents to
> merge) which cause conflicts regardless of which ordering we pick, so
> I'm not sure there is a "best ordering".

I totally agree, but this still does not address the problem of
recursive conflicts, and it's this particular problem that Phillip's
suggestion addresses. Just stop after _every_ conflict and let user
resolve it, whatever the order is. Recursive conflicts are simply
showstoppers. Whatever cleverness is invented to represent them, it will
still outsmart most of the users.

As for your statement, it should be clear the absolute "best ordering"
simply can't exist, as merges are inherently symmetric in the DAG. One
can try all orders in turn and select one that brings less conflicts
though. Comparing conflicts is a problem by itself here. Recursive vs
non-recursive and conflict vs no-conflict are obvious and could be the
only checks adopted, all other cases being considered equal.

If we do select fixed order method, or can't find the best order, the
default order should simply match the natural one, first parent first.
Besides, it's the change to the mainline that is most important for an
actual Git merge, so letting it come first sounds most reasonable.

-- Sergey



Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-19 Thread Jacob Keller
On Wed, Apr 18, 2018 at 9:24 PM, Sergey Organov  wrote:
> Johannes Schindelin  writes:
>
>> Hi Phillip,
>>
>> On Fri, 13 Apr 2018, Phillip Wood wrote:
>>
>>> On 12/04/18 23:02, Johannes Schindelin wrote:
>>> >
>>> > [...]
>>> >
>>> > So: the order of the 3-way merges does matter.
>>> >
>>> > [...]
>>>
>>> Those conflicts certainly look intimidating (and the ones in your later
>>> reply with the N way merge example still look quite complicated). One
>>> option would be just to stop and have the user resolve the conflicts
>>> after each conflicting 3-way merge rather than at the end of all the
>>> merges. There are some downsides: there would need to be a way to
>>> explain to the user that this is an intermediate step (and what that
>>> step was); the code would have to do some book keeping to know where it
>>> had got to; and it would stop and prompt the user to resolve conflicts
>>> more often which could be annoying but hopefully they'd be clearer to
>>> resolve because they weren't nested.
>>
>> I thought about that. But as I pointed out: the order of the merges *does*
>> matter. Otherwise we force the user to resolve conflicts that they
>> *already* resolved during this rebase...
>
> How it's relevant to what Phillip suggested? How the order of taking 2
> steps, A and B, affects an ability to stop after the first step? It's
> still either "A,stop,B" or "B,stop,A", depending on the chosen order.
>
> What's the _actual_ problem here, if any?
>
> -- Sergey

I believe the order of the merges changes which ones cause conflicts,
but it's possible to generate pre-images (i.e. a set of parents to
merge) which cause conflicts regardless of which ordering we pick, so
I'm not sure there is a "best ordering".

Thanks,
Jake


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-18 Thread Sergey Organov
Johannes Schindelin  writes:

> Hi Phillip,
>
> On Fri, 13 Apr 2018, Phillip Wood wrote:
>
>> On 12/04/18 23:02, Johannes Schindelin wrote:
>> > 
>> > [...]
>> > 
>> > So: the order of the 3-way merges does matter.
>> >
>> > [...]
>> 
>> Those conflicts certainly look intimidating (and the ones in your later
>> reply with the N way merge example still look quite complicated). One
>> option would be just to stop and have the user resolve the conflicts
>> after each conflicting 3-way merge rather than at the end of all the
>> merges. There are some downsides: there would need to be a way to
>> explain to the user that this is an intermediate step (and what that
>> step was); the code would have to do some book keeping to know where it
>> had got to; and it would stop and prompt the user to resolve conflicts
>> more often which could be annoying but hopefully they'd be clearer to
>> resolve because they weren't nested.
>
> I thought about that. But as I pointed out: the order of the merges *does*
> matter. Otherwise we force the user to resolve conflicts that they
> *already* resolved during this rebase...

How it's relevant to what Phillip suggested? How the order of taking 2
steps, A and B, affects an ability to stop after the first step? It's
still either "A,stop,B" or "B,stop,A", depending on the chosen order.

What's the _actual_ problem here, if any?

-- Sergey


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-17 Thread Sergey Organov
Hi Jacob,

Jacob Keller  writes:

> On Wed, Apr 11, 2018 at 10:42 PM, Sergey Organov  wrote:
>> Hi Jacob,
>>
>> Jacob Keller  writes:
>>> On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov  wrote:
 It was rather --recreate-merges just a few weeks ago, and I've seen
 nobody actually commented either in favor or against the
 --rebase-merges.

 git rebase --rebase-merges

>>>
>>> I'm going to jump in here and say that *I* prefer --rebase-merges, as
>>> it clearly mentions merge commits (which is the thing that changes).
>>
>> OK, thanks, it's fair and the first argument in favor of --rebase-merges
>> I see.
>>
>
> I'd be ok with "--keep-merges" also. I don't like the idea of
> "flatten" as it, to me, means that anyone who wants to understand the
> option without prior knowledge must immediately read the man page or
> they will be confused. Something like "--rebase-merges" at least my
> coworkers got it instantly. The same could be said for "--keep-merges"
> too, but so far no one I asked said the immediately understood
> "--no-flatten".

If they got --rebase-merges instantly, they should already have known
what "rebase" and "merge" mean. If so, they are likely Git users that
are already familiar with "git rebase" and thus at least heard about a
buddy called --preserve-merges. If it's the case indeed, the outcome
you've got was rather predictable, me thinks.

Now, what are the consequences?

When pleasing maximum number of users of --preserve-merges (and probably
--recreate-merges) is number one target of design, while the rest of
issues are secondary, being in favor of --rebase-merges, --keep-merges,
or ---merges is only natural indeed.

However, I don't believe meeting user expectations should be the number
one criteria of a good design. Sound technical design should come first,
and meeting user expectations, provided they don't contradict the
design, only second. That's how Git was born, that's how it should
continue to evolve. Going in reverse direction, from user expectations
to design, will give us Bzr, not Git.

In discussing of these patch series though I rather see care for user
expectations or preferences being used as an excuse for questionable
design all the time. That's what actually bothers me much more than
choosing particular names for particular options.

Narrowing back to the topic, don't you see, honestly, that there is
something wrong with:

git rebase --rebase-merges

that is supposedly easy to understand even without referring to the
manual, yet when you do happen to refer to the manual, you suddenly
realize it's not that easy to understand:

--rebase-merges[=(rebase-cousins|no-rebase-cousins)]
Rebase merge commits instead of flattening the history by replaying
merges. Merge conflict resolutions or manual amendments to merge
commits are not rebased automatically, but have to be applied
manually.

???

Please read the description. Actually read as if you never knew what's
all this about.

Why does it use "flattening the history" that is supposedly hard to
understand to explain "--rebase-merges" that is supposedly easy to
understand? How comes? And if it's actually a good explanation, why
didn't author just call the option --no-flatten-history, to match its
description?

Next, what is "replaying merges", exactly? That's explaining one term
with another that has not being explained and sounds being even more
vague.

Further, "Merge conflict resolutions or manual amendments to merge
commits are not rebased automatically, but have to be applied manually."
is mutually exclusive with "Rebase merge commits", making all this even
more messy. A merge commit is just content with multiple parents, and
`git rebase`, by definition, reapplies the changes the content
introduces. Any "amendments" or "resolutions" that could have been
happening (or not) when that commit was being created are entirely
irrelevant.

Further yet it goes with:

"By default, or when `no-rebase-cousins` was specified, commits which do
not have `` as direct ancestor will keep their original branch
point."

Really? What does it actually mean? What is "commit branch point",
exactly? What "direct ancestor" means in this context, exactly? Provided
even when I do know what the option actually does, the description looks
wrong, how it could explain anything?

Having all this right here in the patch series, you guys try to convince
me that it should not be fixed? That it meets user expectations? You
know what? I, as a user, have somewhat higher expectations.

Below is my final attempt at actually defining a sane alternative. If
you still find this approach inferior, please feel free to ignore it. I
added "history" at the end of original --no-flatten, as a courtesy to
user expectations, as you seem to prefer more verbose names:

--

--flatten-history
Flatten rebased history by 

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-13 Thread Johannes Schindelin
Hi Phillip,

On Fri, 13 Apr 2018, Phillip Wood wrote:

> On 12/04/18 23:02, Johannes Schindelin wrote:
> > 
> > [...]
> > 
> > So: the order of the 3-way merges does matter.
> >
> > [...]
> 
> Those conflicts certainly look intimidating (and the ones in your later
> reply with the N way merge example still look quite complicated). One
> option would be just to stop and have the user resolve the conflicts
> after each conflicting 3-way merge rather than at the end of all the
> merges. There are some downsides: there would need to be a way to
> explain to the user that this is an intermediate step (and what that
> step was); the code would have to do some book keeping to know where it
> had got to; and it would stop and prompt the user to resolve conflicts
> more often which could be annoying but hopefully they'd be clearer to
> resolve because they weren't nested.

I thought about that. But as I pointed out: the order of the merges *does*
matter. Otherwise we force the user to resolve conflicts that they
*already* resolved during this rebase...

Ciao,
Dscho


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-13 Thread Phillip Wood

On 12/04/18 23:02, Johannes Schindelin wrote:

Hi Jake,

On Thu, 12 Apr 2018, Jacob Keller wrote:


On Wed, Apr 11, 2018 at 10:42 PM, Sergey Organov  wrote:


Jacob Keller  writes:

On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov  wrote:

It was rather --recreate-merges just a few weeks ago, and I've seen
nobody actually commented either in favor or against the
--rebase-merges.

git rebase --rebase-merges


I'm going to jump in here and say that *I* prefer --rebase-merges, as
it clearly mentions merge commits (which is the thing that changes).


OK, thanks, it's fair and the first argument in favor of
--rebase-merges I see.


I'd be ok with "--keep-merges" also.


My main argument against --keep-merges is that there is no good short
option for it: -k and -m are already taken. And I really like my `git
rebase -kir` now...

A minor argument in favor of `--rebase-merges` vs `--keep-merges` is that
we do not really keep the merge commits, we rewrite them. In the version
as per this here patch series, we really create recursive merges from
scratch.

In the later patch series on which I am working, we use a variation of
Phillip's strategy which can be construed as a generalization of the
cherry-pick to include merges: for a cherry-pick, we perform a 3-way merge
between the commit and HEAD, with the commit's parent commit as merge
base. With Phillip's strategy, we perform a 3-way merge between the merge
commit and HEAD (i.e. the rebased first parent), with the merge commit's
first parent as merge base, followed by a 3-way merge with the rebased
2nd parent (with the original 2nd parent as merge base), etc

However. This strategy, while it performed well in my initial tests (and
in Buga's initial tests, too), *does* involve more than one 3-way merge,
and therefore it risks something very, very nasty: *nested* merge
conflicts.

Now, I did see nested merge conflicts in the past, very rarely, but that
can happen, when two developers criss-cross merge each others' `master`
branch and are really happy to perform invasive changes that our merge
does not deal well with, such as indentation changes.

When rebasing a merge conflict, however, such nested conflicts can happen
relatively easily. Not rare at all.

I found out about this by doing what I keep preaching in this thred:
theory is often very nice *right* until the point where it hits reality,
and then frequently turns really useless, real quickly. Theoretical
musings can therefore be an utter waste of time, unless accompanied by
concrete examples.


Exactly (that's one reason I've been keeping a low profile on this 
thread since my initial suggestion - I haven't had time to test out any 
examples). Thanks for taking the time to test out the theory



To start, I built on the example for an "evil merge" that I gave already
in the very beginning of this insanely chatty thread: if one branch
changes the signature of a function, and a second branch adds a caller to
that function, then by necessity a merge between those two branches has to
change the caller to accommodate the signature change. Otherwise it would
end up in a broken state.

In my `sequencer-shears` branch at https://github.com/dscho/git, I added
this as a test case, where I start out with a main.c containing a single
function called core(). I then create one branch where this function is
renamed to hi(), and another branch where the function caller() is added
that calls core(). Then I merge both, amending the merge commit so that
caller() now calls hi(). So this is the main.c after merging:

int hi(void) {
printf("Hello, world!\n");
}
/* caller */
void caller(void) {
hi();
}

To create the kind of problems that are all too common in my daily work
(seemingly every time some stable patch in Git for Windows gets
upstreamed, it changes into an incompatible version, causing merge
conflicts, and sometimes not only that... but I digress...), I then added
an "upstream" where some maintainer decided that core() is better called
greeting(), and also that a placeholder function for an event loop should
be added. So in upstream, main.c looks like this:

int greeting(void) {
printf("Hello, world!\n");
}
/* main event loop */
void event_loop(void) {
/* TODO: place holder for now */
}

Keep in mind: while this is a minimal example of disagreeing changes that
may look unrealistic, in practice this is the exact type of problem I am
dealing with on a daily basis, in Git for Windows as well as in GVFS Git
(which adds a thicket of branches on top of Git for Windows) and with the
MSYS2 runtime (where Git for Windows stacks patches on top of MSYs2, which
in turn maintains their set of patches on top of the Cygwin runtime), and
with BusyBox, and probably other projects I forgot spontaneously. This
makes me convinced that 

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-13 Thread Johannes Schindelin
Hi Jake,

On Thu, 12 Apr 2018, Jacob Keller wrote:

> On Thu, Apr 12, 2018 at 3:02 PM, Johannes Schindelin
>  wrote:
> 
> > [... talking about nested merge conflicts ...]
> >
> > The only way out I can see is to implement some sort of "W merge" or
> > "chandelier merge" that can perform an N-way merge between one revision
> > and N-1 other revisions (each of the N-1 bringing its own merge base). I
> > call them "W" or "chandelier" because such a merge can be visualized by
> > the original merge commit being the center of a chandelier, and each arm
> > representing one of the N-1 merge heads with their own merge bases.
> >
> 
> I think this approach sounds reasonable.

... and it would incidentally also offer a saner way to do octopus merges
(so far, an octopus merge that causes merge conflicts causes... huge
pains, as it usually stops in the middle of everything, without a UI to
help with concluding the merge).

> > Similar to the 3-way merge we have implemented in xdiff/xmerge.c, this
> > "chandelier merge" would then generate the two diffs between merge base
> > and both merge heads, except not only one time, but N-1 times. It would
> > then iterate through all hunks ordered by file name and line range. Any
> > hunk without conflicting changes would be applied as-is, and the remaining
> > ones be turned into conflicts (handling those chandelier arms first where
> > both diffs' hunks look identical).
> >
> > Have I missed any simpler alternative?
> 
> I *think* this would work well if I understand it, but it's difficult
> to process without examples.

Well, I am fairly certain about the implementation details (it's been a
while since I contributed xdiff/xmerge.c, and if you ever want to hear the
horrible story how I wrote the initial version in a stopped train in the
middle of the night, just buy me a beer or three, my memory is fresh on
the "simultaneous walking" of the diff hunks).

So it goes somewhat like this. You have two diffs, and for the matter of
the discussion, let's just look at the hunk headers (with 0 context lines,
i.e. -U0):

diff base..HEAD
@@ -10,1 +10,2 @@
@@ -40,2 +41,0 @@

diff base..branch
@@ -8,4 +8,3 @@

So on one side of the merge, we changed line 10 (e.g. wrapping a long
line), and we removed lines 40 and 41.

In the branch we want to merge, lines 8--11 were edited (removing one
line).

The 3-way merge as implemented in xdiff/xmerge.c handles only one file,
and first uses the diff machinery to figure out the hunk headers of both
diffs, then iterates through both diffs. This is the `while (xscr1 &&
xscr2)` loop in `xdl_do_merge()`, and the "scr" stands for "script" as in
"edit script". In other words, `xscr1` refers to the current hunk in the
first diff, and `xscr2` to the one in the second diff.

Inside the loop, we look whether they overlap. If not, the one with the
smaller line numbers is "applied" and we iterate to the next hunk after
that.

If the hunks overlap, we have a look at the respective post images to see
whether both sides of the merge modified that part identically; if they
don't, we create a conflict (and later, we will try to reduce the conflict
by trimming identially-changed lines at both ends of the line range).

Lather, rinse & repeat.

Now, what I have in mind is that we will have not only two diffs' hunks to
look through, but (N-1)*2 (note that if N == 2, it is the exact same thing
as before).

Again, at each iteration, we look for the next hunk among all available
ones, then determine whether it overlaps with any other hunk. If it does
not, we apply it. If it does, we first look whether all overlapping hunks
agree on the post image and if they do: apply the change, otherwise create
a conflict.

How to present such conflicts to the user, though?

The worst case, I think, would be N diverging changes with N-1 agreeing on
a large part of the post image and the remaining post image being
completely different. Imagine, for example, that the original merge
contains a long function hi() that was renamed to greeting() in HEAD, but
replaced by a completely different implementation in the rebased
branch-to-merge. In such a case, this nested conflict would be most
intuitive, methinks:

<<< intermediate merge
 HEAD
greeting()

hi()
 original merge
... /* original function body */
===
hi()
... /* complete rewrite */
>>> branch

But now that I look at it, it is still hard to parse. *Is* there any good
way to present this conflict?

And then there is the problem that our index really is only prepared for
*three* stages, but we would need N*2-1.

So maybe I am overthinking this and we should stick with the
implementation I have right now (try to merge HEAD and the original merge
first, then merge the rebased 2nd parent if there are no conflicts,
otherwise try the other way round), and simply come up with a *very good*
message to the unfortunate 

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-12 Thread Jacob Keller
On Thu, Apr 12, 2018 at 3:02 PM, Johannes Schindelin
 wrote:
> Hi Jake,
>
> On Thu, 12 Apr 2018, Jacob Keller wrote:
>
>> On Wed, Apr 11, 2018 at 10:42 PM, Sergey Organov  wrote:
>> >
>> > Jacob Keller  writes:
>> >> On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov  
>> >> wrote:
>> >>> It was rather --recreate-merges just a few weeks ago, and I've seen
>> >>> nobody actually commented either in favor or against the
>> >>> --rebase-merges.
>> >>>
>> >>> git rebase --rebase-merges
>> >>
>> >> I'm going to jump in here and say that *I* prefer --rebase-merges, as
>> >> it clearly mentions merge commits (which is the thing that changes).
>> >
>> > OK, thanks, it's fair and the first argument in favor of
>> > --rebase-merges I see.
>>
>> I'd be ok with "--keep-merges" also.
>
> My main argument against --keep-merges is that there is no good short
> option for it: -k and -m are already taken. And I really like my `git
> rebase -kir` now...
>

Right, that's unfortunate.

> A minor argument in favor of `--rebase-merges` vs `--keep-merges` is that
> we do not really keep the merge commits, we rewrite them. In the version
> as per this here patch series, we really create recursive merges from
> scratch.

I also don't have a strong opinion in regards to --keep vs --rebase..

>
> In the later patch series on which I am working, we use a variation of
> Phillip's strategy which can be construed as a generalization of the
> cherry-pick to include merges: for a cherry-pick, we perform a 3-way merge
> between the commit and HEAD, with the commit's parent commit as merge
> base. With Phillip's strategy, we perform a 3-way merge between the merge
> commit and HEAD (i.e. the rebased first parent), with the merge commit's
> first parent as merge base, followed by a 3-way merge with the rebased
> 2nd parent (with the original 2nd parent as merge base), etc
>
> However. This strategy, while it performed well in my initial tests (and
> in Buga's initial tests, too), *does* involve more than one 3-way merge,
> and therefore it risks something very, very nasty: *nested* merge
> conflicts.

Yea, it could. Finding an elegant solution around this would be ideal!
(By elegant, I mean a solution which produces merge conflicts that
users can resolve relatively easily).

>
> Now, I did see nested merge conflicts in the past, very rarely, but that
> can happen, when two developers criss-cross merge each others' `master`
> branch and are really happy to perform invasive changes that our merge
> does not deal well with, such as indentation changes.
>
> When rebasing a merge conflict, however, such nested conflicts can happen
> relatively easily. Not rare at all.

Right. This would be true regardless of what strategy we choose, I think.

>
> I found out about this by doing what I keep preaching in this thred:
> theory is often very nice *right* until the point where it hits reality,
> and then frequently turns really useless, real quickly. Theoretical
> musings can therefore be an utter waste of time, unless accompanied by
> concrete examples.

Agreed.

>
> To start, I built on the example for an "evil merge" that I gave already
> in the very beginning of this insanely chatty thread: if one branch
> changes the signature of a function, and a second branch adds a caller to
> that function, then by necessity a merge between those two branches has to
> change the caller to accommodate the signature change. Otherwise it would
> end up in a broken state.
>
> In my `sequencer-shears` branch at https://github.com/dscho/git, I added
> this as a test case, where I start out with a main.c containing a single
> function called core(). I then create one branch where this function is
> renamed to hi(), and another branch where the function caller() is added
> that calls core(). Then I merge both, amending the merge commit so that
> caller() now calls hi(). So this is the main.c after merging:
>
> int hi(void) {
> printf("Hello, world!\n");
> }
> /* caller */
> void caller(void) {
> hi();
> }
>
> To create the kind of problems that are all too common in my daily work
> (seemingly every time some stable patch in Git for Windows gets
> upstreamed, it changes into an incompatible version, causing merge
> conflicts, and sometimes not only that... but I digress...), I then added
> an "upstream" where some maintainer decided that core() is better called
> greeting(), and also that a placeholder function for an event loop should
> be added. So in upstream, main.c looks like this:
>
> int greeting(void) {
> printf("Hello, world!\n");
> }
> /* main event loop */
> void event_loop(void) {
> /* TODO: place holder for now */
> }
>
> Keep in mind: while this is a minimal example of disagreeing changes that
> may look unrealistic, in 

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-12 Thread Johannes Schindelin
Hi Jake,

On Thu, 12 Apr 2018, Jacob Keller wrote:

> On Wed, Apr 11, 2018 at 10:42 PM, Sergey Organov  wrote:
> >
> > Jacob Keller  writes:
> >> On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov  wrote:
> >>> It was rather --recreate-merges just a few weeks ago, and I've seen
> >>> nobody actually commented either in favor or against the
> >>> --rebase-merges.
> >>>
> >>> git rebase --rebase-merges
> >>
> >> I'm going to jump in here and say that *I* prefer --rebase-merges, as
> >> it clearly mentions merge commits (which is the thing that changes).
> >
> > OK, thanks, it's fair and the first argument in favor of
> > --rebase-merges I see.
> 
> I'd be ok with "--keep-merges" also.

My main argument against --keep-merges is that there is no good short
option for it: -k and -m are already taken. And I really like my `git
rebase -kir` now...

A minor argument in favor of `--rebase-merges` vs `--keep-merges` is that
we do not really keep the merge commits, we rewrite them. In the version
as per this here patch series, we really create recursive merges from
scratch.

In the later patch series on which I am working, we use a variation of
Phillip's strategy which can be construed as a generalization of the
cherry-pick to include merges: for a cherry-pick, we perform a 3-way merge
between the commit and HEAD, with the commit's parent commit as merge
base. With Phillip's strategy, we perform a 3-way merge between the merge
commit and HEAD (i.e. the rebased first parent), with the merge commit's
first parent as merge base, followed by a 3-way merge with the rebased
2nd parent (with the original 2nd parent as merge base), etc

However. This strategy, while it performed well in my initial tests (and
in Buga's initial tests, too), *does* involve more than one 3-way merge,
and therefore it risks something very, very nasty: *nested* merge
conflicts.

Now, I did see nested merge conflicts in the past, very rarely, but that
can happen, when two developers criss-cross merge each others' `master`
branch and are really happy to perform invasive changes that our merge
does not deal well with, such as indentation changes.

When rebasing a merge conflict, however, such nested conflicts can happen
relatively easily. Not rare at all.

I found out about this by doing what I keep preaching in this thred:
theory is often very nice *right* until the point where it hits reality,
and then frequently turns really useless, real quickly. Theoretical
musings can therefore be an utter waste of time, unless accompanied by
concrete examples.

To start, I built on the example for an "evil merge" that I gave already
in the very beginning of this insanely chatty thread: if one branch
changes the signature of a function, and a second branch adds a caller to
that function, then by necessity a merge between those two branches has to
change the caller to accommodate the signature change. Otherwise it would
end up in a broken state.

In my `sequencer-shears` branch at https://github.com/dscho/git, I added
this as a test case, where I start out with a main.c containing a single
function called core(). I then create one branch where this function is
renamed to hi(), and another branch where the function caller() is added
that calls core(). Then I merge both, amending the merge commit so that
caller() now calls hi(). So this is the main.c after merging:

int hi(void) {
printf("Hello, world!\n");
}
/* caller */
void caller(void) {
hi();
}

To create the kind of problems that are all too common in my daily work
(seemingly every time some stable patch in Git for Windows gets
upstreamed, it changes into an incompatible version, causing merge
conflicts, and sometimes not only that... but I digress...), I then added
an "upstream" where some maintainer decided that core() is better called
greeting(), and also that a placeholder function for an event loop should
be added. So in upstream, main.c looks like this:

int greeting(void) {
printf("Hello, world!\n");
}
/* main event loop */
void event_loop(void) {
/* TODO: place holder for now */
}

Keep in mind: while this is a minimal example of disagreeing changes that
may look unrealistic, in practice this is the exact type of problem I am
dealing with on a daily basis, in Git for Windows as well as in GVFS Git
(which adds a thicket of branches on top of Git for Windows) and with the
MSYS2 runtime (where Git for Windows stacks patches on top of MSYs2, which
in turn maintains their set of patches on top of the Cygwin runtime), and
with BusyBox, and probably other projects I forgot spontaneously. This
makes me convinced that this is the exact type of problem that will
challenge whatever --rebase-merges has to deal with, or better put: what
the user of --rebase-merges will have to deal with.

(If I got a penny for 

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-12 Thread Jacob Keller
On Wed, Apr 11, 2018 at 10:42 PM, Sergey Organov  wrote:
> Hi Jacob,
>
> Jacob Keller  writes:
>> On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov  wrote:
>>> It was rather --recreate-merges just a few weeks ago, and I've seen
>>> nobody actually commented either in favor or against the
>>> --rebase-merges.
>>>
>>> git rebase --rebase-merges
>>>
>>
>> I'm going to jump in here and say that *I* prefer --rebase-merges, as
>> it clearly mentions merge commits (which is the thing that changes).
>
> OK, thanks, it's fair and the first argument in favor of --rebase-merges
> I see.
>

I'd be ok with "--keep-merges" also. I don't like the idea of
"flatten" as it, to me, means that anyone who wants to understand the
option without prior knowledge must immediately read the man page or
they will be confused. Something like "--rebase-merges" at least my
coworkers got it instantly. The same could be said for "--keep-merges"
too, but so far no one I asked said the immediately understood
"--no-flatten".

Thanks,
Jake


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-12 Thread Johannes Schindelin
Hi Sergey,

On Thu, 12 Apr 2018, Sergey Organov wrote:

> Johannes Schindelin  writes:
>
> > On Wed, 11 Apr 2018, Sergey Organov wrote:
> >
> >> The RFC v2 and Phillip's strategy are essentially the same, as has been
> >> already shown multiple times, both theoretically and by testing.
> >
> > No, they are not.
> 
> It's off-topic here.

Well, you directed the discussion there. So there.

> If you _really_ want to discuss it further [...]

I am always interested in a constructive discussion toward the goal of
making Git better, to improve its user experience, to give users more
powerful options, and to make things easier to use.

I'll let you know when I detect a change in this discussion in that vague
direction.

> Abrupt change of the topic of discussion indicates your intention to
> take attention off the apparent ugliness of 
> 
> git rebase --rebase-merges

If you want to discuss ugly things in Git, that is really an abrupt
diversion, but I would not fault you: there is plenty of that in Git.

As to `git rebase --rebase-merges`? I do not actually find that really
ugly. I find that it says what I want it to say. And after how many people
agreed, I find it rather pointless and time-wasting to discuss this
further. Naming is hard, and you seem to have a knack for coming up with
names that are really terrible. That is why I stopped discussing this with
you.

> I also get it as an indication that there are no more arguments in favor
> of --rebase-merges on your side, at least for now.

You seem to misinterpret your own arguments against --rebase-merges to be
anywhere in the realm of convincing. They are not.

Did I say "flatten history" to you in this discussion? Sure I did. We also
talked about Darcs. About the theory of patches. About the inner workings
of recursive merges. About commit graphs. And topologies. And we threw
around many terms that we know people understand who are deep into the
inner workings of merges and cherry-picks.

Does this mean that we should expose all the terms we used in this
technical discussion to the user interface?

No, it does not. We should not absolutely not do that.

So it is not at all a convincing argument to say "but you said XYZ". *In
this mail thread*. Which is necessarily full of technical lingo.

Also, I am still waiting for something tangible from your side. Something
non-theoretic. Something practical. Something like taking that FAKE_INIT
example at heart, studying it, deducing from it what weaknesses we cannot
tolerate in strategies to "cherry-pick merge commits" or "forward-port
merges" or "re-apply amendments in merge commits" or whatever you want to
call it.

Your suggestions so far are heavily biased by your own preferences, based
in theoretical musings, not in practical examples. I do not see any focus
on the Git user base at large. "What? They don't know what a topology is?"
is a question I could see you asking.

There has been a lot of talk in this mail thread, and the only actual
outcome I see is my own work, and Buga's tireless efforts to test
approaches for their practicality. There is zilch concrete testing from
your side. No implementation of anything. No demonstration what kinds of
merge conflicts are produced, how often they would have to be resolved by
the user. None.

The important thing to keep in mind is that all my efforts here are spent
in order to come up with a feature in Git that empowers users. And I want
this feature to be as usable as possible. And I want it to use as simple
language and option names as possible. That is what I will keep focusing
on, like it or not.

Ciao,
Johannes


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-12 Thread Sergey Organov
Hi Johannes,

Johannes Schindelin  writes:
> Hi Sergey,
>
> On Wed, 11 Apr 2018, Sergey Organov wrote:
>
>> The RFC v2 and Phillip's strategy are essentially the same, as has been
>> already shown multiple times, both theoretically and by testing.
>
> No, they are not.

It's off-topic here. If you _really_ want to discuss it further, you are
still welcome to come back to where you ran away from and continue:

https://public-inbox.org/git/87po3oddl1@javad.com/

Abrupt change of the topic of discussion indicates your intention to
take attention off the apparent ugliness of 

git rebase --rebase-merges

I also get it as an indication that there are no more arguments in favor
of --rebase-merges on your side, at least for now.

-- Sergey


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-11 Thread Sergey Organov
Hi Jacob,

Jacob Keller  writes:
> On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov  wrote:
>> It was rather --recreate-merges just a few weeks ago, and I've seen
>> nobody actually commented either in favor or against the
>> --rebase-merges.
>>
>> git rebase --rebase-merges
>>
>
> I'm going to jump in here and say that *I* prefer --rebase-merges, as
> it clearly mentions merge commits (which is the thing that changes).

OK, thanks, it's fair and the first argument in favor of --rebase-merges
I see.

I don't get why this detail matters so much it should be reflected in
the option name, and if it is what matters most, why the patch series
are not headed:


rebase -i: offer to rebase merge commits.

Once upon a time, I dreamt of an interactive rebase that would not
drop merge commits, but instead rebase them.


> I hadn't mentioned this before, because it was a suggestion that
> someone else made and it seemed that Johannes liked it, so I didn't
> think further discussion was worthwhile.

So you guys seem to be winning 2:1, or even 3:1, counting the guy who
made the suggestion. Except it was Buga's suggestion [1], and I believe
I was able to convince him that something like --no-flatten would be
better [2]:


> I hope he'd be pleased to be able to say --no-flatten=remerge and get
> back his current mode of operation, that he obviously has a good use
> for.

Makes sense, I like it, thanks for elaborating. [ Especially that you 
used "(no) flatten" phrasing, where original `--preserve-merges` 
documentation says it`s used "not to flatten the history", nice touch
;) ]


So I assume it's 2:2 by now, with the author of original suggestion on
my side.

I still find

git rebase --rebase-merges

both being ugly and missing the point.

When I look at it with a fresh eye, the questions that immediately rise
are: "What the hell else could 'git _rebase_' do with (merge) commits
but _rebase_ them? Why do I even need to specify this option? Should I
also specify --rebase-non-merges to rebase the rest of commits?"

Well, if it was called something like --[no-]keep-merges, it'd make more
sense as it'd be obvious that alternative is to drop merges (notice how
the old --preserve-merges does match this criteria). However, it'd still
miss to reflect the generic intent of the patch series, -- to preserve
history shape as much as possible, -- now citing author's head message
non-twisted: 


rebase -i: offer to recreate commit topology

Once upon a time, I dreamt of an interactive rebase that would not
flatten branch structure, but instead recreate the commit topology
faithfully.


-- Sergey

[1] https://public-inbox.org/git/bc9f82fb-fd18-ee45-36a4-921a1381b...@gmail.com/
[2] https://public-inbox.org/git/a3d40dca-f508-5853-89bc-1f9ab3934...@gmail.com/


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-11 Thread Jacob Keller
On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov  wrote:
> It was rather --recreate-merges just a few weeks ago, and I've seen
> nobody actually commented either in favor or against the
> --rebase-merges.
>
> git rebase --rebase-merges
>

I'm going to jump in here and say that *I* prefer --rebase-merges, as
it clearly mentions merge commits (which is the thing that changes).

I hadn't mentioned this before, because it was a suggestion that
someone else made and it seemed that Johannes liked it, so I didn't
think further discussion was worthwhile.

Thanks,
Jake


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-11 Thread Johannes Schindelin
Hi Sergey,

On Wed, 11 Apr 2018, Sergey Organov wrote:

> The RFC v2 and Phillip's strategy are essentially the same, as has been
> already shown multiple times, both theoretically and by testing.

No, they are not.

I am really tired of repeating myself here, as I have demonstrated it at
length, at least half a dozen times, that they are *not* in practice the
same.

If you had played through the example as I suggested, you would actually
see where the differences are, and that your proposal is simply
impractical.

And you would see that Phillip's strategy is better, but I get the
impression that you want to avoid that insight at all cost.

Ciao,
Johannes




Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-11 Thread Sergey Organov
Johannes Schindelin  writes:

> Hi Sergey,
>
> On Wed, 11 Apr 2018, Sergey Organov wrote:
>
>> Johannes Schindelin  writes:
>>
>> > On Tue, 10 Apr 2018, Sergey Organov wrote:
>> >
>> >> Johannes Schindelin  writes:
>> >>
>> >> > Once upon a time, I dreamt of an interactive rebase that would not
>> >> > flatten branch structure, but instead recreate the commit topology
>> >> > faithfully.
>> >>
>> >> [...]
>> >>
>> >> > Think of --rebase-merges as "--preserve-merges done right".
>> >>
>> >> Both option names seem to miss the primary point of the mode of
>> >> operation that you've formulated in the first sentence. I suggest to
>> >> rather call the new option in accordance to your description, say,
>> >> --no-flatten, --keep-topology, or --preserve-shape.
>> >
>> > A very quick A/B test shows that neither --no-flatten nor --keep-topology
>> > and certainly not --preserve-shape conveys to Git users what those options
>> > are supposed to do.
>>
>> In fact, my preference would be --[no-]flatten, exactly because the
>> default mode of rebase operation flattens the history, and thus what I'm
>> talking about is:
>>
>> git rebase --no-flatten
>
> And this is the option out of the four that fared *worst* in the A/B
> testing. Not even experts in Git internals were able to figure out what
> the heck you are talking about.

It was you who introduced the "flatten" term, not me. I took it from
your descriptions.

So they are able to make sense of your own:

>>> Once upon a time, I dreamt of an interactive rebase that would not
>>> flatten branch structure, but instead recreate the commit topology
>>> faithfully.

Yet they can't get:

--no-flatten::
Instead of flattening branch structure, recreate the commit
topology faithfully

Are you kidding?

Well, suppose for a moment that nobody could even guess what "flatten"
means indeed. Then are you willing to remove the "flatten" from both the
description of our patch series and from the proposed patch to the Git
manual:

-r::
--rebase-merges[=(rebase-cousins|no-rebase-cousins)]::
Rebase merge commits instead of _flattening_ the history by replaying
merges.

???

>
> Now, you can beat that dead horse until it is pulp. Your choice. I'd
> rather go on to more interesting things, because as far as I am concerned,
> the naming issue has been settled, with you being the only person in
> disfavor of --rebase-merges.

It was rather --recreate-merges just a few weeks ago, and I've seen
nobody actually commented either in favor or against the
--rebase-merges.

git rebase --rebase-merges

_is_ plain simple ugly.

>
> What you *could* do is finally take your RFC to the test. Run it with the
> concrete example I showed you in
> https://public-inbox.org/git/nycvar.qro.7.76.6.1803261405170...@zvavag-6oxh6da.rhebcr.pbec.zvpebfbsg.pbz/
>
> It is high time that you demonstrated on this concrete case study how your
> proposed solution performs. And then tally that up with Phillip's
> strategy.

What you could do is to stop shifting the subject of discussion.

The RFC v2 and Phillip's strategy are essentially the same, as has been
already shown multiple times, both theoretically and by testing. Ask
Bugga for details.

One way or another, this doesn't make

git rebase --rebase-merges

even a bit less ugly.

-- Sergey


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-11 Thread Johannes Schindelin
Hi Sergey,

On Wed, 11 Apr 2018, Sergey Organov wrote:

> Johannes Schindelin  writes:
> 
> > On Tue, 10 Apr 2018, Sergey Organov wrote:
> >
> >> Johannes Schindelin  writes:
> >> 
> >> > Once upon a time, I dreamt of an interactive rebase that would not
> >> > flatten branch structure, but instead recreate the commit topology
> >> > faithfully.
> >> 
> >> [...]
> >> 
> >> > Think of --rebase-merges as "--preserve-merges done right".
> >> 
> >> Both option names seem to miss the primary point of the mode of
> >> operation that you've formulated in the first sentence. I suggest to
> >> rather call the new option in accordance to your description, say,
> >> --no-flatten, --keep-topology, or --preserve-shape.
> >
> > A very quick A/B test shows that neither --no-flatten nor --keep-topology
> > and certainly not --preserve-shape conveys to Git users what those options
> > are supposed to do.
> 
> In fact, my preference would be --[no-]flatten, exactly because the
> default mode of rebase operation flattens the history, and thus what I'm
> talking about is:
> 
> git rebase --no-flatten

And this is the option out of the four that fared *worst* in the A/B
testing. Not even experts in Git internals were able to figure out what
the heck you are talking about.

Now, you can beat that dead horse until it is pulp. Your choice. I'd
rather go on to more interesting things, because as far as I am concerned,
the naming issue has been settled, with you being the only person in
disfavor of --rebase-merges.

What you *could* do is finally take your RFC to the test. Run it with the
concrete example I showed you in
https://public-inbox.org/git/nycvar.qro.7.76.6.1803261405170...@zvavag-6oxh6da.rhebcr.pbec.zvpebfbsg.pbz/

It is high time that you demonstrated on this concrete case study how your
proposed solution performs. And then tally that up with Phillip's
strategy.

Ciao,
Johannes


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-10 Thread Sergey Organov
Hi Johannes,

Johannes Schindelin  writes:

> Hi Sergey,
>
> On Tue, 10 Apr 2018, Sergey Organov wrote:
>
>> Johannes Schindelin  writes:
>> 
>> > Once upon a time, I dreamt of an interactive rebase that would not
>> > flatten branch structure, but instead recreate the commit topology
>> > faithfully.
>> 
>> [...]
>> 
>> > Think of --rebase-merges as "--preserve-merges done right".
>> 
>> Both option names seem to miss the primary point of the mode of
>> operation that you've formulated in the first sentence. I suggest to
>> rather call the new option in accordance to your description, say,
>> --no-flatten, --keep-topology, or --preserve-shape.
>
> A very quick A/B test shows that neither --no-flatten nor --keep-topology
> and certainly not --preserve-shape conveys to Git users what those options
> are supposed to do.

In fact, my preference would be --[no-]flatten, exactly because the
default mode of rebase operation flattens the history, and thus what I'm
talking about is:

git rebase --no-flatten

vs 

git rebase --rebase-merges

I honestly fail to see how the latter conveys the purpose of the option
better than the former, sorry. To tell the truth, the latter also looks
plain ugly to me.

> But --rebase-merges did convey the purpose of my patch series. So
> there.

- Except that your primary description of the series (that I find pretty
solid) doesn't mention _merges_ at all and still conveys the purpose?

- Except that this patch series _don't_ actually _rebase_ merges?
Yeah, I remember a follow-up is to be expected, but anyway.

I'm still unconvinced.

-- Sergey


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-10 Thread Johannes Schindelin
Hi Sergey,

On Tue, 10 Apr 2018, Sergey Organov wrote:

> Johannes Schindelin  writes:
> 
> > Once upon a time, I dreamt of an interactive rebase that would not
> > flatten branch structure, but instead recreate the commit topology
> > faithfully.
> 
> [...]
> 
> > Think of --rebase-merges as "--preserve-merges done right".
> 
> Both option names seem to miss the primary point of the mode of
> operation that you've formulated in the first sentence. I suggest to
> rather call the new option in accordance to your description, say,
> --no-flatten, --keep-topology, or --preserve-shape.

A very quick A/B test shows that neither --no-flatten nor --keep-topology
and certainly not --preserve-shape conveys to Git users what those options
are supposed to do.

But --rebase-merges did convey the purpose of my patch series. So there.

Ciao,
Johannes


Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-10 Thread Sergey Organov
Hi Johannes,

Johannes Schindelin  writes:

> Once upon a time, I dreamt of an interactive rebase that would not
> flatten branch structure, but instead recreate the commit topology
> faithfully.

[...]

> Think of --rebase-merges as "--preserve-merges done right".

Both option names seem to miss the primary point of the mode of
operation that you've formulated in the first sentence. I suggest to
rather call the new option in accordance to your description, say,
--no-flatten, --keep-topology, or --preserve-shape.

Besides, this way the option name will only specify one thing: _what_ it
is about, leaving out the _how_ part, that could vary and could then be
specified as option value or as another companion option(s), that is
usually considered to be an indication of a good design.

-- Sergey


[PATCH v6 00/15] rebase -i: offer to recreate commit topology

2018-04-10 Thread Johannes Schindelin
Once upon a time, I dreamt of an interactive rebase that would not
flatten branch structure, but instead recreate the commit topology
faithfully.

My original attempt was --preserve-merges, but that design was so
limited that I did not even enable it in interactive mode.

Subsequently, it *was* enabled in interactive mode, with the predictable
consequences: as the --preserve-merges design does not allow for
specifying the parents of merge commits explicitly, all the new commits'
parents are defined *implicitly* by the previous commit history, and
hence it is *not possible to even reorder commits*.

This design flaw cannot be fixed. Not without a complete re-design, at
least. This patch series offers such a re-design.

Think of --rebase-merges as "--preserve-merges done right". It
introduces new verbs for the todo list, `label`, `reset` and `merge`.
For a commit topology like this:

A - B - C
  \   /
D

the generated todo list would look like this:

# branch D
pick 0123 A
label branch-point
pick 1234 D
label D

reset branch-point
pick 2345 B
merge -C 3456 D # C

There are more patches in the pipeline, based on this patch series, but
left for later in the interest of reviewable patch series: one mini
series to use the sequencer even for `git rebase -i --root`, and another
one to add support for octopus merges to --rebase-merges. And then one
to allow for rebasing merge commits in a smarter way (this one will need
a bit more work, though, as it can result in very complicated, nested
merge conflicts *very* easily).

Changes since v5 (sorry, this one is big, and so is the interdiff):

- rebased to `master`, resolving conflicts with `ws/rebase-p` and
  `pw/rebase-keep-empty-fixes` (these changes are not reflected in the
  interdiff because I still did not find a good way to represent such
  fixups).

- just like `git merge` refuses to merge ancestors of HEAD, so does now
  the todo command `merge`.

- `git remote -v`'s output now differs when pulling with --rebase-merges
  vs pulling with --interactive.

- the `merge` command now also gives rerere a chance (just like `pick`
  already does).

- simplified test for rebase-cousins (no need to run --rebase-merges
  interactively, so there is no need to override the editor either).

- fixed `safe_append()` to roll back the lock file even when *reading*
  failed.

- used `reflog_message()` in `do_reset()` rather than duplicating the
  logic.

- reworded misleading commit message talking about fast-forwarding merge
  commits, when we just fast-forward `merge` commands *to* those merge commits
  whenever possible.

- removed duplicate `if (can_fast_forward)` clause.

- stopped promising support for octopus merges in --make-script in this patch
  series (it will be added in a later patch series).

- fixed grammar error in the message of the commit adding support for
  post-rewrite hooks to handle commits processed via the `merge` command.

- folded TODO_MERGE_AND_EDIT into TODO_MERGE by using a new `flags`
  field.

- the code of `do_merge()` has been made more obvious by using a variable
  `oneline_offset` instead of the non-descriptive `p`.

- renamed the option to `rebase-merges`, in preparation for doing it
  smarter using Phillip Wood's strategy (this will be contributed in a
  follow-up patch series after two others that add support for octopus
  merges and for handling --root via the sequencer).

- included Phillip Wood's test for --keep-empty with the new mode (and folded
  in a fix into the code of the `merge` command).

- added -r as shortcut for --rebase-merges

- added an entire section about "REBASING MERGES" to git-rebase's man page.


Johannes Schindelin (13):
  sequencer: avoid using errno clobbered by rollback_lock_file()
  sequencer: make rearrange_squash() a bit more obvious
  sequencer: introduce new commands to reset the revision
  sequencer: introduce the `merge` command
  sequencer: fast-forward `merge` commands, if possible
  rebase-helper --make-script: introduce a flag to rebase merges
  rebase: introduce the --rebase-merges option
  sequencer: make refs generated by the `label` command worktree-local
  sequencer: handle post-rewrite for merge commands
  rebase --rebase-merges: avoid "empty merges"
  pull: accept --rebase=merges to recreate the branch topology
  rebase -i: introduce --rebase-merges=[no-]rebase-cousins
  rebase -i --rebase-merges: add a section to the man page

Phillip Wood (1):
  rebase --rebase-merges: add test for --keep-empty

Stefan Beller (1):
  git-rebase--interactive: clarify arguments

 Documentation/config.txt   |   8 +
 Documentation/git-pull.txt |   5 +-
 Documentation/git-rebase.txt   | 140 -
 builtin/pull.c |  14 +-
 builtin/rebase--helper.c   |  13 +-
 builtin/remote.c   |  18 +-