This commit fixes Avi Kivity's use case of squashing two commits on either side of a merge together. Changes include: - Delaying storing the rewrite information if the current commit we are applying is being squashed. This means storing multiple lines in the current-commit file and recording each of them as rewritten to the same HEAD on the next commit. - Move the "no squashing merges" check into the case statement for merges as previously it was catching catching even single-parent squashes. - Conditionally pass "--first-parent" to `git rev-list` based on whether this is a rebase is preserving merges or not. - At the end, just take the current head for the new branch ref instead of trying to look up what the OLDHEAD was rewritten to. This fails because the OLDHEAD was squashed/moved up earlier in the timeline, so even if we can find its rewritten HEAD, that is no longer what we ended up at. Signed-off-by: Stephen Haberman <stephen@xxxxxxxxxxxxxxxx> --- I agree with Avi on what the rebase -i -p behavior should be for his scenario. This patch makes it so. However, the bane of my existence, t3404 is failing ~12 tests in, which is a real PITA to debug, so please let me know if this is a worthwhile tangent to continue on. (That last change of dropping the OLDHEAD->NEWHEAD guessing is probably what is causing t3404 to fail, but I can't reason why it'd need to do that rather than just use HEAD.) I've read in the archives about the git-sequencer stuff, which sounds cool, my thought is that, if anything, this will clarify git rebase -i -p behavior and add tests that can later be ensured to still pass when git-sequencer is dropped in. git-rebase--interactive.sh | 53 ++++++++++--------- t/t3411-rebase-preserve-around-merges.sh | 83 ++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 25 deletions(-) create mode 100644 t/t3411-rebase-preserve-around-merges.sh diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index edb6ec6..9914111 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -159,13 +159,18 @@ pick_one_preserving_merges () { if test -f "$DOTEST"/current-commit then - current_commit=$(cat "$DOTEST"/current-commit) && - git rev-parse HEAD > "$REWRITTEN"/$current_commit && - rm "$DOTEST"/current-commit || - die "Cannot write current commit's replacement sha1" + if [ "$fast_forward" == "t" ] + then + cat "$DOTEST"/current-commit | while read current_commit + do + git rev-parse HEAD > "$REWRITTEN"/$current_commit + done + rm "$DOTEST"/current-commit || + die "Cannot write current commit's replacement sha1" + fi fi - echo $sha1 > "$DOTEST"/current-commit + echo $sha1 >> "$DOTEST"/current-commit # rewrite parents; if none were rewritten, we can fast-forward. new_parents= @@ -193,15 +198,19 @@ pick_one_preserving_merges () { die "Cannot fast forward to $sha1" ;; 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" + + if [ "$1" != "-n" ] + then + # detach HEAD to current parent + output git checkout $first_parent 2> /dev/null || + die "Cannot move HEAD to $first_parent" + fi case "$new_parents" in ' '*' '*) + test "a$1" = a-n && die "Refusing to squash a merge: $sha1" + # redo merge author_script=$(get_author_ident_from_commit $sha1) eval "$author_script" @@ -350,20 +359,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)" && @@ -561,11 +557,18 @@ first and then run 'git rebase --continue' again." MERGES_OPTION=--no-merges fi + if test t = "$PRESERVE_MERGES" + then + first_parent="--first-parent" + else + first_parent="" + fi + SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM) SHORTHEAD=$(git rev-parse --short $HEAD) SHORTONTO=$(git rev-parse --short $ONTO) git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \ - --abbrev=7 --reverse --left-right --cherry-pick \ + --abbrev=7 --reverse --left-right --cherry-pick $first_parent \ $UPSTREAM...$HEAD | \ sed -n "s/^>/pick /p" > "$TODO" cat >> "$TODO" << EOF diff --git a/t/t3411-rebase-preserve-around-merges.sh b/t/t3411-rebase-preserve-around-merges.sh new file mode 100644 index 0000000..b130f5f --- /dev/null +++ b/t/t3411-rebase-preserve-around-merges.sh @@ -0,0 +1,83 @@ +#!/bin/sh +# +# Copyright (c) 2008 Stephen Haberman +# + +test_description='git rebase preserve merges + +This test runs git rebase with and tries to squash a commit from after a merge +to before the merge. +' +. ./test-lib.sh + +# Copy/paste from t3404-rebase-interactive.sh +echo "#!$SHELL_PATH" >fake-editor.sh +cat >> fake-editor.sh <<\EOF +case "$1" in +*/COMMIT_EDITMSG) + test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1" + test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1" + exit + ;; +esac +test -z "$EXPECT_COUNT" || + test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) || + exit +test -z "$FAKE_LINES" && exit +grep -v '^#' < "$1" > "$1".tmp +rm -f "$1" +cat "$1".tmp +action=pick +for line in $FAKE_LINES; do + case $line in + squash|edit) + action="$line";; + *) + echo sed -n "${line}s/^pick/$action/p" + sed -n "${line}p" < "$1".tmp + sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" + action=pick;; + esac +done +EOF + +test_set_editor "$(pwd)/fake-editor.sh" +chmod a+x fake-editor.sh + +# set up two branches like this: +# +# A - B - D - E - F +# \ / +# - C - + +test_expect_success 'setup' ' + touch a && + touch b && + git add a && + git commit -m A && + git add b && + git commit -m B && + git tag B && + git checkout -b branch && + touch c && + git add c && + git commit -m C && + git checkout master && + touch d && + git add d && + git commit -m D && + git merge branch && + touch f && + git add f && + git commit -m F && + git tag F +' + +test_expect_success 'squash F into D' ' + FAKE_LINES="1 squash 3 2" git rebase -i -p B && + test "$(git rev-parse HEAD^2)" = "$(git rev-parse branch)" && + test "$(git rev-parse HEAD~2)" = "$(git rev-parse B)" +' + +test_done + -- 1.6.0.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html