git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rebase -i: handle "Nothing to do" case with autostash
@ 2014-05-19 22:05 Ramkumar Ramachandra
  2014-05-20  6:55 ` [PATCH] rebase -i: test " Matthieu Moy
  0 siblings, 1 reply; 5+ messages in thread
From: Ramkumar Ramachandra @ 2014-05-19 22:05 UTC (permalink / raw)
  To: Git List
  Cc: Karen Etheridge, Matthieu Moy, Felipe Contreras, Philippe Vaucher

When a user invokes

  $ git rebase -i @~3

with dirty files and rebase.autostash turned on, and exits the $EDITOR
with an empty buffer, the autostash fails to apply. Although the primary
focus of rr/rebase-autostash was to get the git-rebase--backend.sh
scripts to return control to git-rebase.sh, it missed this case in
git-rebase--interactive.sh. Since this case is unlike the other cases
which return control for housekeeping, assign it a special return status
and handle that return value explicitly in git-rebase.sh.

Reported-by: Karen Etheridge <ether@cpan.org>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Thanks to Karen for reporting this.

 I chose 2 arbitrarily. Let me know if you have a rationale for other
 return values.

 git-rebase--interactive.sh |  4 ++--
 git-rebase.sh              | 11 ++++++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..f267d8b 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1049,14 +1049,14 @@ fi
 
 
 has_action "$todo" ||
-	die_abort "Nothing to do"
+	return 2
 
 cp "$todo" "$todo".backup
 git_sequence_editor "$todo" ||
 	die_abort "Could not execute editor"
 
 has_action "$todo" ||
-	die_abort "Nothing to do"
+	return 2
 
 expand_todo_ids
 
diff --git a/git-rebase.sh b/git-rebase.sh
index 4543815..47ca3b9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -155,7 +155,7 @@ move_to_original_branch () {
 	esac
 }
 
-finish_rebase () {
+apply_autostash () {
 	if test -f "$state_dir/autostash"
 	then
 		stash_sha1=$(cat "$state_dir/autostash")
@@ -171,6 +171,10 @@ You can run "git stash pop" or "git stash drop" at any time.
 '
 		fi
 	fi
+}
+
+finish_rebase () {
+	apply_autostash &&
 	git gc --auto &&
 	rm -rf "$state_dir"
 }
@@ -186,6 +190,11 @@ run_specific_rebase () {
 	if test $ret -eq 0
 	then
 		finish_rebase
+	elif test $ret -eq 2 # special exit status for rebase -i
+	then
+		apply_autostash &&
+		rm -rf "$state_dir" &&
+		die "Nothing to do"
 	fi
 	exit $ret
 }
-- 
2.0.0.rc2.20.gfc2568d.dirty

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] rebase -i: test "Nothing to do" case with autostash
  2014-05-19 22:05 [PATCH] rebase -i: handle "Nothing to do" case with autostash Ramkumar Ramachandra
@ 2014-05-20  6:55 ` Matthieu Moy
  2014-05-20  7:22   ` Eric Sunshine
  0 siblings, 1 reply; 5+ messages in thread
From: Matthieu Moy @ 2014-05-20  6:55 UTC (permalink / raw)
  To: git, gitster
  Cc: ether, felipe.contreras, philippe.vaucher, artagnon, Matthieu Moy

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Ram's patch lacks a test. Here it is. Fails without Ram's patch, and
passes with it.

Can be squashed into Ram's patch.

 t/t3420-rebase-autostash.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 90eb264..c2e9a4c 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -167,4 +167,21 @@ testrebase "" .git/rebase-apply
 testrebase " --merge" .git/rebase-merge
 testrebase " --interactive" .git/rebase-merge
 
+test_expect_success 'Abort rebase with --autostash' '
+	git log &&
+	echo new-content >file0 &&
+	(
+		write_script abort-editor.sh <<-\EOF &&
+			echo > "$1"
+		EOF
+		GIT_EDITOR=\"$(pwd)/abort-editor.sh\" &&
+		export GIT_EDITOR &&
+		test_must_fail git rebase -i --autostash HEAD^ &&
+		rm -f abort-editor.sh
+	) &&
+	git status &&
+	echo new-content >expected &&
+	test_cmp expected file0
+'
+
 test_done
-- 
2.0.0.rc3.499.gd6dc9ad

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] rebase -i: test "Nothing to do" case with autostash
  2014-05-20  6:55 ` [PATCH] rebase -i: test " Matthieu Moy
@ 2014-05-20  7:22   ` Eric Sunshine
  2014-05-20  7:49     ` [PATCH v2] " Matthieu Moy
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Sunshine @ 2014-05-20  7:22 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Git List, Junio C Hamano, ether, Felipe Contreras,
	philippe.vaucher, Ramkumar Ramachandra

On Tue, May 20, 2014 at 2:55 AM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Ram's patch lacks a test. Here it is. Fails without Ram's patch, and
> passes with it.
>
> Can be squashed into Ram's patch.
>
>  t/t3420-rebase-autostash.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
> index 90eb264..c2e9a4c 100755
> --- a/t/t3420-rebase-autostash.sh
> +++ b/t/t3420-rebase-autostash.sh
> @@ -167,4 +167,21 @@ testrebase "" .git/rebase-apply
>  testrebase " --merge" .git/rebase-merge
>  testrebase " --interactive" .git/rebase-merge
>
> +test_expect_success 'Abort rebase with --autostash' '
> +       git log &&
> +       echo new-content >file0 &&
> +       (
> +               write_script abort-editor.sh <<-\EOF &&
> +                       echo > "$1"
> +               EOF
> +               GIT_EDITOR=\"$(pwd)/abort-editor.sh\" &&
> +               export GIT_EDITOR &&

Simpler (replace above two lines):

    test_set_editor "$(pwd)/abort-editor.sh" &&

> +               test_must_fail git rebase -i --autostash HEAD^ &&
> +               rm -f abort-editor.sh
> +       ) &&
> +       git status &&
> +       echo new-content >expected &&
> +       test_cmp expected file0
> +'
> +
>  test_done
> --
> 2.0.0.rc3.499.gd6dc9ad

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] rebase -i: test "Nothing to do" case with autostash
  2014-05-20  7:22   ` Eric Sunshine
@ 2014-05-20  7:49     ` Matthieu Moy
  2014-05-20 18:34       ` Junio C Hamano
  0 siblings, 1 reply; 5+ messages in thread
From: Matthieu Moy @ 2014-05-20  7:49 UTC (permalink / raw)
  To: git, gitster
  Cc: ether, felipe.contreras, philippe.vaucher, artagnon, Matthieu Moy

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Eric Sunshine <sunshine@sunshineco.com> writes:

> Simpler (replace above two lines):
>
>     test_set_editor "$(pwd)/abort-editor.sh" &&

Indeed.

And I had debug statements left.

Hopefully, this after-coffee-v2 will be clear enough and correct ;-).

 t/t3420-rebase-autostash.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 90eb264..ff1e2dc 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -167,4 +167,19 @@ testrebase "" .git/rebase-apply
 testrebase " --merge" .git/rebase-merge
 testrebase " --interactive" .git/rebase-merge
 
+test_expect_success 'abort rebase -i with --autostash' '
+	test_when_finished "git reset --hard" &&
+	echo uncommited-content >file0 &&
+	(
+		write_script abort-editor.sh <<-\EOF &&
+			echo > "$1"
+		EOF
+		test_set_editor "$(pwd)/abort-editor.sh" &&
+		test_must_fail git rebase -i --autostash HEAD^ &&
+		rm -f abort-editor.sh
+	) &&
+	echo uncommited-content >expected &&
+	test_cmp expected file0
+'
+
 test_done
-- 
2.0.0.rc3.499.gd6dc9ad

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] rebase -i: test "Nothing to do" case with autostash
  2014-05-20  7:49     ` [PATCH v2] " Matthieu Moy
@ 2014-05-20 18:34       ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-05-20 18:34 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, ether, felipe.contreras, philippe.vaucher, artagnon

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> Simpler (replace above two lines):
>>
>>     test_set_editor "$(pwd)/abort-editor.sh" &&
>
> Indeed.
>
> And I had debug statements left.
>
> Hopefully, this after-coffee-v2 will be clear enough and correct ;-).

Thanks, will queue on top of Ram's fix.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-05-20 18:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-19 22:05 [PATCH] rebase -i: handle "Nothing to do" case with autostash Ramkumar Ramachandra
2014-05-20  6:55 ` [PATCH] rebase -i: test " Matthieu Moy
2014-05-20  7:22   ` Eric Sunshine
2014-05-20  7:49     ` [PATCH v2] " Matthieu Moy
2014-05-20 18:34       ` Junio C Hamano

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).