[Bug?] "git am --abort" loses previous "git add"

[Bug?] "git am --abort" loses previous "git add"

[Junio C Hamano]

Consider this sequence:

    pwd/master$ git add file
    pwd/master$ git am 0001-patch.txt
    Dirty index: cannot apply patches (dirty: file)
    pwd/master|AM$ git status
    # On branch master
    # Changes to be committed:
    #   (use "git reset HEAD <file>..." to unstage)
    #
    #       new file:   file
    #
    # Untracked files:
    #   (use "git add <file>..." to include in what will be committed)
    #
    #       0001-patch.txt
    pwd/master|AM$ git am --abort
    pwd/master$ git status
    # On branch master
    # Untracked files:
    #   (use "git add <file>..." to include in what will be committed)
    #
    #       0001-patch.txt
    #       file

"git am" itself correctly detected that the index is dirty, and refrained
from doing anything (other than creating and leaving .git/rebase-apply).
It carefully kept what you added so far to the index.

But "git am --abort" is broken and loses the added changes from the index.

There probably are two possible fixes (I am not familiar with the --abort
that was bolted on recently, and haven't checked the code).

 - Perhaps when "git am" fails with a dirty index (i.e. not even starting
   to look at the patches and stopping with unapplicable patches), we
   should discard .git/rebase-apply directory so that we do not even have
   to tell users to run "git am --abort";

 - Perhaps "git am --abort" can be told to tell this case from the usual
   "patch in progress" case, and act differently.

My preference obviously would be the latter, as .git/rebase-apply/ could
be the only place that has the patches fed to "git am" from its standard
input.

[PATCH] git-am: Keep index in case of abort with dirty index

[Michael J Gruber]

git am --abort resets the index unconditionally. But in case a previous
git am exited due to a dirty index it is preferable to keep that index.
Make it so.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Something like this?

 git-am.sh |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 8bcb206..7013fea 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -230,8 +230,10 @@ then
 		;;
 	,t)
 		git rerere clear
-		git read-tree --reset -u HEAD ORIG_HEAD
-		git reset ORIG_HEAD
+		test -f "$dotest/dirtyindex" || {
+			git read-tree --reset -u HEAD ORIG_HEAD
+			git reset ORIG_HEAD
+		}
 		rm -fr "$dotest"
 		exit ;;
 	esac
@@ -287,7 +289,11 @@ fi
 case "$resolved" in
 '')
 	files=$(git diff-index --cached --name-only HEAD --) || exit
-	test "$files" && die "Dirty index: cannot apply patches (dirty: $files)"
+	if test "$files"
+	then
+		: >"$dotest/dirtyindex"
+		die "Dirty index: cannot apply patches (dirty: $files)"
+	fi
 esac
 
 if test "$(cat "$dotest/utf8")" = t
-- 
1.6.2.rc1.30.gd43c

Re: [PATCH] git-am: Keep index in case of abort with dirty index

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> git am --abort resets the index unconditionally. But in case a previous
> git am exited due to a dirty index it is preferable to keep that index.
> Make it so.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> Something like this?

Thanks; I think it is a good start.

>  git-am.sh |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/git-am.sh b/git-am.sh
> index 8bcb206..7013fea 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -230,8 +230,10 @@ then
>  		;;
>  	,t)
>  		git rerere clear
> -		git read-tree --reset -u HEAD ORIG_HEAD
> -		git reset ORIG_HEAD
> +		test -f "$dotest/dirtyindex" || {
> +			git read-tree --reset -u HEAD ORIG_HEAD
> +			git reset ORIG_HEAD
> +		}
>  		rm -fr "$dotest"
>  		exit ;;
>  	esac
> @@ -287,7 +289,11 @@ fi
>  case "$resolved" in
>  '')
>  	files=$(git diff-index --cached --name-only HEAD --) || exit
> -	test "$files" && die "Dirty index: cannot apply patches (dirty: $files)"
> +	if test "$files"
> +	then
> +		: >"$dotest/dirtyindex"
> +		die "Dirty index: cannot apply patches (dirty: $files)"
> +	fi
>  esac
>  
>  if test "$(cat "$dotest/utf8")" = t

This certainly would catch this case:

	$ git add hello.c
        $ git am -3 patch.mbox
        ... oops, I had already added my changes
        $ git am --abort

But I think there should be some other code that resets "dirtyindex" flag
file to deal with a case like this:

	... start from a clean index
	$ git am -3 patch.mbox
        ... applies a first few cleanly and creates commits
        ... then stops with a conflict
        $ edit hello.c
	$ git add hello.c
        ... conflict resolved and this is good
        $ git am
        ... oops, I meant --resolved
        $ git am --resolved
        ... goes a bit more and then gets another conflict
        ... after examining the situation, decide the whole series
        ... is not worth it
        $ git am --abort


I guess you would probably want this single liner on top of your patch
(not tested if it fixes the above sequence, though).

diff --git a/git-am.sh b/git-am.sh
index 7013fea..351b4f8 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -237,6 +237,7 @@ then
 		rm -fr "$dotest"
 		exit ;;
 	esac
+	rm -f "$dotest/dirtyindex"
 else
 	# Make sure we are not given --skip, --resolved, nor --abort
 	test "$skip$resolved$abort" = "" ||

[PATCH] git-am: make --abort less dangerous

[Junio C Hamano]

When you are in the middle of "git rebase", "git am --abort" by mistake
would have referred to nonexistent ORIG_HEAD and barfed, or worse yet, used
a stale ORIG_HEAD and taken you to an unexpected commit.

Also the option parsing did not reject "git am --abort --skip".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * An independent fix but textually depends on your patch.

 git-am.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 351b4f8..d339075 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -221,6 +221,9 @@ then
 	resume=yes
 
 	case "$skip,$abort" in
+	t,t)
+		die "Please make up your mind. --skip or --abort?"
+		;;
 	t,)
 		git rerere clear
 		git read-tree --reset -u HEAD HEAD
@@ -229,6 +232,10 @@ then
 		git update-ref ORIG_HEAD $orig_head
 		;;
 	,t)
+		if test -f "$dotest/rebasing"
+		then
+			exec git rebase --abort
+		fi
 		git rerere clear
 		test -f "$dotest/dirtyindex" || {
 			git read-tree --reset -u HEAD ORIG_HEAD