[PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sebastian Schuberth]

Ignoring a merge sounds like ignoring the changes a merge commit
introduces altogether, as if the merge commit was skipped or dropped from
history. But that is not what happens if this options is not specified.
Instead, what happens is that the separate commits a merge commit
introduces are replayed in order, and only any possible merge conflict
resolutions are ignored. Get this straight in the docs.

Also, do not say that merge commits are *tried* to be recreated. They are
recreated, but possibly with conflicts, which in turn are likely to be
different from any original conflicts. Still recreating a merge commit does
not just silently fail in any case, which is how "try to recreate" might
sound.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 Documentation/git-rebase.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index d728030..3a6d139 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -362,7 +362,7 @@ default is `--no-fork-point`, otherwise the
default is `--fork-point`.

 -p::
 --preserve-merges::
-    Instead of ignoring merges, try to recreate them.
+    Recreate merge commits instead of replaying commits a merge
commit introduces.
 +
 This uses the `--interactive` machinery internally, but combining it
 with the `--interactive` option explicitly is generally not a good
-- 
1.9.5.msysgit.1

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Junio C Hamano]

Sebastian Schuberth <sschuberth@gmail.com> writes:

> Also, do not say that merge commits are *tried* to be recreated.

Good point.  "We will try but it might fail" is better left unsaid
as that is true almost everywhere.


> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
>  Documentation/git-rebase.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index d728030..3a6d139 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -362,7 +362,7 @@ default is `--no-fork-point`, otherwise the
> default is `--fork-point`.
>
>  -p::
>  --preserve-merges::
> -    Instead of ignoring merges, try to recreate them.
> +    Recreate merge commits instead of replaying commits a merge
> commit introduces.

Hmm, is this line-wrapped?

Although I fully agree that the new text is better than the original,
I think the new text fails to point out one major aspect by not
mentioning "linear" or "flatten" anywhere.  The point of "git rebase"
without "-p" is not just to replay but to flatten

	Instead of flattening the history by replaying each
	non-merge commit to be rebased, preserve the shape of the
	rebased history by recreating merge commits as well.

or something along that line, perhaps?

This reminds me a related tangent discussion we had long time ago (I
thought j6t was involved hence a new Cc:, but my apologies to j6t if
I am misremembering), about what exactly is "recreate merges instead
of replaying commits".

I think the current preserve-merges considers everything between
<upstream> and <branch> as "commits to be rebased", and recreate
merges across these rebased tips of branches that are merged.  There
however were repeated wishes (or wishful misunderstandings ;-) that
there were a mode to rebuild the trunk, considering only the commits
on the first-parent chain as "commits to be rebased", recreating the
history by replaying the merge commits (whose first parent might be
rewritten during the rebase, but the tips of side branches these
merges bring into the history are kept intact).

Surely there is no such mode right now, but I am fairly sure that I
wouldn't have any objection against a patch to implement such a
feature (perhaps "--first-parent --preserve-merges"?), and with or
without such a feature in the system, I would be happier if we made
sure that the description we are discussing to update makes it clear
that the current behaviour is "everything between <upstream> and
<branch>", and cannot be misread as "do not touch side branches
instead of dropping merged commits".

>  +
>  This uses the `--interactive` machinery internally, but combining it
>  with the `--interactive` option explicitly is generally not a good

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sebastian Schuberth]

On 26.03.2015 19:18, Junio C Hamano wrote:

>> Also, do not say that merge commits are *tried* to be recreated.
>
> Good point.  "We will try but it might fail" is better left unsaid
> as that is true almost everywhere.

Exactly.

>>   -p::
>>   --preserve-merges::
>> -    Instead of ignoring merges, try to recreate them.
>> +    Recreate merge commits instead of replaying commits a merge
>> commit introduces.
>
> Hmm, is this line-wrapped?

Probably, I had to send this via GMail.

> Although I fully agree that the new text is better than the original,
> I think the new text fails to point out one major aspect by not
> mentioning "linear" or "flatten" anywhere.  The point of "git rebase"
> without "-p" is not just to replay but to flatten
>
> 	Instead of flattening the history by replaying each
> 	non-merge commit to be rebased, preserve the shape of the
> 	rebased history by recreating merge commits as well.
>
> or something along that line, perhaps?

Hm, I'm not sure about the "as well" here. Non-merge commits basically 
are just picked, not recreated in the same sense as merge commits. I'll 
come up with another proposal.

> I think the current preserve-merges considers everything between
> <upstream> and <branch> as "commits to be rebased", and recreate
> merges across these rebased tips of branches that are merged.  There
> however were repeated wishes (or wishful misunderstandings ;-) that
> there were a mode to rebuild the trunk, considering only the commits
> on the first-parent chain as "commits to be rebased", recreating the
> history by replaying the merge commits (whose first parent might be
> rewritten during the rebase, but the tips of side branches these
> merges bring into the history are kept intact).

I guess I'm a victim of that wishful misunderstanding then, as I indeed 
though that's exactly what the current -p is doing. Well, modulo the 
special case where the second parent is the tip of a branch whose 
fork-point with the trunk is part of the rebase, see "Example 1" at [1].

In other word, you're saying that the current preserve-merges does not 
keep the tips of side branches intact. If that's so, what is is doing 
with the tips of the side branches?

> without such a feature in the system, I would be happier if we made
> sure that the description we are discussing to update makes it clear
> that the current behaviour is "everything between <upstream> and
> <branch>", and cannot be misread as "do not touch side branches
> instead of dropping merged commits".

Agreed. As soon as I understand the difference between the two :-)

[1] http://stackoverflow.com/a/15915431/1127485

-- 
Sebastian Schuberth

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Junio C Hamano]

Sebastian Schuberth <sschuberth@gmail.com> writes:

>> 	Instead of flattening the history by replaying each
>> 	non-merge commit to be rebased, preserve the shape of the
>> 	rebased history by recreating merge commits as well.
>>
>> or something along that line, perhaps?
>
> Hm, I'm not sure about the "as well" here. Non-merge commits basically
> are just picked, not recreated in the same sense as merge
> commits. I'll come up with another proposal.

OK.  I do not see qualitative difference between picking a non-merge
and picking a merge; they are both being replayed and it is not like
the machiery is trying to preserve an evil merge.  Having said that,
I do not have a strong feeling either way between keeping and
dropping that "as well".  I threw it in there only to contrast the
preserve mode (where merges are also picked) with the normal mode
(where merges are not picked).

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

[...]

> I think the current preserve-merges considers everything between
> <upstream> and <branch> as "commits to be rebased", and recreate
> merges across these rebased tips of branches that are merged.
>
> There however were repeated wishes (or wishful misunderstandings ;-)
> that there were a mode to rebuild the trunk, considering only the
> commits on the first-parent chain as "commits to be rebased",
> recreating the history by replaying the merge commits (whose first
> parent might be rewritten during the rebase, but the tips of side
> branches these merges bring into the history are kept intact).
>
> Surely there is no such mode right now,

Isn't it what Johannes Sixt <j6t@kdbg.org> mentions here?:

[QUOTE]
If you want a version of --preserve-merges that does what *you* need,
consider this commit:

  git://repo.or.cz/git/mingw/j6t.git rebase-p-first-parent

Use it like this:

  git rebase -i -p --first-parent ...

Beware, its implementation is incomplete: if the rebase is interrupted,
then 'git rebase --continue' behaves as if --first-parent were not given.
[/QUOTE].

ref: http://permalink.gmane.org/gmane.comp.version-control.git/263584

If so, then I'd say such mode almost exists.

> but I am fairly sure that I wouldn't have any objection against a
> patch to implement such a feature (perhaps "--first-parent
> --preserve-merges"?),

It'd be very welcome feature here, preferably along with a way to pass
it to 'git pull --rebase', including a way to configure it to be the
default. 

> and with or without such a feature in the system, I would be happier
> if we made sure that the description we are discussing to update makes
> it clear that the current behaviour is "everything between <upstream>
> and <branch>", and cannot be misread as "do not touch side branches
> instead of dropping merged commits".

I'd also suggest to somehow warn in the manual that current modes of
operation silently drop changes to merge commits that were made to
non-conflicting paths (either during conflict resolution or otherwise.)

-- Sergey.

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Johannes Sixt]

Am 26.03.2015 um 22:17 schrieb Sergey Organov:
> Junio C Hamano <gitster@pobox.com> writes:
>> There however were repeated wishes (or wishful misunderstandings ;-)
>> that there were a mode to rebuild the trunk, considering only the
>> commits on the first-parent chain as "commits to be rebased",
>> recreating the history by replaying the merge commits (whose first
>> parent might be rewritten during the rebase, but the tips of side
>> branches these merges bring into the history are kept intact).
>>
>> Surely there is no such mode right now,
>
> Isn't it what Johannes Sixt <j6t@kdbg.org> mentions here?:
>
> [QUOTE]
> If you want a version of --preserve-merges that does what *you* need,
> consider this commit:
>
>    git://repo.or.cz/git/mingw/j6t.git rebase-p-first-parent
>
> Use it like this:
>
>    git rebase -i -p --first-parent ...
>
> Beware, its implementation is incomplete: if the rebase is interrupted,
> then 'git rebase --continue' behaves as if --first-parent were not given.
> [/QUOTE].
>
> ref: http://permalink.gmane.org/gmane.comp.version-control.git/263584
>
> If so, then I'd say such mode almost exists.

The patch was discussed here:
http://thread.gmane.org/gmane.comp.version-control.git/198125

The reason that this has not progressed any further is this response in 
the thread:

http://thread.gmane.org/gmane.comp.version-control.git/198125/focus=198483

where you basically say that a --first-parent mode is good, but it 
should be separate from --preserve-merges. I haven't made up my mind, 
yet, how to proceed from there.

If somebody wants to pick up the baton, I'll happily pass it on.

-- Hannes

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sebastian Schuberth]

On 3/26/2015 19:18, Junio C Hamano wrote:

> Although I fully agree that the new text is better than the original,
> I think the new text fails to point out one major aspect by not
> mentioning "linear" or "flatten" anywhere.  The point of "git rebase"
> without "-p" is not just to replay but to flatten

I think it's a bit odd to say that "-p" does not flatten if the whole current rebase docs do not use the terms flattening / linearize at all. If we say that that "-p" does not flatten, we should say that a "standard" rebase wihtout "-p" does. I plan to send a patch for that later.


> 	Instead of flattening the history by replaying each
> 	non-merge commit to be rebased, preserve the shape of the
> 	rebased history by recreating merge commits as well.

Personally, I like "positive logic" better here, i.e. start with saying what the option does, not what it does not do.

So how about:

[PATCH] docs: Clarify what git-rebase's "-p" / "--preserve-merges" does

Ignoring a merge sounds like ignoring the changes a merge commit
introduces altogether, as if the merge commit was skipped or dropped from
history. But that is not what happens if "-p" is not specified.

Instead, what happens is that the individual commits a merge commit
introduces are replayed in order, and only any possible merge conflict
resolutions or manual amendments to the merge commit are ignored. Get this
straight in the docs.

Also, do not say that merge commits are *tried* to be recreated. As that is
true almost everywhere it is better left unsaid.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 Documentation/git-rebase.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index d728030..47984e8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -362,7 +362,9 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`.
 
 -p::
 --preserve-merges::
-	Instead of ignoring merges, try to recreate them.
+	Recreate merge commits instead of flattening the history by replaying
+	commits a merge commit introduces. Merge conflict resolutions or manual
+	amendments to merge commits are not preserved.
 +
 This uses the `--interactive` machinery internally, but combining it
 with the `--interactive` option explicitly is generally not a good
-- 
1.9.5.msysgit.1

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Junio C Hamano]

Sebastian Schuberth <sschuberth@gmail.com> writes:

> So how about:
>
> [PATCH] docs: Clarify what git-rebase's "-p" / "--preserve-merges" does
>
> Ignoring a merge sounds like ignoring the changes a merge commit
> introduces altogether, as if the merge commit was skipped or dropped from
> history. But that is not what happens if "-p" is not specified.

Every time I read the above (which is essentially the same from your
first edition I think) I find the "ignoring the changes a merge
commit introduces altogether" and "as if the merge commit was
skipped" somehow contradicting with each other.  If the latter were
"as if the side branch that was merged was skipped or dropped", I do
not see the room for such a misinterpretation.

That is, at least to me,

         D---E---F
        /         \
    ---A---B---C---G---H

the former, i.e. "the changes the merge G introdues", is "diff C G",
while "merge commit was skipped" would mean a history like this:

    ---A---B---C---D'--E'--F'--H'

And I think with "as if the side branch that was merged was
dropped", this misinterpretation will become impossible.  It can
only mean this history:

    ---A---B---C---.---H'

> Instead, what happens is that the individual commits a merge commit
> introduces are replayed in order, and only any possible merge conflict
> resolutions or manual amendments to the merge commit are ignored. Get this
> straight in the docs.
>
> Also, do not say that merge commits are *tried* to be recreated. As that is
> true almost everywhere it is better left unsaid.
>
> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
>  Documentation/git-rebase.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index d728030..47984e8 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -362,7 +362,9 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`.
>  
>  -p::
>  --preserve-merges::
> -	Instead of ignoring merges, try to recreate them.
> +	Recreate merge commits instead of flattening the history by replaying
> +	commits a merge commit introduces. Merge conflict resolutions or manual
> +	amendments to merge commits are not preserved.

The patch text itself reads fine.

>  +
>  This uses the `--interactive` machinery internally, but combining it
>  with the `--interactive` option explicitly is generally not a good

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sebastian Schuberth]

On Mon, Mar 30, 2015 at 7:23 PM, Junio C Hamano <gitster@pobox.com> wrote:

>> Ignoring a merge sounds like ignoring the changes a merge commit
>> introduces altogether, as if the merge commit was skipped or dropped from
>> history. But that is not what happens if "-p" is not specified.
>
> Every time I read the above (which is essentially the same from your
> first edition I think) I find the "ignoring the changes a merge
> commit introduces altogether" and "as if the merge commit was
> skipped" somehow contradicting with each other.  If the latter were
> "as if the side branch that was merged was skipped or dropped", I do
> not see the room for such a misinterpretation.
>
> That is, at least to me,
>
>          D---E---F
>         /         \
>     ---A---B---C---G---H
>
> the former, i.e. "the changes the merge G introdues", is "diff C G",

To me, too. In other words, this is the combined diff of all commits
reachable from all parents of the merge (other than the first parent).
In your example, that would be the combined diff of D, E and F, which
equals "git diff C G".

However, I'm used to thinking that a non-conflicting merge itself has
no diff, but introduces commits that have diffs. This way of seeing it
probably comes from my intensive use of gitk which does not display a
diff when selecting a merge commit unless it has conflicts resolved.

> while "merge commit was skipped" would mean a history like this:
>
>     ---A---B---C---D'--E'--F'--H'
>

This is where your interpretation differs. If a merge commit was
skipped, it is non-existent. If it is non-existent, it also has no
parents. If it has no parents, there are no commits that D, E, F could
be replayed on to of C. Thus for me, skipping merge commit G in your
example would result in:

    ---A---B---C---H'

> And I think with "as if the side branch that was merged was
> dropped", this misinterpretation will become impossible.  It can
> only mean this history:
>
>     ---A---B---C---.---H'

I think we need to generalize the wording a bit for merges with more
than two parents. How about making the the first paragraph of the
commit message simply read:

Ignoring a merge sounds like dropping the merge commit and all side
branches it merges from history. But that is not what happens if "-p" is
not specified.

-- 
Sebastian Schuberth

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Junio C Hamano]

Sebastian Schuberth <sschuberth@gmail.com> writes:

>> That is, at least to me,
>>
>>          D---E---F
>>         /         \
>>     ---A---B---C---G---H
>>
>> the former, i.e. "the changes the merge G introdues", is "diff C G",
>
> To me, too. In other words, this is the combined diff of all commits
> reachable from all parents of the merge (other than the first parent).
> In your example, that would be the combined diff of D, E and F, which
> equals "git diff C G".
>
>> while "merge commit was skipped" would mean a history like this:
>>
>>     ---A---B---C---D'--E'--F'--H'
>>
>
> This is where your interpretation differs.

I know.  If I am rebasing this whole graph on top of somewhere while
ignoring merge G:

>>          D---E---F
>>         /         \
>>     ---A---B---C---G---H

I'll expect changes made by A, B, C, D, E, F and H appear as
different commits in the result.  That is what "ignore G" means to
me.

But it is pointless for you to say "Your attempt to misunderstand
the current and proposed text is flawed".  People are flawed and I
am merely pointing out that what you wrote can be read by people
other than you in a way you did not mean to.

And my response was an attempt to offer a suggestion to make it
harder to be misread by anybody.

> Ignoring a merge sounds like dropping the merge commit and all side
> branches it merges from history.

... Yes to _some_ people (including you, but not me).  And that is
why we are trying to improve the text in the documentation, no?

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> Ignoring a merge sounds like dropping the merge commit and all side
>> branches it merges from history.
>
> ... Yes to _some_ people (including you, but not me).  And that is
> why we are trying to improve the text in the documentation, no?

Let's try this again as I do not think there is no need to argue in
the end.

We agree that this is a good patch:

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index d728030..47984e8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -362,7 +362,9 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`.
 
 -p::
 --preserve-merges::
-	Instead of ignoring merges, try to recreate them.
+	Recreate merge commits instead of flattening the history by replaying
+	commits a merge commit introduces. Merge conflict resolutions or manual
+	amendments to merge commits are not preserved.
 +
 This uses the `--interactive` machinery internally, but combining it
 with the `--interactive` option explicitly is generally not a good

The primary reason is "ignoring merges" can be read in two ways.

 - Some people, including the ones who wrote the original text (and
   also those who haven't complained to the original text since 2008
   because their reading of the phrase was the same as those who
   wrote it), read it as "We will normally replay what appears in
   the output from 'git log --no-merges', losing what was merged
   where into what.  This option instead keeps the shape of the
   history by considering where the merge commits sit in the
   original history".

 - It can however be read in a different way, namely, "We will
   replay what appears in 'git log --first-parent --no-merges'
   output, ignoring all the side branches".  Some other people,
   including you, find that inconsistent with what is actually
   implemented as a normal rebase.

And we agree the updated text is a good change, because it does not
say "ignore merge" anywhere to invite such a confusion.

I am only reacting to your description to justify that change, which
was:

> Ignoring a merge sounds like ignoring the changes a merge commit
> introduces altogether, as if the merge commit was skipped or dropped from
> history. But that is not what happens if "-p" is not specified.

And we agree that the first part of the sentence, up to
"... altogether", is good.

I am saying "as if" part is reintroducing the confusion.

    as if the merge commit was skipped or dropped from history.

To those who did not interpret the existing "ignoring merges", which
meant to be read as "replay the output of 'log --no-merges'
sequencially", the above exactly says "replay the output of 'log
--no-merges'".  That is the reason why they were happy with the
current text, without seeing how it can be read differently.

To them (including me), the justfication is saying "... sounds like
X, as if Y, but it is wrong" using Y that is completely opposite of
X, even they would agree that X is a wrong interpretation caused by
a non-optimal phrasing.

That is why I suggested to avoid "merge commit was skipped or
dropped" in the description and instead say "as if the side branch
that was merged was skipped or dropped".

The following may be agreeable to both camps of people, those who
read "ignoring merge" to mean "log --no-merges" and those who read
it to mean "log --first-parent --no-merges", I would think.

-- >8 --
From: Sebastian Schuberth <sschuberth@gmail.com>
Date: Mon, 30 Mar 2015 11:29:46 +0200
Subject docs: clarify what git-rebase's "-p" / "--preserve-merges" does

Ignoring a merge can be read as ignoring the changes a merge commit
introduces altogether, as if the entire side branch the merge commit
merged was removed from the history.  But that is not what happens
if "-p" is not specified.  What happens is that the individual
commits a merge commit introduces are replayed in order, and only
any possible merge conflict resolutions or manual amendments to the
merge commit are ignored.

Get this straight in the docs.

Also, do not say that merge commits are *tried* to be recreated. As that is
true almost everywhere it is better left unsaid.

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
---
 Documentation/git-rebase.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index d728030..47984e8 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -362,7 +362,9 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`.
 
 -p::
 --preserve-merges::
-	Instead of ignoring merges, try to recreate them.
+	Recreate merge commits instead of flattening the history by replaying
+	commits a merge commit introduces. Merge conflict resolutions or manual
+	amendments to merge commits are not preserved.
 +
 This uses the `--interactive` machinery internally, but combining it
 with the `--interactive` option explicitly is generally not a good

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sebastian Schuberth]

On 30.03.2015 22:23, Junio C Hamano wrote:

> Ignoring a merge can be read as ignoring the changes a merge commit
> introduces altogether, as if the entire side branch the merge commit
> merged was removed from the history.  But that is not what happens

Sounds good to me. Thanks.

-- 
Sebastian Schuberth

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sergey Organov]

Johannes Sixt <j6t@kdbg.org> writes:

>> Junio C Hamano <gitster@pobox.com> writes:

[...]

> The patch was discussed here:
> http://thread.gmane.org/gmane.comp.version-control.git/198125
>
> The reason that this has not progressed any further is this response
> in the thread:
>
> http://thread.gmane.org/gmane.comp.version-control.git/198125/focus=198483
>
> where you basically say that a --first-parent mode is good, but it
> should be separate from --preserve-merges. I haven't made up my mind,
> yet, how to proceed from there.

As far as I can see, there are 2 separate issues:

1. How to calculate the set of commits to rebase.

2. How to rebase merge commits.

Can we leave (1) for a while at its current state and focus on (2)?
Johannes's patch contains a fix for that: use cherry-pick to recreate
merge commits instead of trying to re-merge. Could this change be
accepted alone, as a known fix for current buggy behavior of loosing
user changes in merge commits?

-- Sergey.

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> 1. How to calculate the set of commits to rebase.
>
> 2. How to rebase merge commits.
>
> Can we leave (1) for a while at its current state and focus on (2)?

Perhaps.  You would have to be careful though, so let me think aloud
a bit...

Suppose you started from an upstream history whose tip was B, and
you worked on merging some changes X an Y you made earlier on a side
branch.

         X---Y
        /     \
   O---A---B---Z---

In the meantime, the upstream history has advance and now it looks
like this:

   O---A---B---C---D

You want to forward-port the change done by X, Y on the side branch
and its merge Z on top of D, right?

In other words, you want to have this:

         X-----------Y
        /             \
   O---A---B---C---D---Z'

In this case, replaying the difference going from B to Z on top of D
may be better than redoing a merge between Y and D, in that the
former will carry evil merges and resolution of conflict forward.

I wonder if it will be the right way to get a correct result to
apply the difference to go from B to Z on top of an old commit when
you are side-porting.

Imagine you want to backport the same X-Y history by redoing the
merge Z on top of another child of O (i.e. A's sibling).  That is,
you start from this:


         X---Y
        /     \
   O---A---B---Z---
    \
     M---N

and would want to create this:
    
    
   O           X'--Y'
    \         /     \    
     M---N---A'--B'--Z'--

As long as everything down to the merge-base of the parents of the
original merge (in this example, merge-base across Y and B that are
Z's parents, which is A) is being transplanted, "apply the
difference going from B to Z, on top of B', to obtain Z'" should
work, I would think.

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> 1. How to calculate the set of commits to rebase.
>>
>> 2. How to rebase merge commits.
>>
>> Can we leave (1) for a while at its current state and focus on (2)?
>
> Perhaps.  You would have to be careful though, so let me think aloud
> a bit...

Yeah, care should be taken indeed, and it's not trivial to foresee all
possible troubles from changing to cherry-picking of merge commits.
However, in general it looks like it's better to get some conflict to
deal with from cherry-picking than to miss essential changes silently as
it sometimes happens now.

I also wonder if git remembers in merge commits what merge strategy was
used? If not, then it's yet another argument in favor of cherry-picking.

-- Sergey.

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> I wonder if it will be the right way to get a correct result to
> apply the difference to go from B to Z on top of an old commit when
> you are side-porting.
>
> Imagine you want to backport the same X-Y history by redoing the
> merge Z on top of another child of O (i.e. A's sibling).  That is,
> you start from this:
>
>
>          X---Y
>         /     \
>    O---A---B---Z---
>     \
>      M---N
>
> and would want to create this:
>     
>     
>    O           X'--Y'
>     \         /     \    
>      M---N---A'--B'--Z'--
>
> As long as everything down to the merge-base of the parents of the
> original merge (in this example, merge-base across Y and B that are
> Z's parents, which is A) is being transplanted, "apply the
> difference going from B to Z, on top of B', to obtain Z'" should
> work, I would think.

And just after I send the message because I needed to catch a bus, I
notice that there is a problem.

Actually, "replay diff going from B to Z instead of merging" must be
done very carefully.  Imagine when Y in the original history were a
cherry-pick of M.  What you would be creating would look more like
this instead:
     
    O           X'--.
     \         /     \    
      M---N---A'--B'--Z'--

because Y' becomes a no-op, as the transplanted history already has
M applied.  But the original "diff going from B to Z" has the effect
of M already in there.  You would end up adding the same hunk twice
without noticing.  You somehow need to come up with a way to deal
with this.

If you did a real merge between X' and B' to recreate Z', you would
not have such a problem.

One way to be careful when recreating Z' out of Z might be:

    - Retry a merge between the original B and Y, with conflict
      markers intact;

    - Compare the result with what is recorded in Z.  The
      differences are textual conflict resolution and evil merge
      changes;

    - Now try a merge between B' and Y', with conflict markers
      intact;

    - Apply the difference you obtained in the second step to the
      result of the third step.

which is essentially the same as what rerere does.

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> I wonder if it will be the right way to get a correct result to
>> apply the difference to go from B to Z on top of an old commit when
>> you are side-porting.
>>
>> Imagine you want to backport the same X-Y history by redoing the
>> merge Z on top of another child of O (i.e. A's sibling).  That is,
>> you start from this:
>>
>>
>>          X---Y
>>         /     \
>>    O---A---B---Z---
>>     \
>>      M---N
>>
>> and would want to create this:
>>     
>>     
>>    O           X'--Y'
>>     \         /     \    
>>      M---N---A'--B'--Z'--
>>
>> As long as everything down to the merge-base of the parents of the
>> original merge (in this example, merge-base across Y and B that are
>> Z's parents, which is A) is being transplanted, "apply the
>> difference going from B to Z, on top of B', to obtain Z'" should
>> work, I would think.
>
> And just after I send the message because I needed to catch a bus, I
> notice that there is a problem.
>
> Actually, "replay diff going from B to Z instead of merging" must be
> done very carefully.  Imagine when Y in the original history were a
> cherry-pick of M.  What you would be creating would look more like
> this instead:
>      
>     O           X'--.
>      \         /     \    
>       M---N---A'--B'--Z'--
>
> because Y' becomes a no-op, as the transplanted history already has
> M applied.  But the original "diff going from B to Z" has the effect
> of M already in there.  You would end up adding the same hunk twice
> without noticing. You somehow need to come up with a way to deal
> with this.

Nope. It seems like cherry-pick takes care of that:

[SCRIPT]
git init t; cd t; git config rerere.enabled false

cat > a <<EOF
Line01
Line02
Line03
Line04
Line05
Line06
Line07
Line08
Line09
EOF

git add a
git commit -aqm "A"

git checkout -b two-chunks
sed -i -e 's/Line04/Line04_1/' a
sed -i -e 's/Line08/Line08_1/' a
git commit -aqm "A_04_08_1"

git checkout master
sed -i -e 's/Line04/Line04_1/' a
git commit -aqm "A_04_1"

git cherry-pick two-chunks

git log --oneline
cat a
[/SCRIPT]

What do I miss?

-- Sergey.

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Junio C Hamano]

Sergey Organov <s.organov@javad.com> writes:

> Nope. It seems like cherry-pick takes care of that:
> ...
> What do I miss?

The fact that cherry-pick did not flag it as a potential conflict
situation where a manual verification is required (the cherry-pick
process can be fooled by textual similarity and either add the same
thing twice or fail to add one thing that is needed).

Re: [PATCH] docs: Clarify what git-rebase's "--preserve-merges" does

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <s.organov@javad.com> writes:
>
>> Nope. It seems like cherry-pick takes care of that:
>> ...
>> What do I miss?
>
> The fact that cherry-pick did not flag it as a potential conflict
> situation where a manual verification is required
> (the cherry-pick process can be fooled by textual similarity and
> either add the same thing twice or fail to add one thing that is
> needed).

Well, it was not required in the simple case I tested, and cherry-pick
did the right thing. I suspect it will do the right thing (flag a
conflict) where manual verification is required. A test-case
demonstrating the problem you have in mind, maybe?

Anyway, how is it different to cherry-pick merge commit compared to any
other commit? I mean, provided we cherry-pick other commits, we already
accepted all the possible negative consequences of cherry-picking. How
cherry-picking merge commits make this worse?

I.e., do you think we have a show-stopper, or just that there are ways
to handle merge commits event better than simple "cherry-pick -m1"? The
latter is probably true, but simple cherry-pick still looks much better
than what we have now, no?

-- Sergey.