# [PATCH v2 0/1] Fix rebase -p --onto 5 messages from 2009-07-22 to 2009-07-22. Participants: Greg Price, Johannes Schindelin. Thread: https://gitlist.dev/t/20195 ## Greg Price, 2009-07-22 16:37 Subject: [PATCH v2 0/1] Fix rebase -p --onto Message-ID: <20090722163738.GO7503@vinegar-pot.mit.edu> URL: https://gitlist.dev/e/20090722163738.GO7503%40vinegar-pot.mit.edu ``` I sent a version of the following patch last week to fix the behavior of rebase -p to respect --onto. Some questions were raised and I believe I've addressed all of them. The code already distinguishes the cases Junio described in a natural way, as demonstrated by a test case in the revised patch. The use case Hannes described has never worked, as Dscho explained, and in any case concerns a different part of the code from this patch -- when the great day comes that rebase -i -p can reorder commits, this patch will work smoothly with that. Are there further comments? If not, I believe this is ready for merge. Greg ``` ## Greg Price, 2009-07-22 16:38 Subject: [PATCH v2 1/1] Fix rebase -p --onto Message-ID: <20090722163858.GP7503@vinegar-pot.mit.edu> URL: https://gitlist.dev/e/20090722163858.GP7503%40vinegar-pot.mit.edu In-Reply-To: <20090722163738.GO7503@vinegar-pot.mit.edu> ``` In a rebase with --onto, the correct test for whether we can skip rewriting a commit is if it is already on top of $ONTO, not $UPSTREAM. Without --onto, this distinction does not exist and the behavior does not change. In a situation with two merged branches on a common base X: X---o---o---o---M \ / x---x---x---x Y if we try to move the branches from their base on X to be based on Y, so as to get X Y---o'--o'--o'--M' \ / x'--x'--x'--x' then we fail. The command `git rebase -p --onto Y X M` moves only the first-parent chain, like so: X \ x---x---x---x \ Y---o'--o'--o'--M' because it mistakenly drops the other branch(es) x---x---x---x from the TODO file. This tests and fixes this behavior. Signed-off-by: Greg Price --- git-rebase--interactive.sh | 2 +- t/t3414-rebase-preserve-onto.sh | 80 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletions(-) create mode 100755 t/t3414-rebase-preserve-onto.sh diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index f96d887..23ded48 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -703,7 +703,7 @@ first and then run 'git rebase --continue' again." preserve=t for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-) do - if test -f "$REWRITTEN"/$p -a \( $p != $UPSTREAM -o $sha1 = $first_after_upstream \) + if test -f "$REWRITTEN"/$p -a \( $p != $ONTO -o $sha1 = $first_after_upstream \) then preserve=f fi diff --git a/t/t3414-rebase-preserve-onto.sh b/t/t3414-rebase-preserve-onto.sh new file mode 100755 index 0000000..80019ee --- /dev/null +++ b/t/t3414-rebase-preserve-onto.sh @@ -0,0 +1,80 @@ +#!/bin/sh +# +# Copyright (c) 2009 Greg Price +# + +test_description='git rebase -p should respect --onto + +In a rebase with --onto, we should rewrite all the commits that +aren'"'"'t on top of $ONTO, even if they are on top of $UPSTREAM. +' +. ./test-lib.sh + +. ../lib-rebase.sh + +# Set up branches like this: +# A1---B1---E1---F1---G1 +# \ \ / +# \ \--C1---D1--/ +# H1 + +test_expect_success 'setup' ' + test_commit A1 && + test_commit B1 && + test_commit C1 && + test_commit D1 && + git reset --hard B1 && + test_commit E1 && + test_commit F1 && + test_merge G1 D1 && + git reset --hard A1 && + test_commit H1 +' + +# Now rebase merge G1 from both branches' base B1, both should move: +# A1---B1---E1---F1---G1 +# \ \ / +# \ \--C1---D1--/ +# \ +# H1---E2---F2---G2 +# \ / +# \--C2---D2--/ + +test_expect_success 'rebase from B1 onto H1' ' + git checkout G1 && + git rebase -p --onto H1 B1 && + test "$(git rev-parse HEAD^1^1^1)" = "$(git rev-parse H1)" && + test "$(git rev-parse HEAD^2^1^1)" = "$(git rev-parse H1)" +' + +# On the other hand if rebase from E1 which is within one branch, +# then the other branch stays: +# A1---B1---E1---F1---G1 +# \ \ / +# \ \--C1---D1--/ +# \ \ +# H1-----F3-----G3 + +test_expect_success 'rebase from E1 onto H1' ' + git checkout G1 && + git rebase -p --onto H1 E1 && + test "$(git rev-parse HEAD^1^1)" = "$(git rev-parse H1)" && + test "$(git rev-parse HEAD^2)" = "$(git rev-parse D1)" +' + +# And the same if we rebase from a commit in the second-parent branch. +# A1---B1---E1---F1----G1 +# \ \ \ / +# \ \--C1---D1-\-/ +# \ \ +# H1------D3------G4 + +test_expect_success 'rebase from C1 onto H1' ' + git checkout G1 && + git rev-list --first-parent --pretty=oneline C1..G1 && + git rebase -p --onto H1 C1 && + test "$(git rev-parse HEAD^2^1)" = "$(git rev-parse H1)" && + test "$(git rev-parse HEAD^1)" = "$(git rev-parse F1)" +' + +test_done -- 1.6.3.1.499.ge7b8da ``` ## Johannes Schindelin, 2009-07-22 17:07 Subject: Re: [PATCH v2 0/1] Fix rebase -p --onto Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0907221905360.7343%40intel-tinevez-2-302 In-Reply-To: <20090722163738.GO7503@vinegar-pot.mit.edu> ``` Hi, On Wed, 22 Jul 2009, Greg Price wrote: > [...] when the great day comes that rebase -i -p can reorder commits > [...] If you want to help, I would be so thankful. In that case, it might make more sense to hold off this patch and integrate it into the rebase-i-p patch series, rather than requiring a rebase of rebase-i-p on top of your patches, which will postpone said day only further. Ciao, Dscho ``` ## Greg Price, 2009-07-22 17:36 Subject: Re: [PATCH v2 0/1] Fix rebase -p --onto Message-ID: <20090722173635.GN7878@vinegar-pot.mit.edu> URL: https://gitlist.dev/e/20090722173635.GN7878%40vinegar-pot.mit.edu In-Reply-To: ``` On Wed, Jul 22, 2009 at 07:07:17PM +0200, Johannes Schindelin wrote: > On Wed, 22 Jul 2009, Greg Price wrote: > > [...] when the great day comes that rebase -i -p can reorder commits > > If you want to help, I would be so thankful. In that case, it might make > more sense to hold off this patch and integrate it into the rebase-i-p > patch series, rather than requiring a rebase of rebase-i-p on top of your > patches, which will postpone said day only further. Where is your patch series and what are the big things that need doing before it can be merged? I can't promise a great deal of time up front, but I would be glad to look at it and may be able to pitch in. Because this patch is about --onto together with -p and doesn't really involve -i, I think it still makes sense to merge. It's only one line plus tests, so I don't think it will create that much of a rebase burden. Cheers, Greg ``` ## Johannes Schindelin, 2009-07-22 18:05 Subject: Re: [PATCH v2 0/1] Fix rebase -p --onto Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0907222004370.7343%40intel-tinevez-2-302 In-Reply-To: <20090722173635.GN7878@vinegar-pot.mit.edu> ``` Hi, On Wed, 22 Jul 2009, Greg Price wrote: > On Wed, Jul 22, 2009 at 07:07:17PM +0200, Johannes Schindelin wrote: > > On Wed, 22 Jul 2009, Greg Price wrote: > > > [...] when the great day comes that rebase -i -p can reorder commits > > > > If you want to help, I would be so thankful. In that case, it might make > > more sense to hold off this patch and integrate it into the rebase-i-p > > patch series, rather than requiring a rebase of rebase-i-p on top of your > > patches, which will postpone said day only further. > > Where is your patch series and what are the big things that need doing > before it can be merged? I can't promise a great deal of time up > front, but I would be glad to look at it and may be able to pitch in. > > Because this patch is about --onto together with -p and doesn't really > involve -i, I think it still makes sense to merge. It's only one line > plus tests, so I don't think it will create that much of a rebase burden. Fair enough. I was fooled by the size of the mail; the main part is the test (which is good), though, so strike my objections. Ciao, Dscho ```