# [PATCH] New tests to check rebase with preserve merges 19 messages from 2008-03-22 to 2008-03-22. Participants: Jörg Sommer, Johannes Schindelin, Björn Steinbrink. Thread: https://gitlist.dev/t/12805 ## Jörg Sommer, 2008-03-22 01:19 Subject: [PATCH] rebase with preserve merges should not show merged commits Message-ID: <1206148785-29466-1-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206148785-29466-1-git-send-email-joerg%40alea.gnuu.de ``` The current version of git-rebase--interactive shows the user the commits coming from a merge. M---A---B \ \ o---o---+---o branch Rebasing branch on M with preserve merges gives the commits A and B. But if you mark them for editing or remove them the rebase fails. You must keep them as they are. It's useless to bother the user with these commits and might lead to mistakes. Signed-off-by: Jörg Sommer --- git-rebase--interactive.sh | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 8aa7371..3879841 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -162,6 +162,8 @@ pick_one_preserving_merges () { new_parents="$new_parents $new_p" ;; esac + else + new_parents="$new_parents $p" fi done case $fast_forward in @@ -523,7 +525,7 @@ do SHORTONTO=$(git rev-parse --short $ONTO) git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \ --abbrev=7 --reverse --left-right --cherry-pick \ - $UPSTREAM...$HEAD | \ + --first-parent $UPSTREAM...$HEAD | \ sed -n "s/^>/pick /p" > "$TODO" cat >> "$TODO" << EOF -- 1.5.4.4 ``` ## Jörg Sommer, 2008-03-22 01:19 Subject: [PATCH] Check for non‐foreign commits in rebase-interactive test Message-ID: <1206148785-29466-2-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206148785-29466-2-git-send-email-joerg%40alea.gnuu.de In-Reply-To: <1206148785-29466-1-git-send-email-joerg@alea.gnuu.de> ``` Signed-off-by: Jörg Sommer --- t/t3404-rebase-interactive.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 9cf873f..7d1e469 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -185,7 +185,7 @@ test_expect_success 'retain authorship when squashing' ' test_expect_success '-p handles "no changes" gracefully' ' HEAD=$(git rev-parse HEAD) && - git rebase -i -p HEAD^ && + EXPECT_COUNT=1 git rebase -i -p HEAD^ && test $HEAD = $(git rev-parse HEAD) ' @@ -205,7 +205,7 @@ test_expect_success 'preserve merges with -p' ' test_tick && git commit -m K file1 && test_tick && - git rebase -i -p --onto branch1 master && + EXPECT_COUNT=3 git rebase -i -p --onto branch1 master && test $(git rev-parse HEAD^^2) = $(git rev-parse to-be-preserved) && test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && test $(git show HEAD:file1) = C && -- 1.5.4.4 ``` ## Jörg Sommer, 2008-03-22 01:19 Subject: [PATCH] Handle fast forward correctly in rebase with preserve merges Message-ID: <1206148785-29466-3-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206148785-29466-3-git-send-email-joerg%40alea.gnuu.de In-Reply-To: <1206148785-29466-2-git-send-email-joerg@alea.gnuu.de> ``` Rebase-interactive with preserve merges does fast forward commits while the parent of the old commit is not the parent of the new commit. If the parent of the changed commit is not touched, e.g. has no entry in the REWRITTEN database, a fast forward happens. With these commits “A---B---C” and rebase “A---C---B” would do a fast forward for C which leads to an incorrect result. The fast forward is also not realised, i.e. the HEAD is not updated. After all is done, it was assumed that the new head is the rewritten old head. But if the old head was applied before current head—as in the example above—the commits after the rewritten old head are lost. Signed-off-by: Jörg Sommer --- git-rebase--interactive.sh | 35 ++++++++++++++++++----------------- 1 files changed, 18 insertions(+), 17 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 3879841..04fe3bf 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -144,6 +144,7 @@ pick_one_preserving_merges () { die "Cannot write current commit's replacement sha1" fi + current_sha1=$(git rev-parse --verify HEAD) # rewrite parents; if none were rewritten, we can fast-forward. fast_forward=t preserve=t @@ -166,18 +167,31 @@ pick_one_preserving_merges () { new_parents="$new_parents $p" fi done + + # Don't do a fast forward, if current commit is not the parent of + # the new commit + case "$new_parents" in + ""|" $current_sha1"*) + ;; + *) + fast_forward=f + ;; + esac + case $fast_forward in t) output warn "Fast forward to $sha1" test $preserve = f || echo $sha1 > "$REWRITTEN"/$sha1 + output git reset --hard $sha1 + if test "a$1" = a-n + then + output git reset --soft $current_sha1 + fi ;; f) test "a$1" = a-n && die "Refusing to squash a merge: $sha1" first_parent=$(expr "$new_parents" : ' \([^ ]*\)') - # detach HEAD to current parent - output git checkout $first_parent 2> /dev/null || - die "Cannot move HEAD to $first_parent" echo $sha1 > "$DOTEST"/current-commit case "$new_parents" in @@ -330,20 +344,7 @@ do_next () { HEADNAME=$(cat "$DOTEST"/head-name) && OLDHEAD=$(cat "$DOTEST"/head) && SHORTONTO=$(git rev-parse --short $(cat "$DOTEST"/onto)) && - if test -d "$REWRITTEN" - then - test -f "$DOTEST"/current-commit && - current_commit=$(cat "$DOTEST"/current-commit) && - git rev-parse HEAD > "$REWRITTEN"/$current_commit - if test -f "$REWRITTEN"/$OLDHEAD - then - NEWHEAD=$(cat "$REWRITTEN"/$OLDHEAD) - else - NEWHEAD=$OLDHEAD - fi - else - NEWHEAD=$(git rev-parse HEAD) - fi && + NEWHEAD=$(git rev-parse HEAD) && case $HEADNAME in refs/*) message="$GIT_REFLOG_ACTION: $HEADNAME onto $SHORTONTO)" && -- 1.5.4.4 ``` ## Jörg Sommer, 2008-03-22 01:19 Subject: [PATCH] New tests to check rebase with preserve merges Message-ID: <1206148785-29466-4-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206148785-29466-4-git-send-email-joerg%40alea.gnuu.de In-Reply-To: <1206148785-29466-3-git-send-email-joerg@alea.gnuu.de> ``` Signed-off-by: Jörg Sommer --- t/t3404-rebase-interactive.sh | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 7d1e469..50974f0 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -212,6 +212,33 @@ test_expect_success 'preserve merges with -p' ' test $(git show HEAD~2:file1) = B ' +test_expect_success 'preserve merges with -p (case 2)' ' + test_tick && + EXPECT_COUNT=3 FAKE_LINES="1 3 2" git rebase -v -i -p branch1 && + test $(git rev-parse HEAD^2) = $(git rev-parse to-be-preserved) && + test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && + test $(git show HEAD~2:file1) = B && + test $(git show HEAD~1:file1) = C +' + +test_expect_success 'preserve merges with -p (case 3)' ' + test_tick && + EXPECT_COUNT=3 FAKE_LINES="3 1 2" git rebase -i -p branch1 && + test $(git rev-parse HEAD~2^2) = $(git rev-parse to-be-preserved) && + test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && + test $(git show HEAD~1:file1) = B && + test $(git show HEAD:file1) = C +' + +test_expect_success 'preserve merges really uses fast forward' ' + head=$(git rev-parse HEAD) && + test_tick && + EXPECT_COUNT=3 git rebase -v -i -p branch1 2>pm-ff-err && + cat pm-ff-err && + test $(grep "^Fast forward" pm-ff-err | wc -l) -eq 3 && + test $(git rev-parse HEAD) = $head +' + test_expect_success '--continue tries to commit' ' test_tick && ! git rebase -i --onto new-branch1 HEAD^ && -- 1.5.4.4 ``` ## Johannes Schindelin, 2008-03-22 01:33 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0803220231020.4124%40racer.site In-Reply-To: <1206148785-29466-1-git-send-email-joerg@alea.gnuu.de> ``` Hi, On Sat, 22 Mar 2008, Jörg Sommer wrote: > The current version of git-rebase--interactive shows the user the > commits coming from a merge. > > M---A---B > \ \ > o---o---+---o branch > > Rebasing branch on M with preserve merges gives the commits A and B. But > if you mark them for editing or remove them the rebase fails. You must > keep them as they are. It's useless to bother the user with these > commits and might lead to mistakes. I don't understand. Rebasing with "rebase --onto M" _should_ show A and B. Besides, I think that this would break exactly that case: > @@ -523,7 +525,7 @@ do > SHORTONTO=$(git rev-parse --short $ONTO) > git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \ > --abbrev=7 --reverse --left-right --cherry-pick \ > - $UPSTREAM...$HEAD | \ > + --first-parent $UPSTREAM...$HEAD | \ If I am not mistaken, you now mark A and B to be _not_ in the list of commits all of a sudden, even if A and B _are_ reachable from "branch", but not from "M". So I think this is exactly one of the cases which made me unsure if your expectation was always right. IOW I think this is _very_ easy to get wrong, and needs careful thought. Ciao, Dscho ``` ## Björn Steinbrink, 2008-03-22 01:52 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: <20080322015252.GA7570@atjola.homenet> URL: https://gitlist.dev/e/20080322015252.GA7570%40atjola.homenet In-Reply-To: <1206148785-29466-1-git-send-email-joerg@alea.gnuu.de> ``` On 2008.03.22 02:19:42 +0100, Jörg Sommer wrote: > The current version of git-rebase--interactive shows the user the commits > coming from a merge. > > M---A---B > \ \ > o---o---+---o branch > > Rebasing branch on M with preserve merges gives the commits A and B. But > if you mark them for editing or remove them the rebase fails. You must > keep them as they are. It's useless to bother the user with these commits > and might lead to mistakes. Uhm, why do you completely remove the possibility to edit A instead of fixing the code so that the editing actually works? Björn ``` ## Jörg Sommer, 2008-03-22 09:40 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: <20080322094051.GA30074@alea.gnuu.de> URL: https://gitlist.dev/e/20080322094051.GA30074%40alea.gnuu.de In-Reply-To: <20080322015252.GA7570@atjola.homenet> ``` Hallo Björn, Björn Steinbrink schrieb am Sat 22. Mar, 02:52 (+0100): > On 2008.03.22 02:19:42 +0100, Jörg Sommer wrote: > > The current version of git-rebase--interactive shows the user the commits > > coming from a merge. > > > > M---A---B > > \ \ > > o---o---+---o branch > > > > Rebasing branch on M with preserve merges gives the commits A and B. But > > if you mark them for editing or remove them the rebase fails. You must > > keep them as they are. It's useless to bother the user with these commits > > and might lead to mistakes. > > Uhm, why do you completely remove the possibility to edit A instead of > fixing the code so that the editing actually works? Because I didn't see why it's useful to edit A and create A' and merge in A again, later. M---A---B \ \ C---D---+---o branch M---A--------------B \ \ C---B'---D'---A'---+---o branch Bye, Jörg. -- Viele Leute glauben, dass sie denken, wenn sie lediglich ihre Vorurteile neu ordnen. ``` ## Jörg Sommer, 2008-03-22 09:43 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: <20080322094331.GB30074@alea.gnuu.de> URL: https://gitlist.dev/e/20080322094331.GB30074%40alea.gnuu.de In-Reply-To: ``` Hallo, Johannes Schindelin schrieb am Sat 22. Mar, 02:33 (+0100): > On Sat, 22 Mar 2008, Jörg Sommer wrote: > > > The current version of git-rebase--interactive shows the user the > > commits coming from a merge. > > > > M---A---B > > \ \ > > o---o---+---o branch > > > > Rebasing branch on M with preserve merges gives the commits A and B. But > > if you mark them for editing or remove them the rebase fails. You must > > keep them as they are. It's useless to bother the user with these > > commits and might lead to mistakes. > > I don't understand. Rebasing with "rebase --onto M" > _should_ show A and B. But IMO not “rebase --onto --preserve-merges -i M”. Schöne Grüße, Jörg. -- Geld allein macht nicht glücklich, aber es ist besser in einem Taxi zu weinen, als in der Straßenbahn. (Marcel Reich‐Ranicki) ``` ## Johannes Schindelin, 2008-03-22 11:22 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0803221222210.4124%40racer.site In-Reply-To: <20080322094331.GB30074@alea.gnuu.de> ``` Hi, On Sat, 22 Mar 2008, Jörg Sommer wrote: > Johannes Schindelin schrieb am Sat 22. Mar, 02:33 (+0100): > > On Sat, 22 Mar 2008, Jörg Sommer wrote: > > > > > The current version of git-rebase--interactive shows the user the > > > commits coming from a merge. > > > > > > M---A---B > > > \ \ > > > o---o---+---o branch > > > > > > Rebasing branch on M with preserve merges gives the commits A and B. But > > > if you mark them for editing or remove them the rebase fails. You must > > > keep them as they are. It's useless to bother the user with these > > > commits and might lead to mistakes. > > > > I don't understand. Rebasing with "rebase --onto M" > > _should_ show A and B. > > But IMO not “rebase --onto --preserve-merges -i M”. Umm, yes it should. You are asking to transplant everything up to and including M onto something else. Ciao, Dscho ``` ## Björn Steinbrink, 2008-03-22 12:37 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: <20080322123734.GA10467@atjola.homenet> URL: https://gitlist.dev/e/20080322123734.GA10467%40atjola.homenet In-Reply-To: <20080322094051.GA30074@alea.gnuu.de> ``` On 2008.03.22 10:40:51 +0100, Jörg Sommer wrote: > Hallo Björn, > > Björn Steinbrink schrieb am Sat 22. Mar, 02:52 (+0100): > > On 2008.03.22 02:19:42 +0100, Jörg Sommer wrote: > > > The current version of git-rebase--interactive shows the user the commits > > > coming from a merge. > > > > > > M---A---B > > > \ \ > > > o---o---+---o branch > > > > > > Rebasing branch on M with preserve merges gives the commits A and B. But > > > if you mark them for editing or remove them the rebase fails. You must > > > keep them as they are. It's useless to bother the user with these commits > > > and might lead to mistakes. > > > > Uhm, why do you completely remove the possibility to edit A instead of > > fixing the code so that the editing actually works? > > Because I didn't see why it's useful to edit A and create A' and merge in > A again, later. > > M---A---B > \ \ > C---D---+---o branch > > M---A--------------B > \ \ > C---B'---D'---A'---+---o branch Hm? Why do you have A' and B' on the other side of the merge? Using -p means that you deliberately _disable_ the linearization. The structure of the history is not supposed to change at all. You're just editing A and the merge should pull A(edited) and B in. Björn ``` ## Jörg Sommer, 2008-03-22 14:06 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: <20080322140648.GA3189@alea.gnuu.de> URL: https://gitlist.dev/e/20080322140648.GA3189%40alea.gnuu.de In-Reply-To: <20080322015252.GA7570@atjola.homenet> ``` Hallo Björn, Björn Steinbrink schrieb am Sat 22. Mar, 02:52 (+0100): > On 2008.03.22 02:19:42 +0100, Jörg Sommer wrote: > > The current version of git-rebase--interactive shows the user the commits > > coming from a merge. > > > > M---A---B > > \ \ > > o---o---+---o branch > > > > Rebasing branch on M with preserve merges gives the commits A and B. But > > if you mark them for editing or remove them the rebase fails. You must > > keep them as they are. It's useless to bother the user with these commits > > and might lead to mistakes. > > Uhm, why do you completely remove the possibility to edit A Ahh, now I see what you've tried to say. I did add the option --first-parent for rebase interactive *without* preserve merges, too. I'll update my patch. Bye, Jörg. -- “Science is the game we play with God to find out what his rules are.” ``` ## Jörg Sommer, 2008-03-22 14:08 Subject: [PATCH v2 1/5] rebase with preserve merges should not show merged commits Message-ID: <1206194907-3340-1-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206194907-3340-1-git-send-email-joerg%40alea.gnuu.de In-Reply-To: <1206148785-29466-1-git-send-email-joerg@alea.gnuu.de> ``` The current version of git-rebase--interactive shows the user the commits coming from a merge. M---A---B \ \ o---o---+---o branch Rebasing branch on M with preserve merges gives the commits A and B. But if you mark them for editing or remove them the rebase fails. You must keep them as they are. It's useless to bother the user with these commits and might lead to mistakes. Signed-off-by: Jörg Sommer --- git-rebase--interactive.sh | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 8aa7371..e1ce44e 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -162,6 +162,8 @@ pick_one_preserving_merges () { new_parents="$new_parents $new_p" ;; esac + else + new_parents="$new_parents $p" fi done case $fast_forward in @@ -513,7 +515,7 @@ do echo $ONTO > "$REWRITTEN"/$c || die "Could not init rewritten commits" done - MERGES_OPTION= + MERGES_OPTION=--first-parent else MERGES_OPTION=--no-merges fi -- 1.5.4.4 ``` ## Jörg Sommer, 2008-03-22 14:08 Subject: [PATCH v2 2/5] New test: no merges without preserve merges Message-ID: <1206194907-3340-2-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206194907-3340-2-git-send-email-joerg%40alea.gnuu.de In-Reply-To: <1206194907-3340-1-git-send-email-joerg@alea.gnuu.de> ``` This test checks that no merges are included, if --preserve-merges is not given. To see a difference between with and without merges add a second commit to the branch to-be-preserved. Otherwise you exchange one merge with one commit, which isn't cognizable with EXPECT_COUNT. The for loop in the test looks somewhat strange, but I didn't saw a different way (than || exit 1) to make the test fail if an inner test fails. Recall: The exit code of a for loop is the exit code of the last command in the last pass, i.e. “for a in 1 2; do test $a != 1; do” returns success. Signed-off-by: Jörg Sommer --- t/t3404-rebase-interactive.sh | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) This patch must be applied after the first patch that fixes rebase, because it triggers a bug. diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 9cf873f..8de1f21 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -195,6 +195,9 @@ test_expect_success 'preserve merges with -p' ' git add unrelated-file && test_tick && git commit -m "unrelated" && + echo 2 > unrelated-file && + test_tick && + git commit -m "second unrelated commit" unrelated-file && git checkout -b to-be-rebased master && echo B > file1 && test_tick && @@ -212,6 +215,18 @@ test_expect_success 'preserve merges with -p' ' test $(git show HEAD~2:file1) = B ' +test_expect_success 'no merges without preserve merges' ' + head=$(git rev-parse HEAD) && + test_tick && + EXPECT_COUNT=4 git rebase -i branch1 && + test $(git rev-parse HEAD) != $head && + for i in 0 1 2 3 + do + test $? -eq 0 && + test "$(git rev-list --parents -1 HEAD~$i | tr -dc " ")" = " " + done +' + test_expect_success '--continue tries to commit' ' test_tick && ! git rebase -i --onto new-branch1 HEAD^ && -- 1.5.4.4 ``` ## Jörg Sommer, 2008-03-22 14:08 Subject: [PATCH v2 3/5] Check for non‐foreign commits in rebase-interactive test Message-ID: <1206194907-3340-3-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206194907-3340-3-git-send-email-joerg%40alea.gnuu.de In-Reply-To: <1206194907-3340-2-git-send-email-joerg@alea.gnuu.de> ``` Signed-off-by: Jörg Sommer --- t/t3404-rebase-interactive.sh | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 8de1f21..8a801a0 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -185,7 +185,7 @@ test_expect_success 'retain authorship when squashing' ' test_expect_success '-p handles "no changes" gracefully' ' HEAD=$(git rev-parse HEAD) && - git rebase -i -p HEAD^ && + EXPECT_COUNT=1 git rebase -i -p HEAD^ && test $HEAD = $(git rev-parse HEAD) ' @@ -208,7 +208,7 @@ test_expect_success 'preserve merges with -p' ' test_tick && git commit -m K file1 && test_tick && - git rebase -i -p --onto branch1 master && + EXPECT_COUNT=3 git rebase -i -p --onto branch1 master && test $(git rev-parse HEAD^^2) = $(git rev-parse to-be-preserved) && test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && test $(git show HEAD:file1) = C && -- 1.5.4.4 ``` ## Jörg Sommer, 2008-03-22 14:08 Subject: [PATCH v2 4/5] Handle fast forward correctly in rebase with preserve merges Message-ID: <1206194907-3340-4-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206194907-3340-4-git-send-email-joerg%40alea.gnuu.de In-Reply-To: <1206194907-3340-3-git-send-email-joerg@alea.gnuu.de> ``` Rebase-interactive with preserve merges does fast forward commits while the parent of the old commit is not the parent of the new commit. If the parent of the changed commit is not touched, e.g. has no entry in the REWRITTEN database, a fast forward happens. With these commits “A---B---C” and rebase “A---C---B” would do a fast forward for C which leads to an incorrect result. The fast forward is also not realised, i.e. the HEAD is not updated. After all is done, it was assumed that the new head is the rewritten old head. But if the old head was applied before current head—as in the example above—the commits after the rewritten old head are lost. Signed-off-by: Jörg Sommer --- git-rebase--interactive.sh | 35 ++++++++++++++++++----------------- 1 files changed, 18 insertions(+), 17 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index e1ce44e..8626ef6 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -144,6 +144,7 @@ pick_one_preserving_merges () { die "Cannot write current commit's replacement sha1" fi + current_sha1=$(git rev-parse --verify HEAD) # rewrite parents; if none were rewritten, we can fast-forward. fast_forward=t preserve=t @@ -166,18 +167,31 @@ pick_one_preserving_merges () { new_parents="$new_parents $p" fi done + + # Don't do a fast forward, if current commit is not the parent of + # the new commit + case "$new_parents" in + ""|" $current_sha1"*) + ;; + *) + fast_forward=f + ;; + esac + case $fast_forward in t) output warn "Fast forward to $sha1" test $preserve = f || echo $sha1 > "$REWRITTEN"/$sha1 + output git reset --hard $sha1 + if test "a$1" = a-n + then + output git reset --soft $current_sha1 + fi ;; f) test "a$1" = a-n && die "Refusing to squash a merge: $sha1" first_parent=$(expr "$new_parents" : ' \([^ ]*\)') - # detach HEAD to current parent - output git checkout $first_parent 2> /dev/null || - die "Cannot move HEAD to $first_parent" echo $sha1 > "$DOTEST"/current-commit case "$new_parents" in @@ -330,20 +344,7 @@ do_next () { HEADNAME=$(cat "$DOTEST"/head-name) && OLDHEAD=$(cat "$DOTEST"/head) && SHORTONTO=$(git rev-parse --short $(cat "$DOTEST"/onto)) && - if test -d "$REWRITTEN" - then - test -f "$DOTEST"/current-commit && - current_commit=$(cat "$DOTEST"/current-commit) && - git rev-parse HEAD > "$REWRITTEN"/$current_commit - if test -f "$REWRITTEN"/$OLDHEAD - then - NEWHEAD=$(cat "$REWRITTEN"/$OLDHEAD) - else - NEWHEAD=$OLDHEAD - fi - else - NEWHEAD=$(git rev-parse HEAD) - fi && + NEWHEAD=$(git rev-parse HEAD) && case $HEADNAME in refs/*) message="$GIT_REFLOG_ACTION: $HEADNAME onto $SHORTONTO)" && -- 1.5.4.4 ``` ## Jörg Sommer, 2008-03-22 14:08 Subject: [PATCH v2 5/5] New tests to check rebase with preserve merges Message-ID: <1206194907-3340-5-git-send-email-joerg@alea.gnuu.de> URL: https://gitlist.dev/e/1206194907-3340-5-git-send-email-joerg%40alea.gnuu.de In-Reply-To: <1206194907-3340-4-git-send-email-joerg@alea.gnuu.de> ``` Signed-off-by: Jörg Sommer --- t/t3404-rebase-interactive.sh | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 8a801a0..2172065 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -215,6 +215,33 @@ test_expect_success 'preserve merges with -p' ' test $(git show HEAD~2:file1) = B ' +test_expect_success 'preserve merges with -p (case 2)' ' + test_tick && + EXPECT_COUNT=3 FAKE_LINES="1 3 2" git rebase -v -i -p branch1 && + test $(git rev-parse HEAD^2) = $(git rev-parse to-be-preserved) && + test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && + test $(git show HEAD~2:file1) = B && + test $(git show HEAD~1:file1) = C +' + +test_expect_success 'preserve merges with -p (case 3)' ' + test_tick && + EXPECT_COUNT=3 FAKE_LINES="3 1 2" git rebase -i -p branch1 && + test $(git rev-parse HEAD~2^2) = $(git rev-parse to-be-preserved) && + test $(git rev-parse HEAD~3) = $(git rev-parse branch1) && + test $(git show HEAD~1:file1) = B && + test $(git show HEAD:file1) = C +' + +test_expect_success 'preserve merges really uses fast forward' ' + head=$(git rev-parse HEAD) && + test_tick && + EXPECT_COUNT=3 git rebase -v -i -p branch1 2>pm-ff-err && + cat pm-ff-err && + test $(grep "^Fast forward" pm-ff-err | wc -l) -eq 3 && + test $(git rev-parse HEAD) = $head +' + test_expect_success 'no merges without preserve merges' ' head=$(git rev-parse HEAD) && test_tick && -- 1.5.4.4 ``` ## Johannes Schindelin, 2008-03-22 14:46 Subject: Re: [PATCH v2 1/5] rebase with preserve merges should not show merged commits Message-ID: URL: https://gitlist.dev/e/alpine.LSU.1.00.0803221545100.4124%40racer.site In-Reply-To: <1206194907-3340-1-git-send-email-joerg@alea.gnuu.de> ``` Hi, On Sat, 22 Mar 2008, Jörg Sommer wrote: > The current version of git-rebase--interactive shows the user the > commits coming from a merge. > > M---A---B > \ \ > o---o---+---o branch > > Rebasing branch on M with preserve merges gives the commits A and B. But > if you mark them for editing or remove them the rebase fails. You must > keep them as they are. It's useless to bother the user with these > commits and might lead to mistakes. It is not useless. It's just that you seemed to have found buggy behaviour. I am _totally_ opposed to your patch without even reading more than the commit message. Ciao, Dscho ``` ## Björn Steinbrink, 2008-03-22 15:12 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: <20080322151234.GA19558@atjola.homenet> URL: https://gitlist.dev/e/20080322151234.GA19558%40atjola.homenet In-Reply-To: <20080322140648.GA3189@alea.gnuu.de> ``` On 2008.03.22 15:06:48 +0100, Jörg Sommer wrote: > Björn Steinbrink schrieb am Sat 22. Mar, 02:52 (+0100): > > On 2008.03.22 02:19:42 +0100, Jörg Sommer wrote: > > > The current version of git-rebase--interactive shows the user the commits > > > coming from a merge. > > > > > > M---A---B > > > \ \ > > > o---o---+---o branch > > > > > > Rebasing branch on M with preserve merges gives the commits A and B. But > > > if you mark them for editing or remove them the rebase fails. You must > > > keep them as they are. It's useless to bother the user with these commits > > > and might lead to mistakes. > > > > Uhm, why do you completely remove the possibility to edit A > > Ahh, now I see what you've tried to say. I did add the option > --first-parent for rebase interactive *without* preserve merges, too. > I'll update my patch. I didn't even look at it closely enough to notice that. --preserve-merges preserves the structure of the history. You seem to interpret it as to preserve the merges against the original parents, except for the first one, and that's simply not what it's meant to do. I can see how that might be useful, but you'd have to add that as an additional mode of operation, and not break the normal one. Björn ``` ## Jörg Sommer, 2008-03-22 15:37 Subject: Re: [PATCH] rebase with preserve merges should not show merged commits Message-ID: <20080322153755.GA3498@alea.gnuu.de> URL: https://gitlist.dev/e/20080322153755.GA3498%40alea.gnuu.de In-Reply-To: <20080322151234.GA19558@atjola.homenet> ``` Hallo Björn, Björn Steinbrink schrieb am Sat 22. Mar, 16:12 (+0100): > On 2008.03.22 15:06:48 +0100, Jörg Sommer wrote: > > Björn Steinbrink schrieb am Sat 22. Mar, 02:52 (+0100): > > > On 2008.03.22 02:19:42 +0100, Jörg Sommer wrote: > > > > The current version of git-rebase--interactive shows the user the commits > > > > coming from a merge. > > > > > > > > M---A---B > > > > \ \ > > > > o---o---+---o branch > > > > > > > > Rebasing branch on M with preserve merges gives the commits A and B. But > > > > if you mark them for editing or remove them the rebase fails. You must > > > > keep them as they are. It's useless to bother the user with these commits > > > > and might lead to mistakes. > > > > > > Uhm, why do you completely remove the possibility to edit A > > > > Ahh, now I see what you've tried to say. I did add the option > > --first-parent for rebase interactive *without* preserve merges, too. > > I'll update my patch. > > I didn't even look at it closely enough to notice that. > --preserve-merges preserves the structure of the history. You seem to > interpret it as to preserve the merges against the original parents, > except for the first one, Yes, exactly this is my intent. > and that's simply not what it's meant to do. That's a pity. So it's meant to be for such cases: M---A---B \ \ o---C---+---o branch M---A---B | \ | `-B' \ \ o---C'--+---o branch > I can see how that might be useful, but you'd have to add that as an > additional mode of operation, and not break the normal one. What's the intention of the patch that adds --first-parent somewhere that you've mentioned in the IRC? I would like to send some tests for bugs I've seen. How do I correctly cleanup after rebase failed? It's necessary to not break following tests. test_expect_failure '…' ' … git rebase -i … ' Should I do something like this: test_expect_failure '…' ' … if !git rebase -i …; then git rebase --abort; false fi ' Bye, Jörg. -- Manchmal denke ich, das sicherste Indiz dafür, daß anderswo im Universum intelligentes Leben existiert, ist, daß niemand versucht hat, mit uns Kontakt aufzunehmen. (Calvin und Hobbes) ```