From 42f939e4735bc144e1767395e84cffb1bd805a1c Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 15 Oct 2008 02:44:34 -0500 Subject: [PATCH 1/9] rebase-i-p: test to exclude commits from todo based on its parents The first case was based off a script from Avi Kivity . The second case includes a merge-of-a-merge to ensure both are included in todo. Signed-off-by: Stephen Haberman Signed-off-by: Junio C Hamano --- t/t3411-rebase-preserve-around-merges.sh | 135 +++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100755 t/t3411-rebase-preserve-around-merges.sh diff --git a/t/t3411-rebase-preserve-around-merges.sh b/t/t3411-rebase-preserve-around-merges.sh new file mode 100755 index 0000000000..f9c549b3b8 --- /dev/null +++ b/t/t3411-rebase-preserve-around-merges.sh @@ -0,0 +1,135 @@ +#!/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: +# +# A1 - B1 - D1 - E1 - F1 +# \ / +# -- C1 -- + +test_expect_success 'setup' ' + touch a && + touch b && + git add a && + git commit -m A1 && + git tag A1 + git add b && + git commit -m B1 && + git tag B1 && + git checkout -b branch && + touch c && + git add c && + git commit -m C1 && + git checkout master && + touch d && + git add d && + git commit -m D1 && + git merge branch && + touch f && + git add f && + git commit -m F1 && + git tag F1 +' + +# Should result in: +# +# A1 - B1 - D2 - E2 +# \ / +# -- C1 -- +# +test_expect_failure 'squash F1 into D1' ' + FAKE_LINES="1 squash 3 2" git rebase -i -p B1 && + test "$(git rev-parse HEAD^2)" = "$(git rev-parse branch)" && + test "$(git rev-parse HEAD~2)" = "$(git rev-parse B1)" && + git tag E2 +' + +# Start with: +# +# A1 - B1 - D2 - E2 +# \ +# G1 ---- L1 ---- M1 +# \ / +# H1 -- J1 -- K1 +# \ / +# -- I1 -- +# +# And rebase G1..M1 onto E2 + +test_expect_failure 'rebase two levels of merge' ' + git checkout -b branch2 A1 && + touch g && + git add g && + git commit -m G1 && + git checkout -b branch3 && + touch h + git add h && + git commit -m H1 && + git checkout -b branch4 && + touch i && + git add i && + git commit -m I1 && + git tag I1 && + git checkout branch3 && + touch j && + git add j && + git commit -m J1 && + git merge I1 --no-commit && + git commit -m K1 && + git tag K1 && + git checkout branch2 && + touch l && + git add l && + git commit -m L1 && + git merge K1 --no-commit && + git commit -m M1 && + GIT_EDITOR=: git rebase -i -p E2 && + test "$(git rev-parse HEAD~3)" = "$(git rev-parse E2)" && + test "$(git rev-parse HEAD~2)" = "$(git rev-parse HEAD^2^2~2)" && + test "$(git rev-parse HEAD^2^1^1)" = "$(git rev-parse HEAD^2^2^1)" +' + +test_done From 72583e6c685a85b9354ee2310cec3d9240df3c0f Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 15 Oct 2008 02:44:35 -0500 Subject: [PATCH 2/9] rebase-i-p: use HEAD for updating the ref instead of mapping OLDHEAD If OLDHEAD was reordered in the todo, and its mapped NEWHEAD was used to set the ref, commits reordered after OLDHEAD in the todo would should up as un-committed changes. Signed-off-by: Stephen Haberman Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 30e45237a2..c9681178f7 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -376,20 +376,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)" && From bb645071645240a23ede4e93c9425252a0eaaefb Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 15 Oct 2008 02:44:36 -0500 Subject: [PATCH 3/9] rebase-i-p: delay saving current-commit to REWRITTEN if squashing If the current-commit was dumped to REWRITTEN, but then we squash the next commit in to it, we have invalidated the HEAD was just written to REWRITTEN. Instead, append the squash hash to current-commit and save both of them the next time around. Signed-off-by: Stephen Haberman Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index c9681178f7..23cf7a5d68 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -170,13 +170,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= From a4f25e368247108e89e5ee1a02c2f1b6fc31c56f Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 15 Oct 2008 02:44:37 -0500 Subject: [PATCH 4/9] rebase-i-p: fix 'no squashing merges' tripping up non-merges Also only check out the first parent if this commit if not a squash--if it is a squash, we want to explicitly ignore the parent and leave the wc as is, as cherry-pick will apply the squash on top of it. Signed-off-by: Stephen Haberman Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 23cf7a5d68..274251f697 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -219,15 +219,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" From acc8559aa33b7a1ea49b0841e0759ecf05c6f26e Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 15 Oct 2008 02:44:38 -0500 Subject: [PATCH 5/9] rebase-i-p: only list commits that require rewriting in todo This is heavily based on Stephan Beyer's git sequencer rewrite of rebase-i-p. Each commit is still found by rev-list UPSTREAM..HEAD, but a commit is only included in todo if at least one its parents has been marked for rewriting. Signed-off-by: Stephen Haberman Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 77 +++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 25 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 274251f697..851066f0f4 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -579,18 +579,67 @@ first and then run 'git rebase --continue' again." echo $ONTO > "$REWRITTEN"/$c || die "Could not init rewritten commits" done + # No cherry-pick because our first pass is to determine + # parents to rewrite and skipping dropped commits would + # prematurely end our probe MERGES_OPTION= else - MERGES_OPTION=--no-merges + MERGES_OPTION="--no-merges --cherry-pick" 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 --topo-order \ $UPSTREAM...$HEAD | \ - sed -n "s/^>/pick /p" > "$TODO" + sed -n "s/^>//p" | while read shortsha1 rest + do + if test t != "$PRESERVE_MERGES" + then + echo "pick $shortsha1 $rest" >> "$TODO" + else + sha1=$(git rev-parse $shortsha1) + preserve=t + for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-) + do + if test -f "$REWRITTEN"/$p + then + preserve=f + fi + done + if test f = "$preserve" + then + touch "$REWRITTEN"/$sha1 + echo "pick $shortsha1 $rest" >> "$TODO" + fi + fi + done + + # Watch for commits that been dropped by --cherry-pick + if test t = "$PRESERVE_MERGES" + then + mkdir "$DROPPED" + # Save all non-cherry-picked changes + git rev-list $UPSTREAM...$HEAD --left-right --cherry-pick | \ + sed -n "s/^>//p" > "$DOTEST"/not-cherry-picks + # Now all commits and note which ones are missing in + # not-cherry-picks and hence being dropped + git rev-list $UPSTREAM...$HEAD --left-right | \ + sed -n "s/^>//p" | while read rev + do + if test -f "$REWRITTEN"/$rev -a "$(grep "$rev" "$DOTEST"/not-cherry-picks)" = "" + then + # Use -f2 because if rev-list is telling us this commit is + # not worthwhile, we don't want to track its multiple heads, + # just the history of its first-parent for others that will + # be rebasing on top of it + git rev-list --parents -1 $rev | cut -d' ' -f2 > "$DROPPED"/$rev + cat "$TODO" | grep -v "${rev:0:7}" > "${TODO}2" ; mv "${TODO}2" "$TODO" + rm "$REWRITTEN"/$rev + fi + done + fi test -s "$TODO" || echo noop >> "$TODO" cat >> "$TODO" << EOF @@ -606,28 +655,6 @@ first and then run 'git rebase --continue' again." # EOF - # Watch for commits that been dropped by --cherry-pick - if test t = "$PRESERVE_MERGES" - then - mkdir "$DROPPED" - # drop the --cherry-pick parameter this time - git rev-list $MERGES_OPTION --abbrev-commit \ - --abbrev=7 $UPSTREAM...$HEAD --left-right | \ - sed -n "s/^>//p" | while read rev - do - grep --quiet "$rev" "$TODO" - if [ $? -ne 0 ] - then - # Use -f2 because if rev-list is telling this commit is not - # worthwhile, we don't want to track its multiple heads, - # just the history of its first-parent for others that will - # be rebasing on top of us - full=$(git rev-parse $rev) - git rev-list --parents -1 $rev | cut -d' ' -f2 > "$DROPPED"/$full - fi - done - fi - has_action "$TODO" || die_abort "Nothing to do" From d80d6bc146232d81f1bb4bc58e5d89263fd228d4 Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 15 Oct 2008 02:44:39 -0500 Subject: [PATCH 6/9] rebase-i-p: do not include non-first-parent commits touching UPSTREAM This covers an odd boundary case found by Avi Kivity's script where a branch coming off of UPSTREAM is merged into HEAD. Initially it show up in UPSTREAM..HEAD, but technically UPSTREAM is not moving, the rest of head is, so we should not need to rewrite the merge. This adds a check saying we can keep `preserve=t` if `p=UPSTREAM`...unless this is the first first-parent commit in our UPSTREAM..HEAD rev-list, which could very well point to UPSTREAM, but we still need to consider it as rewritten so we start pulling in the rest of the UPSTREAM..HEAD commits that point to it. Signed-off-by: Stephen Haberman Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 851066f0f4..495f554b65 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -583,6 +583,7 @@ first and then run 'git rebase --continue' again." # parents to rewrite and skipping dropped commits would # prematurely end our probe MERGES_OPTION= + first_after_upstream="$(git rev-list --reverse --first-parent $UPSTREAM..$HEAD | head -n 1)" else MERGES_OPTION="--no-merges --cherry-pick" fi @@ -603,7 +604,7 @@ first and then run 'git rebase --continue' again." preserve=t for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-) do - if test -f "$REWRITTEN"/$p + if test -f "$REWRITTEN"/$p -a \( $p != $UPSTREAM -o $sha1 = $first_after_upstream \) then preserve=f fi From 80fe82e4eb365773ba6518c4539c9235ea9a8b2e Mon Sep 17 00:00:00 2001 From: Stephen Haberman Date: Wed, 15 Oct 2008 02:44:40 -0500 Subject: [PATCH 7/9] rebase-i-p: if todo was reordered use HEAD as the rewritten parent This seems like the best guess we can make until git sequencer marks are available. That being said, within the context of re-ordering a commit before its parent in todo, I think applying it on top of the current commit seems like a reasonable assumption of what the user intended. Signed-off-by: Stephen Haberman Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 9 +++++++++ t/t3411-rebase-preserve-around-merges.sh | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) mode change 100755 => 100644 t/t3411-rebase-preserve-around-merges.sh diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 495f554b65..848fbe7d59 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -194,6 +194,15 @@ pick_one_preserving_merges () { if test -f "$REWRITTEN"/$p then new_p=$(cat "$REWRITTEN"/$p) + + # If the todo reordered commits, and our parent is marked for + # rewriting, but hasn't been gotten to yet, assume the user meant to + # drop it on top of the current HEAD + if test -z "$new_p" + then + new_p=$(git rev-parse HEAD) + fi + test $p != $new_p && fast_forward=f case "$new_parents" in *$new_p*) diff --git a/t/t3411-rebase-preserve-around-merges.sh b/t/t3411-rebase-preserve-around-merges.sh old mode 100755 new mode 100644 index f9c549b3b8..aacfaae843 --- a/t/t3411-rebase-preserve-around-merges.sh +++ b/t/t3411-rebase-preserve-around-merges.sh @@ -80,7 +80,7 @@ test_expect_success 'setup' ' # \ / # -- C1 -- # -test_expect_failure 'squash F1 into D1' ' +test_expect_success 'squash F1 into D1' ' FAKE_LINES="1 squash 3 2" git rebase -i -p B1 && test "$(git rev-parse HEAD^2)" = "$(git rev-parse branch)" && test "$(git rev-parse HEAD~2)" = "$(git rev-parse B1)" && @@ -99,7 +99,7 @@ test_expect_failure 'squash F1 into D1' ' # # And rebase G1..M1 onto E2 -test_expect_failure 'rebase two levels of merge' ' +test_expect_success 'rebase two levels of merge' ' git checkout -b branch2 A1 && touch g && git add g && From e249044c67d347dcffff247c72a503a9dd592294 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 20 Oct 2008 16:36:38 -0700 Subject: [PATCH 8/9] rebase-i-p: minimum fix to obvious issues Jeff King noticed that this series uses non-portable ${var:0:7} syntax to splice a string, which is not even in POSIX, in the script. A quick look at around the offending part revealed a few issues, which this commit fixes: * Why filter output from "rev-list --left-right A...B" and look for the ones that begin with ">"? Wouldn't "rev-list A..B" give that? * The abbreviated SHA-1 are made with "rev-list --abbrev=7" into $TODO in an earlier invocation, and it can be more than 7 letters to avoid ambiguity. Not just that "${r:0:7} is not even in POSIX", but use of it here is actively wrong. * There is no point in catting a single file and piping it into grep. Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 848fbe7d59..a563dea9b5 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -635,8 +635,8 @@ first and then run 'git rebase --continue' again." sed -n "s/^>//p" > "$DOTEST"/not-cherry-picks # Now all commits and note which ones are missing in # not-cherry-picks and hence being dropped - git rev-list $UPSTREAM...$HEAD --left-right | \ - sed -n "s/^>//p" | while read rev + git rev-list $UPSTREAM..$HEAD | + while read rev do if test -f "$REWRITTEN"/$rev -a "$(grep "$rev" "$DOTEST"/not-cherry-picks)" = "" then @@ -645,7 +645,8 @@ first and then run 'git rebase --continue' again." # just the history of its first-parent for others that will # be rebasing on top of it git rev-list --parents -1 $rev | cut -d' ' -f2 > "$DROPPED"/$rev - cat "$TODO" | grep -v "${rev:0:7}" > "${TODO}2" ; mv "${TODO}2" "$TODO" + short=$(git rev-list -1 --abbrev-commit --abbrev=7 $rev) + grep -v "^[a-z][a-z]* $short" <"$TODO" > "${TODO}2" ; mv "${TODO}2" "$TODO" rm "$REWRITTEN"/$rev fi done From 4c1360f472ca5706a3dd1eed0b88603cb05d0827 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 22 Oct 2008 11:59:30 -0700 Subject: [PATCH 9/9] git-rebase--interactive.sh: comparision with == is bashism Signed-off-by: Junio C Hamano --- git-rebase--interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index a563dea9b5..0cae3be6f6 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -170,7 +170,7 @@ pick_one_preserving_merges () { if test -f "$DOTEST"/current-commit then - if [ "$fast_forward" == "t" ] + if test "$fast_forward" = t then cat "$DOTEST"/current-commit | while read current_commit do