diff options
author | Junio C Hamano <gitster@pobox.com> | 2010-01-20 14:42:07 -0800 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2010-01-20 14:42:07 -0800 |
commit | 668993ff19477652d793385b4f27a62891bcd3da (patch) | |
tree | 0ff91d3c252f3faac93f2b5bde88b1076ee173d3 | |
parent | Merge branch 'ns/rebase-auto-squash' (diff) | |
parent | rebase -i: Retain user-edited commit messages after squash/fixup conflicts (diff) | |
download | tgif-668993ff19477652d793385b4f27a62891bcd3da.tar.xz |
Merge branch 'mh/rebase-fixup'
* mh/rebase-fixup:
rebase -i: Retain user-edited commit messages after squash/fixup conflicts
t3404: Set up more of the test repo in the "setup" step
rebase -i: For fixup commands without squashes, do not start editor
rebase -i: Change function make_squash_message into update_squash_message
rebase -i: Extract function do_with_author
rebase -i: Handle the author script all in one place in do_next
rebase -i: Extract a function "commit_message"
rebase -i: Simplify commit counting for generated commit messages
rebase -i: Improve consistency of commit count in generated commit messages
t3404: Test the commit count in commit messages generated by "rebase -i"
rebase -i: Introduce a constant AMEND
rebase -i: Introduce a constant AUTHOR_SCRIPT
rebase -i: Document how temporary files are used
rebase -i: Use symbolic constant $MSG consistently
rebase -i: Use "test -n" instead of "test ! -z"
rebase -i: Inline expression
rebase -i: Remove dead code
rebase -i: Make the condition for an "if" more transparent
-rwxr-xr-x | git-rebase--interactive.sh | 224 | ||||
-rw-r--r-- | t/lib-rebase.sh | 6 | ||||
-rwxr-xr-x | t/t3404-rebase-interactive.sh | 101 |
3 files changed, 219 insertions, 112 deletions
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 2e56e64a17..e551906ecd 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -35,12 +35,67 @@ autosquash move commits that begin with squash!/fixup! under -i require_work_tree DOTEST="$GIT_DIR/rebase-merge" + +# The file containing rebase commands, comments, and empty lines. +# This file is created by "git rebase -i" then edited by the user. As +# the lines are processed, they are removed from the front of this +# file and written to the tail of $DONE. TODO="$DOTEST"/git-rebase-todo + +# The rebase command lines that have already been processed. A line +# is moved here when it is first handled, before any associated user +# actions. DONE="$DOTEST"/done + +# The commit message that is planned to be used for any changes that +# need to be committed following a user interaction. MSG="$DOTEST"/message + +# The file into which is accumulated the suggested commit message for +# squash/fixup commands. When the first of a series of squash/fixups +# is seen, the file is created and the commit message from the +# previous commit and from the first squash/fixup commit are written +# to it. The commit message for each subsequent squash/fixup commit +# is appended to the file as it is processed. +# +# The first line of the file is of the form +# # This is a combination of $COUNT commits. +# where $COUNT is the number of commits whose messages have been +# written to the file so far (including the initial "pick" commit). +# Each time that a commit message is processed, this line is read and +# updated. It is deleted just before the combined commit is made. SQUASH_MSG="$DOTEST"/message-squash + +# If the current series of squash/fixups has not yet included a squash +# command, then this file exists and holds the commit message of the +# original "pick" commit. (If the series ends without a "squash" +# command, then this can be used as the commit message of the combined +# commit without opening the editor.) +FIXUP_MSG="$DOTEST"/message-fixup + +# $REWRITTEN is the name of a directory containing files for each +# commit that is reachable by at least one merge base of $HEAD and +# $UPSTREAM. They are not necessarily rewritten, but their children +# might be. This ensures that commits on merged, but otherwise +# unrelated side branches are left alone. (Think "X" in the man page's +# example.) REWRITTEN="$DOTEST"/rewritten + DROPPED="$DOTEST"/dropped + +# A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and +# GIT_AUTHOR_DATE that will be used for the commit that is currently +# being rebased. +AUTHOR_SCRIPT="$DOTEST"/author-script + +# When an "edit" rebase command is being processed, the SHA1 of the +# commit to be edited is recorded in this file. When "git rebase +# --continue" is executed, if there are any staged changes then they +# will be amended to the HEAD commit, but only provided the HEAD +# commit is still the commit to be edited. When any other rebase +# command is processed, this file is deleted. +AMEND="$DOTEST"/amend + PRESERVE_MERGES= STRATEGY= ONTO= @@ -72,6 +127,11 @@ output () { esac } +# Output the commit message for the specified commit. +commit_message () { + git cat-file commit "$1" | sed "1,/^$/d" +} + run_pre_rebase_hook () { if test -z "$OK_TO_SKIP_PRE_REBASE" && test -x "$GIT_DIR/hooks/pre-rebase" @@ -131,10 +191,10 @@ make_patch () { echo "Root commit" ;; esac > "$DOTEST"/patch - test -f "$DOTEST"/message || - git cat-file commit "$1" | sed "1,/^$/d" > "$DOTEST"/message - test -f "$DOTEST"/author-script || - get_author_ident_from_commit "$1" > "$DOTEST"/author-script + test -f "$MSG" || + commit_message "$1" > "$MSG" + test -f "$AUTHOR_SCRIPT" || + get_author_ident_from_commit "$1" > "$AUTHOR_SCRIPT" } die_with_patch () { @@ -152,13 +212,22 @@ has_action () { sane_grep '^[^#]' "$1" >/dev/null } +# Run command with GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and +# GIT_AUTHOR_DATE exported from the current environment. +do_with_author () { + GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \ + GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \ + GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \ + "$@" +} + pick_one () { no_ff= case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1" test -d "$REWRITTEN" && pick_one_preserving_merges "$@" && return - if test ! -z "$REBASE_ROOT" + if test -n "$REBASE_ROOT" then output git cherry-pick "$@" return @@ -166,11 +235,10 @@ pick_one () { parent_sha1=$(git rev-parse --verify $sha1^) || die "Could not get the parent of $sha1" current_sha1=$(git rev-parse --verify HEAD) - if test "$no_ff$current_sha1" = "$parent_sha1"; then + if test -z "$no_ff" -a "$current_sha1" = "$parent_sha1" + then output git reset --hard $sha1 - test "a$1" = a-n && output git reset --soft $current_sha1 - sha1=$(git rev-parse --short $sha1) - output warn Fast-forward to $sha1 + output warn Fast-forward to $(git rev-parse --short $sha1) else output git cherry-pick "$@" fi @@ -271,14 +339,11 @@ pick_one_preserving_merges () { # redo merge author_script=$(get_author_ident_from_commit $sha1) eval "$author_script" - msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')" + msg="$(commit_message $sha1)" # No point in merging the first parent, that's HEAD new_parents=${new_parents# $first_parent} - if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \ - GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \ - GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \ - output git merge $STRATEGY -m "$msg" \ - $new_parents + if ! do_with_author output \ + git merge $STRATEGY -m "$msg" $new_parents then printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG die_with_patch $sha1 "Error redoing merge $sha1" @@ -302,50 +367,66 @@ nth_string () { esac } -make_squash_message () { +update_squash_messages () { if test -f "$SQUASH_MSG"; then - # We want to be careful about matching only the commit - # message comment lines generated by this function. - # "[snrt][tdh]" matches the nth_string endings. - COUNT=$(($(sed -n "s/^# Th[^0-9]*\([1-9][0-9]*\)[snrt][tdh] commit message.*:/\1/p" \ - < "$SQUASH_MSG" | sed -ne '$p')+1)) - echo "# This is a combination of $COUNT commits." - sed -e 1d -e '2,/^./{ - /^$/d - }' <"$SQUASH_MSG" + mv "$SQUASH_MSG" "$SQUASH_MSG".bak || exit + COUNT=$(($(sed -n \ + -e "1s/^# This is a combination of \(.*\) commits\./\1/p" \ + -e "q" < "$SQUASH_MSG".bak)+1)) + { + echo "# This is a combination of $COUNT commits." + sed -e 1d -e '2,/^./{ + /^$/d + }' <"$SQUASH_MSG".bak + } >$SQUASH_MSG else + commit_message HEAD > "$FIXUP_MSG" || die "Cannot write $FIXUP_MSG" COUNT=2 - echo "# This is a combination of two commits." - echo "# The first commit's message is:" - echo - git cat-file commit HEAD | sed -e '1,/^$/d' + { + echo "# This is a combination of 2 commits." + echo "# The first commit's message is:" + echo + cat "$FIXUP_MSG" + } >$SQUASH_MSG fi case $1 in squash) + rm -f "$FIXUP_MSG" echo echo "# This is the $(nth_string $COUNT) commit message:" echo - git cat-file commit $2 | sed -e '1,/^$/d' + commit_message $2 ;; fixup) echo echo "# The $(nth_string $COUNT) commit message will be skipped:" echo - # Comment the lines of the commit message out using - # "# " rather than "# " to make them less likely to - # confuse the sed regexp above. - git cat-file commit $2 | sed -e '1,/^$/d' -e 's/^/# /' + commit_message $2 | sed -e 's/^/# /' ;; - esac + esac >>$SQUASH_MSG } peek_next_command () { sed -n -e "/^#/d" -e "/^$/d" -e "s/ .*//p" -e "q" < "$TODO" } +# A squash/fixup has failed. Prepare the long version of the squash +# commit message, then die_with_patch. This code path requires the +# user to edit the combined commit message for all commits that have +# been squashed/fixedup so far. So also erase the old squash +# messages, effectively causing the combined commit to be used as the +# new basis for any further squash/fixups. Args: sha1 rest +die_failed_squash() { + mv "$SQUASH_MSG" "$MSG" || exit + rm -f "$FIXUP_MSG" + cp "$MSG" "$GIT_DIR"/MERGE_MSG || exit + warn + warn "Could not apply $1... $2" + die_with_patch $1 "" +} + do_next () { - rm -f "$DOTEST"/message "$DOTEST"/author-script \ - "$DOTEST"/amend || exit + rm -f "$MSG" "$AUTHOR_SCRIPT" "$AMEND" || exit read command sha1 rest < "$TODO" case "$command" in '#'*|''|noop) @@ -373,7 +454,7 @@ do_next () { pick_one $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest" make_patch $sha1 - git rev-parse --verify HEAD > "$DOTEST"/amend + git rev-parse --verify HEAD > "$AMEND" warn "Stopped at $sha1... $rest" warn "You can amend the commit now, with" warn @@ -400,45 +481,34 @@ do_next () { die "Cannot '$squash_style' without a previous commit" mark_action_done - make_squash_message $squash_style $sha1 > "$MSG" - failed=f + update_squash_messages $squash_style $sha1 author_script=$(get_author_ident_from_commit HEAD) + echo "$author_script" > "$AUTHOR_SCRIPT" + eval "$author_script" output git reset --soft HEAD^ - pick_one -n $sha1 || failed=t + pick_one -n $sha1 || die_failed_squash $sha1 "$rest" case "$(peek_next_command)" in squash|s|fixup|f) - USE_OUTPUT=output - MSG_OPT=-F - EDIT_OR_FILE="$MSG" - cp "$MSG" "$SQUASH_MSG" + # This is an intermediate commit; its message will only be + # used in case of trouble. So use the long version: + do_with_author output git commit --no-verify -F "$SQUASH_MSG" || + die_failed_squash $sha1 "$rest" ;; *) - USE_OUTPUT= - MSG_OPT= - EDIT_OR_FILE=-e - rm -f "$SQUASH_MSG" || exit - cp "$MSG" "$GIT_DIR"/SQUASH_MSG - rm -f "$GIT_DIR"/MERGE_MSG || exit + # This is the final command of this squash/fixup group + if test -f "$FIXUP_MSG" + then + do_with_author git commit --no-verify -F "$FIXUP_MSG" || + die_failed_squash $sha1 "$rest" + else + cp "$SQUASH_MSG" "$GIT_DIR"/SQUASH_MSG || exit + rm -f "$GIT_DIR"/MERGE_MSG + do_with_author git commit --no-verify -e || + die_failed_squash $sha1 "$rest" + fi + rm -f "$SQUASH_MSG" "$FIXUP_MSG" ;; esac - echo "$author_script" > "$DOTEST"/author-script - if test $failed = f - then - # This is like --amend, but with a different message - eval "$author_script" - GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \ - GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \ - GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \ - $USE_OUTPUT git commit --no-verify \ - $MSG_OPT "$EDIT_OR_FILE" || failed=t - fi - if test $failed = t - then - cp "$MSG" "$GIT_DIR"/MERGE_MSG - warn - warn "Could not apply $sha1... $rest" - die_with_patch $sha1 "" - fi ;; *) warn "Unknown command: $command $sha1 $rest" @@ -598,21 +668,20 @@ do then : Nothing to commit -- skip this else - . "$DOTEST"/author-script || + . "$AUTHOR_SCRIPT" || die "Cannot find the author identity" amend= - if test -f "$DOTEST"/amend + if test -f "$AMEND" then amend=$(git rev-parse --verify HEAD) - test "$amend" = $(cat "$DOTEST"/amend) || + test "$amend" = $(cat "$AMEND") || die "\ You have uncommitted changes in your working tree. Please, commit them first and then run 'git rebase --continue' again." git reset --soft HEAD^ || die "Cannot rewind the HEAD" fi - export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE && - git commit --no-verify -F "$DOTEST"/message -e || { + do_with_author git commit --no-verify -F "$MSG" -e || { test -n "$amend" && git reset --soft $amend die "Could not commit staged changes." } @@ -739,13 +808,6 @@ first and then run 'git rebase --continue' again." test t = "$VERBOSE" && : > "$DOTEST"/verbose if test t = "$PRESERVE_MERGES" then - # $REWRITTEN contains files for each commit that is - # reachable by at least one merge base of $HEAD and - # $UPSTREAM. They are not necessarily rewritten, but - # their children might be. - # This ensures that commits on merged, but otherwise - # unrelated side branches are left alone. (Think "X" - # in the man page's example.) if test -z "$REBASE_ROOT" then mkdir "$REWRITTEN" && diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 0db8250c58..2d922ae43c 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -2,9 +2,10 @@ # After setting the fake editor with this function, you can # -# - override the commit message with $FAKE_COMMIT_MESSAGE, +# - override the commit message with $FAKE_COMMIT_MESSAGE # - amend the commit message with $FAKE_COMMIT_AMEND # - check that non-commit messages have a certain line count with $EXPECT_COUNT +# - check the commit count in the commit message header with $EXPECT_HEADER_COUNT # - rewrite a rebase -i script as directed by $FAKE_LINES. # $FAKE_LINES consists of a sequence of words separated by spaces. # The following word combinations are possible: @@ -25,6 +26,9 @@ set_fake_editor () { cat >> fake-editor.sh <<\EOF case "$1" in */COMMIT_EDITMSG) + test -z "$EXPECT_HEADER_COUNT" || + test "$EXPECT_HEADER_COUNT" = $(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1") || + exit test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1" test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1" exit diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index d9382e41d3..4e3513709e 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -14,15 +14,20 @@ that the result still makes sense. set_fake_editor -# set up two branches like this: +# Set up the repository like this: # -# A - B - C - D - E (master) +# one - two - three - four (conflict-branch) +# / +# A - B - C - D - E (master) +# | \ +# | F - G - H (branch1) +# | \ +# \ I (branch2) # \ -# F - G - H (branch1) -# \ -# I (branch2) +# J - K - L - M (no-conflict-branch) # -# where A, B, D and G touch the same file. +# where A, B, D and G all touch file1, and one, two, three, four all +# touch file "conflict". test_expect_success 'setup' ' test_commit A file1 && @@ -36,9 +41,20 @@ test_expect_success 'setup' ' test_commit H file5 && git checkout -b branch2 F && test_commit I file6 + git checkout -b conflict-branch A && + for n in one two three four + do + test_commit $n conflict + done && + git checkout -b no-conflict-branch A && + for n in J K L M + do + test_commit $n file$n + done ' test_expect_success 'no changes are a nop' ' + git checkout branch2 && git rebase -i F && test "$(git symbolic-ref -q HEAD)" = "refs/heads/branch2" && test $(git rev-parse I) = $(git rev-parse HEAD) @@ -97,7 +113,7 @@ cat > expect2 << EOF D ======= G ->>>>>>> 91201e5... G +>>>>>>> 51047de... G EOF test_expect_success 'stop on conflicting pick' ' @@ -135,7 +151,8 @@ test_expect_success 'squash' ' test_tick && GIT_AUTHOR_NAME="Nitfol" git commit -m "nitfol" file7 && echo "******************************" && - FAKE_LINES="1 squash 2" git rebase -i --onto master HEAD~2 && + FAKE_LINES="1 squash 2" EXPECT_HEADER_COUNT=2 \ + git rebase -i --onto master HEAD~2 && test B = $(cat file7) && test $(git rev-parse HEAD^) = $(git rev-parse master) ' @@ -230,22 +247,59 @@ test_expect_success 'verbose flag is heeded, even after --continue' ' test_expect_success 'multi-squash only fires up editor once' ' base=$(git rev-parse HEAD~4) && FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 squash 2 squash 3 squash 4" \ + EXPECT_HEADER_COUNT=4 \ git rebase -i $base && test $base = $(git rev-parse HEAD^) && test 1 = $(git show | grep ONCE | wc -l) ' -test_expect_success 'multi-fixup only fires up editor once' ' +test_expect_success 'multi-fixup does not fire up editor' ' git checkout -b multi-fixup E && base=$(git rev-parse HEAD~4) && - FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 fixup 2 fixup 3 fixup 4" \ + FAKE_COMMIT_AMEND="NEVER" FAKE_LINES="1 fixup 2 fixup 3 fixup 4" \ git rebase -i $base && test $base = $(git rev-parse HEAD^) && - test 1 = $(git show | grep ONCE | wc -l) && + test 0 = $(git show | grep NEVER | wc -l) && git checkout to-be-rebased && git branch -D multi-fixup ' +test_expect_success 'commit message used after conflict' ' + git checkout -b conflict-fixup conflict-branch && + base=$(git rev-parse HEAD~4) && + ( + FAKE_LINES="1 fixup 3 fixup 4" && + export FAKE_LINES && + test_must_fail git rebase -i $base + ) && + echo three > conflict && + git add conflict && + FAKE_COMMIT_AMEND="ONCE" EXPECT_HEADER_COUNT=2 \ + git rebase --continue && + test $base = $(git rev-parse HEAD^) && + test 1 = $(git show | grep ONCE | wc -l) && + git checkout to-be-rebased && + git branch -D conflict-fixup +' + +test_expect_success 'commit message retained after conflict' ' + git checkout -b conflict-squash conflict-branch && + base=$(git rev-parse HEAD~4) && + ( + FAKE_LINES="1 fixup 3 squash 4" && + export FAKE_LINES && + test_must_fail git rebase -i $base + ) && + echo three > conflict && + git add conflict && + FAKE_COMMIT_AMEND="TWICE" EXPECT_HEADER_COUNT=2 \ + git rebase --continue && + test $base = $(git rev-parse HEAD^) && + test 2 = $(git show | grep TWICE | wc -l) && + git checkout to-be-rebased && + git branch -D conflict-squash +' + cat > expect-squash-fixup << EOF B @@ -258,6 +312,7 @@ test_expect_success 'squash and fixup generate correct log messages' ' git checkout -b squash-fixup E && base=$(git rev-parse HEAD~4) && FAKE_COMMIT_AMEND="ONCE" FAKE_LINES="1 fixup 2 squash 3 fixup 4" \ + EXPECT_HEADER_COUNT=4 \ git rebase -i $base && git cat-file commit HEAD | sed -e 1,/^\$/d > actual-squash-fixup && test_cmp expect-squash-fixup actual-squash-fixup && @@ -290,24 +345,15 @@ test_expect_success 'squash ignores blank lines' ' ' test_expect_success 'squash works as expected' ' - for n in one two three four - do - echo $n >> file$n && - git add file$n && - git commit -m $n - done && + git checkout -b squash-works no-conflict-branch && one=$(git rev-parse HEAD~3) && - FAKE_LINES="1 squash 3 2" git rebase -i HEAD~3 && + FAKE_LINES="1 squash 3 2" EXPECT_HEADER_COUNT=2 \ + git rebase -i HEAD~3 && test $one = $(git rev-parse HEAD~2) ' test_expect_success 'interrupted squash works as expected' ' - for n in one two three four - do - echo $n >> conflict && - git add conflict && - git commit -m $n - done && + git checkout -b interrupted-squash conflict-branch && one=$(git rev-parse HEAD~3) && ( FAKE_LINES="1 squash 3 2" && @@ -324,12 +370,7 @@ test_expect_success 'interrupted squash works as expected' ' ' test_expect_success 'interrupted squash works as expected (case 2)' ' - for n in one two three four - do - echo $n >> conflict && - git add conflict && - git commit -m $n - done && + git checkout -b interrupted-squash2 conflict-branch && one=$(git rev-parse HEAD~3) && ( FAKE_LINES="3 squash 1 2" && |