threads / rfc / 30581

RFC patchrebase -p: do not redo the merge, but cherry-pick first-parent changes

Subject: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

## tl;dr

16 messages between May 21, 2012 and May 25, 2012. Diffs are folded; open one to read it.

replies: 15people: 4as markdown or json

Johannes Sixt· May 21, 2012, 20:19 UTC · lore

When rebase -p had to replay a merge commit, it used to redo the merge. But this has drawbacks:

- When the merge was evil, i.e., contained changes that are in neither of
  the parents, that change was not preserved.
- The 'git merge' invocation passed the commit message of the old merge
  commit, but it still obeyed the merge.log option. If it was set, the log
  ended up twice in the commit message.

Replace the 'git merge' by a cherry-pick that picks the changes that the merge introduces with respect to the first parent.

The change in t3409 reflects the new situation after a conflicting merge is replayed: Earlier, 'git merge' found a conflict in an add/add situation with file B only in stage 2 and 3, now it is a regular three-way merge conflict.

A test in t3410 fails now with this new implementation. The branch that is merged in by the old history and that is to be preserved is already merged into the new upstream. In the old implementation, 'git merge' detected this situation and succeeded with "Already up to date", but the new implementation does not notice that it is pointless to keep the merge. It just tries to follow instructions, but fails due to a conflicting cherry-pick. This is a corner case whose correct solution should be that the "pick" instruction is not generated in the first place.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 This is RFC because of the new failure addressed in the last paragraph
 above (see also the art work in the patch context below), how serious
 the failure is, and what to do about it.
 See also this recent thread for further motivation:
 http://thread.gmane.org/gmane.comp.version-control.git/194434/focus=194737
 git-rebase--interactive.sh                | 12 +++++++-----
 t/t3409-rebase-preserve-merges.sh         |  2 +-
 t/t3410-rebase-preserve-dropped-merges.sh |  4 ++--
 3 files changed, 10 insertions(+), 8 deletions(-)
Show changes to 3 files +10 −8

git-rebase--interactive.sh, t/t3409-rebase-preserve-merges.sh, t/t3410-rebase-preserve-dropped-merges.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c19b7c..642daab 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -312,12 +312,14 @@ pick_one_preserving_merges () {
 			msg_content="$(commit_message $sha1)"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
-			if ! do_with_author output \
-				git merge --no-ff ${strategy:+-s $strategy} -m \
-					"$msg_content" $new_parents
+			printf "%s\n" $new_parents >"$GIT_DIR"/MERGE_HEAD
+			printf "%s\n" "$msg_content" >"$GIT_DIR"/MERGE_MSG
+			if output git cherry-pick -m 1 -n "$sha1"
 			then
-				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG
-				die_with_patch $sha1 "Error redoing merge $sha1"
+				do_with_author output git commit --no-verify -F "$GIT_DIR"/MERGE_MSG ||
+					die_with_patch $sha1 "Could not replay merge $sha1"
+			else
+				die_with_patch $sha1 "Could not pick merge $sha1"
 			fi
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 6de4e22..7161b99 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -116,7 +116,7 @@ test_expect_success '--continue works after a conflict' '
 	cd clone2 &&
 	git fetch &&
 	test_must_fail git rebase -p origin/topic &&
-	test 2 = $(git ls-files B | wc -l) &&
+	test 3 = $(git ls-files B | wc -l) &&
 	echo Resolved again > B &&
 	test_must_fail git rebase --continue &&
 	grep "^@@@ " .git/rebase-merge/patch &&
diff --git a/t/t3410-rebase-preserve-dropped-merges.sh b/t/t3410-rebase-preserve-dropped-merges.sh
index 6f73b95..36c69b2 100755
--- a/t/t3410-rebase-preserve-dropped-merges.sh
+++ b/t/t3410-rebase-preserve-dropped-merges.sh
@@ -60,12 +60,12 @@ test_expect_success 'skip same-resolution merges with -p' '
 # A - B - C - D - E
 #   \             \ \
 #     F - G - H -- L2 \        -->   L2
 #       \             |                \
 #         I -- G3 --- J2 -- K2           I -- G3 -- K2
-# G2 = different changes as G
-test_expect_success 'keep different-resolution merges with -p' '
+# G3 = different changes as G
+test_expect_failure 'keep different-resolution merges with -p' '
 	git checkout H &&
 	test_must_fail git merge E &&
 	test_commit L2 file1 23 &&
 	git checkout I &&
 	test_commit G3 file1 4 &&

-- 
1.7.10.2.529.g0c18cfd
Junio C Hamano· May 22, 2012, 18:23 UTC · re: Johannes Sixt · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Johannes Sixt <j6t@kdbg.org> writes:
Show 5 quoted lines
> When rebase -p had to replay a merge commit, it used to redo the merge.
> But this has drawbacks:
>
> - When the merge was evil, i.e., contained changes that are in neither of
>   the parents, that change was not preserved.

This is a desiable property, and not necessarily limited to "evil" merges but also applies to everyday conflict resolutions. Replaying the change between the merge and its first parent is a way to achieve it, but I think it also has downsides. If you are replaying a merge to an updated history that already contains a part of what is merged, some part of the difference between the original merge and its first parent already exists in the commit that the will become the first parent of the replayed merge.

> - The 'git merge' invocation passed the commit message of the old merge
>   commit, but it still obeyed the merge.log option. If it was set, the log
>   ended up twice in the commit message.

This should be fixed independent of this patch, no? Is it a matter of just passing --no-log or something, or is there anything more elaborate necessary?

Johannes Sixt· May 22, 2012, 19:30 UTC · re: Junio C Hamano · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Am 22.05.2012 20:23, schrieb Junio C Hamano:
Show 15 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> When rebase -p had to replay a merge commit, it used to redo the merge.
>> But this has drawbacks:
>>
>> - When the merge was evil, i.e., contained changes that are in neither of
>>   the parents, that change was not preserved.
> 
> This is a desiable property, and not necessarily limited to "evil" merges
> but also applies to everyday conflict resolutions.  Replaying the change
> between the merge and its first parent is a way to achieve it, but I think
> it also has downsides.  If you are replaying a merge to an updated history
> that already contains a part of what is merged, some part of the
> difference between the original merge and its first parent already exists
> in the commit that the will become the first parent of the replayed merge.

Yes, in such a case we need some cleverness from the merge machinery in cherry-pick: It has to recognize that some changes were applied on both branches. This might generate new conflicts.

I'll have to see whether this is an issue in practice.

At any rate, if any difficulties with a particular merge commit can be anticipated, it is always possible to replace the

  pick that-merge-commit
instruction by
  x git merge that-branch
(we could even generate these lines in the instruction sheet as a comment).
Show 7 quoted lines
>> - The 'git merge' invocation passed the commit message of the old merge
>>   commit, but it still obeyed the merge.log option. If it was set, the log
>>   ended up twice in the commit message.
> 
> This should be fixed independent of this patch, no?  Is it a matter of
> just passing --no-log or something, or is there anything more elaborate
> necessary?

Frankly, IMHO[*], rebase -p that is based on git-cherry-pick is so much better that I don't want to think about which workarounds would be needed to make it work better with git-merge :-)

[*] It is still only an opinion. I haven't gotten around to use the updated rebase -p in the field. Another reason that it's still RFC.

Thanks, -- Hannes

Jonathan Nieder· May 22, 2012, 23:38 UTC · re: Johannes Sixt · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Johannes Sixt wrote:
> - The 'git merge' invocation passed the commit message of the old merge
>   commit, but it still obeyed the merge.log option. If it was set, the log
>   ended up twice in the commit message.

Likewise for the "Conflicts:" paragraph, which should also be pretty simple to fix directly. I don't think that points directly to a fundamental flaw in the current implementation, but it does make it seem like no one has been using "rebase -p" for anything nontrivial. :)

[...]
Show 20 quoted lines
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -312,12 +312,14 @@ pick_one_preserving_merges () {
>  			msg_content="$(commit_message $sha1)"
>  			# No point in merging the first parent, that's HEAD
>  			new_parents=${new_parents# $first_parent}
> -			if ! do_with_author output \
> -				git merge --no-ff ${strategy:+-s $strategy} -m \
> -					"$msg_content" $new_parents
> +			printf "%s\n" $new_parents >"$GIT_DIR"/MERGE_HEAD
> +			printf "%s\n" "$msg_content" >"$GIT_DIR"/MERGE_MSG
> +			if output git cherry-pick -m 1 -n "$sha1"
>  			then
> +				do_with_author output git commit --no-verify -F "$GIT_DIR"/MERGE_MSG ||
> +					die_with_patch $sha1 "Could not replay merge $sha1"
> +			else
> -				printf "%s\n" "$msg_content" > "$GIT_DIR"/MERGE_MSG
> -				die_with_patch $sha1 "Error redoing merge $sha1"
> +				die_with_patch $sha1 "Could not pick merge $sha1"
>  			fi
Cute.  The approach looks very sensible to me.

Thanks, Jonathan

Martin von Zweigbergk· May 23, 2012, 15:37 UTC · re: Johannes Sixt · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

On Mon, May 21, 2012 at 1:19 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Replace the 'git merge' by a cherry-pick that picks the changes that the
> merge introduces with respect to the first parent.
Just a reminder of what I said in [1] (the same thread that you referenced):
I think this will break "git rebase -p --onto g b f" on  the below history.
          .-e-.
         /     \
      .-c---d---f
     /
a---b---g

Even if this wasn't the intended use case, git currently supports it and I would personally be a little surprised if no one has gotten used to it working. So should we at least check for this case and handle it with "git merge" as usual? Or stop supporting it and error out instead (and mention in release notes?)?

Btw, with a history without "internal" merges, but where the merge commit was generated "backwards" so the first parent is not in the to-be-rebased history, am I correct in thinking that the "git cherry-pick -m 1" will fail? Do you think we should consider this case or do we consider that too broken to care about?

Martin
 [1] http://thread.gmane.org/gmane.comp.version-control.git/194434/focus=194786
Junio C Hamano· May 23, 2012, 18:53 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
Show 13 quoted lines
> On Mon, May 21, 2012 at 1:19 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Replace the 'git merge' by a cherry-pick that picks the changes that the
>> merge introduces with respect to the first parent.
>
> Just a reminder of what I said in [1] (the same thread that you referenced):
>
> I think this will break "git rebase -p --onto g b f" on  the below history.
>
>           .-e-.
>          /     \
>       .-c---d---f
>      /
> a---b---g
... where the replayed history would look like this:
                  .-e'.
                 /     \
    a---b---g---c'--d'--f'

and e' and e, even though being "moral equivalent", may have different degrees of change since c' and c respectively depending on how changes between b and c and changes between b and g are related to each other.

If the change in the dag b..f does not have much to do with the change between b and g, then the change from c to e would be the same as the change from c' to e', and if the original merge at f resolved conflicts between e and d (or added an evil change), replaying the difference from e to f on top of e' may give a better result.

I think your original point was that the above clean picture would not hold if e-b and g-b has interactions. g-b may contain some change that e-b has independently done, in which case e'-g would be made smaller than e-b when the conflict is resolved while replaying e to produce e' on top of c' (the same applies if you replace e with any commit in the dag "rev-list ^b e d"). The merge to produce f' may result in conflicts, whether you merge e' and d' or replay f-e on top of e'.

A better way to keep potential "evil merges" at f while replaying to produce f' may *not* be by applying the difference f-e on top of e'. Instead, you can learn from what 'rerere' does. That is, to attempt a mechanical merge between e and d and call the result (with possible conflict markers and all) pre-f. If you compare pre-f and f, that is the resolution and evil change made at f. When reproducing f', mechanically merge e' and d', call the result (again, with possible conflict markers and all) pre-f', and try reproducing f' by a 3-way merge between pre-f, pre-f' and f.

Martin von Zweigbergk· May 23, 2012, 20:41 UTC · re: Junio C Hamano · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

On Wed, May 23, 2012 at 11:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> I think your original point was that the above clean picture would not
> hold if e-b and g-b has interactions.  g-b may contain some change that
> e-b has independently done, in which case e'-g would be made smaller than
> e-b when the conflict is resolved while replaying e to produce e' on top
> of c' (the same applies if you replace e with any commit in the dag
> "rev-list ^b e d").  The merge to produce f' may result in conflicts,
> whether you merge e' and d' or replay f-e on top of e'.

Right. Or if e' was changed as a result of a "edit" action (I meant to include '-i' in addition to '-p').

Show 9 quoted lines
> A better way to keep potential "evil merges" at f while replaying to
> produce f' may *not* be by applying the difference f-e on top of e'.
> Instead, you can learn from what 'rerere' does.  That is, to attempt a
> mechanical merge between e and d and call the result (with possible
> conflict markers and all) pre-f.  If you compare pre-f and f, that is the
> resolution and evil change made at f.  When reproducing f', mechanically
> merge e' and d', call the result (again, with possible conflict markers
> and all) pre-f', and try reproducing f' by a 3-way merge between pre-f,
> pre-f' and f.

Yes, I've had the same idea myself. Anyway, as Johannes said, that's probably something to consider for the sequencer.

Martin
Junio C Hamano· May 24, 2012, 17:31 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> Yes, I've had the same idea myself. Anyway, as Johannes said, that's
> probably something to consider for the sequencer.

Are you saying that "rebase -any-variant" and the sequencer should behave differently? It is not immediately obvious to me why it is a good idea.

Martin von Zweigbergk· May 24, 2012, 17:47 UTC · re: Junio C Hamano · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

On Thu, May 24, 2012 at 10:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
>
>> Yes, I've had the same idea myself. Anyway, as Johannes said, that's
>> probably something to consider for the sequencer.
>
> Are you saying that "rebase -any-variant" and the sequencer should behave
> differently?  It is not immediately obvious to me why it is a good idea.

That's not what I meant to say. I thought the sequencer is supposed to replace much of git-rebase and I thought that's what Johannes was referring to as well when he said not to make git-rebase too intelligent. I assumed the reasoning was that any work spent on git-rebase at this point will be thrown away once git-rebase instead calls into the sequencer. But I have not been very involved in the discussions about the sequencer, so I may very well have misunderstood things. Or are you saying that even if it's true that git-rebase will eventually call into the sequencer for much of its operation, this specific part (if at all implemented) does not belong in the sequencer?

Martin
Junio C Hamano· May 24, 2012, 20:09 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
> ... I assumed the reasoning was that any work spent on
> git-rebase at this point will be thrown away once git-rebase instead
> calls into the sequencer....
Ah, sorry I mis-read you.
Johannes Sixt· May 24, 2012, 20:32 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Am 24.05.2012 19:47, schrieb Martin von Zweigbergk:
Show 13 quoted lines
> On Thu, May 24, 2012 at 10:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
>>
>>> Yes, I've had the same idea myself. Anyway, as Johannes said, that's
>>> probably something to consider for the sequencer.
>>
>> Are you saying that "rebase -any-variant" and the sequencer should behave
>> differently?  It is not immediately obvious to me why it is a good idea.
> 
> That's not what I meant to say. I thought the sequencer is supposed to
> replace much of git-rebase and I thought that's what Johannes was
> referring to as well when he said not to make git-rebase too
> intelligent.
You are probably refering to what I said here:
http://thread.gmane.org/gmane.comp.version-control.git/194434/focus=195074

When I wrote the post, I was not aware that rebase -p *is* indeed able to transplant a branchy topic to a new upstream. I was convinced that rebase -p can only move a (first-parent) topic line which may have merged in some unrelated other topics. So, you should take it with a large grain of salt.

-------

Today I was able to use rebase -i -p in the field. I used it to rebuild an integration branch (akin to git's pu branch). Guess what? It did not work as expected:

Two of the topic branches' early parts were already merged in the upstream. The instruction sheet had only 'pick' of merge commits for the topics. Except for these two; there, all commits (that were not yet in upstream) were offered to pick, including the merge commit.

I started with this:
    A--M--o--o   <- master
   /  /
--o--X--Y        <- side branch (partially merged in master)
   \     \
    R--S--N--T   <- integration (to be rebuilt on master)
I wanted this:
     A--M--o--o
    /  /       \
   /  /          R'--S'--N'--T'
--o--X--Y---------------´
But I got this:
    A--M--o--o-------Y'
   /  /       \       \
--o--X--Y      R'--S'--N'--T'

(Note that this has nothing to do with my patch; the badness happens already before any rebasing begins.)

Gah! I'm frustrated. When --preserve-merges was invented, it supported two very important use-cases:

1. Rebuild an integration branch.
2. Rebase a topic that merges an 'unrelated side branch'.

Then people came along thinking that "preserve merges" means that *any* sort of merges should be preserved, including a branchy-and-mergy topic like the example you gave. *Of course* it is much more difficult to support this case. And sure as hell with all the work-arounds needed to support it, a good deal of other good functionality became broken subtly. This is why I say that we should drop support for the complicated cases and resurrect correctness for the simpler, but important cases.

-- Hannes
Junio C Hamano· May 24, 2012, 21:34 UTC · re: Johannes Sixt · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Johannes Sixt <j6t@kdbg.org> writes:
Show 23 quoted lines
> Today I was able to use rebase -i -p in the field. I used it to rebuild
> an integration branch (akin to git's pu branch). Guess what? It did not
> work as expected:
>
> Two of the topic branches' early parts were already merged in the
> upstream. The instruction sheet had only 'pick' of merge commits for the
> topics. Except for these two; there, all commits (that were not yet in
> upstream) were offered to pick, including the merge commit.
>
> I started with this:
>
>     A--M--o--o   <- master
>    /  /
> --o--X--Y        <- side branch (partially merged in master)
>    \     \
>     R--S--N--T   <- integration (to be rebuilt on master)
>
> I wanted this:
>
>      A--M--o--o
>     /  /       \
>    /  /          R'--S'--N'--T'
> --o--X--Y---------------´

It is unclear what exact revisions you gave to rebase, but I am assuming that you asked "rebase --onto master X^ T" (you can use R^ instead of X^; they refer to the same commit).

It is straightforward to see what should happen to R and S; they should be a straight replay of single parent commit on top of 'master'. 

But it is unclear what should happen to X and Y.

When you are rebasing the X^..T sub-DAG, can you say what makes S more special than Y by looking at the original topology? Your "I wanted this" picture depicts S' to be rewritten but Y stays the same. Why?

They are both in the X^..T DAG, and neither of them is merged to 'master'. I can sort of see why X and R would be treated differently (X is part of master, R is not), but I cannot justify why your "I wanted this" picture replays S' without replaying Y'.

What am I missing?
Show 5 quoted lines
> But I got this:
>
>     A--M--o--o-------Y'
>    /  /       \       \
> --o--X--Y      R'--S'--N'--T'

Which is what I would expect, from the "Y and S play a similar role in the sub-DAG X^..T in the original DAG, with respect to master" point of view.

Johannes Sixt· May 25, 2012, 15:58 UTC · re: Junio C Hamano · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Am 24.05.2012 23:34, schrieb Junio C Hamano:
Show 29 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> Today I was able to use rebase -i -p in the field. I used it to rebuild
>> an integration branch (akin to git's pu branch). Guess what? It did not
>> work as expected:
>>
>> Two of the topic branches' early parts were already merged in the
>> upstream. The instruction sheet had only 'pick' of merge commits for the
>> topics. Except for these two; there, all commits (that were not yet in
>> upstream) were offered to pick, including the merge commit.
>>
>> I started with this:
>>
>>     A--M--o--o   <- master
>>    /  /
>> --o--X--Y        <- side branch (partially merged in master)
>>    \     \
>>     R--S--N--T   <- integration (to be rebuilt on master)
>>
>> I wanted this:
>>
>>      A--M--o--o
>>     /  /       \
>>    /  /          R'--S'--N'--T'
>> --o--X--Y---------------´
> 
> It is unclear what exact revisions you gave to rebase, but I am assuming
> that you asked "rebase --onto master X^ T" (you can use R^ instead of X^;
> they refer to the same commit).
Sorry, I should have been more precise. The command was
  git rebase -i -p master T

An important detail is that the order of parents of N is S, Y. (I assumed that this was somewhat clear from the context of my message.)

> It is straightforward to see what should happen to R and S; they should be
> a straight replay of single parent commit on top of 'master'.  
> 
> But it is unclear what should happen to X and Y.

Nothing. Relabel "integration" to "topic" in my artwork above. Then X and Y are on an "unrelated side branch" that is merged into the topic whose tip is at T. The early part of this side branch is already in master. In fact, if X were not merged into master, yet, then 'rebase -i -p' already works as (I) intended: it leaves X and Y alone.

> When you are rebasing the X^..T sub-DAG, can you say what makes S more
> special than Y by looking at the original topology?  Your "I wanted this"
> picture depicts S' to be rewritten but Y stays the same.  Why?

It is "not on topic" (pun intended). It is the second parent of N. Think of X and Y as an independent bug fix sub-topic. It is merged into topic only because T depends on the fix.

Show 6 quoted lines
> They are both in the X^..T DAG, and neither of them is merged to 'master'.
> I can sort of see why X and R would be treated differently (X is part of
> master, R is not), but I cannot justify why your "I wanted this" picture
> replays S' without replaying Y'.
> 
> What am I missing?

First-parentship. When a topic or an integration branch is rebased (with --preserve-merges), only the first-parent chain should be rewritten.

Show 8 quoted lines
>> But I got this:
>>
>>     A--M--o--o-------Y'
>>    /  /       \       \
>> --o--X--Y      R'--S'--N'--T'
> 
> Which is what I would expect, from the "Y and S play a similar role in the
> sub-DAG X^..T in the original DAG, with respect to master" point of view.
-- Hannes
Junio C Hamano· May 25, 2012, 16:58 UTC · re: Johannes Sixt · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Johannes Sixt <j6t@kdbg.org> writes:
> First-parentship. When a topic or an integration branch is rebased (with
> --preserve-merges), only the first-parent chain should be rewritten.

While I think the "start from commit T and down to where it branches off of 'master', rebuild first-parent chain on top of elsewhere" mode has uses, there is no way it can co-exist under the same option name with the current "transplant the DAG as a whole on top of elsewhere, preserving the topology inside the DAG" mode. The name "preserve merges" clicks with the latter mode better than the former mode, at least to me. Perhaps the other mode can be called "rebase --first-parent" or something?

Johannes Sixt· May 25, 2012, 20:03 UTC · re: Junio C Hamano · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Am 25.05.2012 18:58, schrieb Junio C Hamano:
Show 10 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> First-parentship. When a topic or an integration branch is rebased (with
>> --preserve-merges), only the first-parent chain should be rewritten.
> 
> While I think the "start from commit T and down to where it branches off
> of 'master', rebuild first-parent chain on top of elsewhere" mode has
> uses, there is no way it can co-exist under the same option name with the
> current "transplant the DAG as a whole on top of elsewhere, preserving the
> topology inside the DAG" mode.
Fair enough.
>  The name "preserve merges" clicks with the
> latter mode better than the former mode, at least to me.  Perhaps the
> other mode can be called "rebase --first-parent" or something?
Sounds good. I'll take this route.
-- Hannes
Johannes Sixt· May 23, 2012, 18:59 UTC · re: Martin von Zweigbergk · lore

Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

Am 23.05.2012 17:37, schrieb Martin von Zweigbergk:
Show 13 quoted lines
> On Mon, May 21, 2012 at 1:19 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Replace the 'git merge' by a cherry-pick that picks the changes that the
>> merge introduces with respect to the first parent.
> 
> Just a reminder of what I said in [1] (the same thread that you referenced):
> 
> I think this will break "git rebase -p --onto g b f" on  the below history.
> 
>           .-e-.
>          /     \
>       .-c---d---f
>      /
> a---b---g
This still works. It is tested by t3411.3.
Show 5 quoted lines
> Even if this wasn't the intended use case, git currently supports it
> and I would personally be a little surprised if no one has gotten used
> to it working. So should we at least check for this case and handle it
> with "git merge" as usual? Or stop supporting it and error out instead
> (and mention in release notes?)?

YESSS, drop it. It would allow us to remove all sorts of crufty work-arounds for work-arounds. THIS IS NOT 'git sequencer'!

Show 5 quoted lines
> Btw, with a history without "internal" merges, but where the merge
> commit was generated "backwards" so the first parent is not in the
> to-be-rebased history, am I correct in thinking that the "git
> cherry-pick -m 1" will fail? Do you think we should consider this case
> or do we consider that too broken to care about?

I think something odd will happen. If in doubt, I would suggest to consider this such an odd case which rebase is not intended for ;-) (garbage in - garbage out).

-- Hannes

← back to recent threads