git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phil Hord <hordp@cisco.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, phil.hord@gmail.com,
	Phil Hord <hordp@cisco.com>, Fabian Ruch <bafain@gmail.com>
Subject: [PATCH] rebase -i: redo tasks that die during cherry-pick
Date: Tue, 28 Apr 2015 18:55:20 -0400	[thread overview]
Message-ID: <1430261720-9051-1-git-send-email-hordp@cisco.com> (raw)

When rebase--interactive processes a task, it removes the item from
the todo list and appends it to another list of executed tasks. If a
pick (this includes squash and fixup) fails before the index has
recorded the changes, take the corresponding item and put it on the todo
list again. Otherwise, the changes introduced by the scheduled commit
would be lost.

That kind of decision is possible since the cherry-pick command
signals why it failed to apply the changes of the given commit. Either
the changes are recorded in the index using a conflict (return value 1)
and rebase does not continue until they are resolved or the changes
are not recorded in the index (return value neither 0 nor 1) and
rebase has to try again with the same task.

Add a test cases for regression testing to the "rebase-interactive"
test suite.

Signed-off-by: Fabian Ruch <bafain@gmail.com>
Signed-off-by: Phil Hord <hordp@cisco.com>
---

Notes:
    Last year in ${gmane}/250126 Fabian Ruch helpfully provided a patch
    to fix a rebase bug I complained about. I have simplified it a bit
    and merged in the tests which had been in a separate commit.
    
    It has bitten me twice since the original discussion and has also
    been reported by others, though I haven't found those emails to
    add them to the CC list yet.
    
    CC: Michael Haggerty <mhagger@alum.mit.edu>

 git-rebase--interactive.sh    | 16 +++++++++++++++
 t/t3404-rebase-interactive.sh | 47 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 08e5d86..bab0dcc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -132,6 +132,16 @@ mark_action_done () {
 	fi
 }
 
+# Put the last action marked done at the beginning of the todo list
+# again. If there has not been an action marked done yet, leave the list of
+# items on the todo list unchanged.
+reschedule_last_action () {
+	tail -n 1 "$done" | cat - "$todo" >"$todo".new
+	sed -e \$d <"$done" >"$done".new
+	mv -f "$todo".new "$todo"
+	mv -f "$done".new "$done"
+}
+
 append_todo_help () {
 	git stripspace --comment-lines >>"$todo" <<\EOF
 
@@ -252,6 +262,12 @@ pick_one () {
 	output eval git cherry-pick \
 			${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")} \
 			"$strategy_args" $empty_args $ff "$@"
+
+	# If cherry-pick dies it leaves the to-be-picked commit unrecorded. Reschedule
+	# previous task so this commit is not lost.
+	ret=$?
+	case "$ret" in [01]) ;; *) reschedule_last_action ;; esac
+	return $ret
 }
 
 pick_one_preserving_merges () {
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index eed76cc..ac429a0 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1055,4 +1055,51 @@ test_expect_success 'todo count' '
 	grep "^# Rebase ..* onto ..* ([0-9]" actual
 '
 
+test_expect_success 'rebase -i commits that overwrite untracked files (pick)' '
+	git checkout --force branch2 &&
+	git clean -f &&
+	set_fake_editor &&
+	FAKE_LINES="edit 1 2" git rebase -i A &&
+	test_cmp_rev HEAD F &&
+	test_path_is_missing file6 &&
+	>file6 &&
+	test_must_fail git rebase --continue &&
+	test_cmp_rev HEAD F &&
+	rm file6 &&
+	git rebase --continue &&
+	test_cmp_rev HEAD I
+'
+
+test_expect_success 'rebase -i commits that overwrite untracked files (squash)' '
+	git checkout --force branch2 &&
+	git clean -f &&
+	git tag original-branch2 &&
+	set_fake_editor &&
+	FAKE_LINES="edit 1 squash 2" git rebase -i A &&
+	test_cmp_rev HEAD F &&
+	test_path_is_missing file6 &&
+	>file6 &&
+	test_must_fail git rebase --continue &&
+	test_cmp_rev HEAD F &&
+	rm file6 &&
+	git rebase --continue &&
+	test $(git cat-file commit HEAD | sed -ne \$p) = I &&
+	git reset --hard original-branch2
+'
+
+test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' '
+	git checkout --force branch2 &&
+	git clean -f &&
+	set_fake_editor &&
+	FAKE_LINES="edit 1 2" git rebase -i --no-ff A &&
+	test $(git cat-file commit HEAD | sed -ne \$p) = F &&
+	test_path_is_missing file6 &&
+	>file6 &&
+	test_must_fail git rebase --continue &&
+	test $(git cat-file commit HEAD | sed -ne \$p) = F &&
+	rm file6 &&
+	git rebase --continue &&
+	test $(git cat-file commit HEAD | sed -ne \$p) = I
+'
+
 test_done
-- 
2.4.0.rc3.329.gd1f7d3b

             reply	other threads:[~2015-04-28 23:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-28 22:55 Phil Hord [this message]
2015-04-28 23:17 ` [PATCH] rebase -i: redo tasks that die during cherry-pick Johannes Schindelin
2015-04-29 17:15   ` Junio C Hamano
2015-04-29 19:32     ` Phil Hord
2015-04-29 19:41       ` Junio C Hamano
2015-04-30  7:49     ` rebase -i's todo/done list, was " Johannes Schindelin
2015-04-30  9:54 ` Fabian Ruch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1430261720-9051-1-git-send-email-hordp@cisco.com \
    --to=hordp@cisco.com \
    --cc=bafain@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phil.hord@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).