{"thread":{"id":"18018","subject":"[Bug?] \"git am --abort\" loses previous \"git add\"","startedAt":"2009-02-25T18:23:12Z","lastAt":"2009-02-26T19:24:29Z","messageCount":4,"participants":["Junio C Hamano","Michael J Gruber"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"106238","messageId":"7v8wnu2x67.fsf@gitster.siamese.dyndns.org","threadId":"18018","inReplyTo":null,"subject":"[Bug?] \"git am --abort\" loses previous \"git add\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-25T18:23:12Z","receivedAt":"2009-02-25T18:23:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Consider this sequence:\n\n    pwd/master$ git add file\n    pwd/master$ git am 0001-patch.txt\n    Dirty index: cannot apply patches (dirty: file)\n    pwd/master|AM$ git status\n    # On branch master\n    # Changes to be committed:\n    #   (use \"git reset HEAD <file>...\" to unstage)\n    #\n    #       new file:   file\n    #\n    # Untracked files:\n    #   (use \"git add <file>...\" to include in what will be committed)\n    #\n    #       0001-patch.txt\n    pwd/master|AM$ git am --abort\n    pwd/master$ git status\n    # On branch master\n    # Untracked files:\n    #   (use \"git add <file>...\" to include in what will be committed)\n    #\n    #       0001-patch.txt\n    #       file\n\n\"git am\" itself correctly detected that the index is dirty, and refrained\nfrom doing anything (other than creating and leaving .git/rebase-apply).\nIt carefully kept what you added so far to the index.\n\nBut \"git am --abort\" is broken and loses the added changes from the index.\n\nThere probably are two possible fixes (I am not familiar with the --abort\nthat was bolted on recently, and haven't checked the code).\n\n - Perhaps when \"git am\" fails with a dirty index (i.e. not even starting\n   to look at the patches and stopping with unapplicable patches), we\n   should discard .git/rebase-apply directory so that we do not even have\n   to tell users to run \"git am --abort\";\n\n - Perhaps \"git am --abort\" can be told to tell this case from the usual\n   \"patch in progress\" case, and act differently.\n\nMy preference obviously would be the latter, as .git/rebase-apply/ could\nbe the only place that has the patches fed to \"git am\" from its standard\ninput.\n"},{"id":"106309","messageId":"1235641973-18307-1-git-send-email-git@drmicha.warpmail.net","threadId":"18018","inReplyTo":"7v8wnu2x67.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-am: Keep index in case of abort with dirty index","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-02-26T09:52:53Z","receivedAt":"2009-02-26T09:52:53Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"git am --abort resets the index unconditionally. But in case a previous\ngit am exited due to a dirty index it is preferable to keep that index.\nMake it so.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nSomething like this?\n\n git-am.sh |   12 +++++++++---\n 1 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 8bcb206..7013fea 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -230,8 +230,10 @@ then\n \t\t;;\n \t,t)\n \t\tgit rerere clear\n-\t\tgit read-tree --reset -u HEAD ORIG_HEAD\n-\t\tgit reset ORIG_HEAD\n+\t\ttest -f \"$dotest/dirtyindex\" || {\n+\t\t\tgit read-tree --reset -u HEAD ORIG_HEAD\n+\t\t\tgit reset ORIG_HEAD\n+\t\t}\n \t\trm -fr \"$dotest\"\n \t\texit ;;\n \tesac\n@@ -287,7 +289,11 @@ fi\n case \"$resolved\" in\n '')\n \tfiles=$(git diff-index --cached --name-only HEAD --) || exit\n-\ttest \"$files\" && die \"Dirty index: cannot apply patches (dirty: $files)\"\n+\tif test \"$files\"\n+\tthen\n+\t\t: >\"$dotest/dirtyindex\"\n+\t\tdie \"Dirty index: cannot apply patches (dirty: $files)\"\n+\tfi\n esac\n \n if test \"$(cat \"$dotest/utf8\")\" = t\n-- \n1.6.2.rc1.30.gd43c\n"},{"id":"106351","messageId":"7vfxi1rohn.fsf@gitster.siamese.dyndns.org","threadId":"18018","inReplyTo":"1235641973-18307-1-git-send-email-git@drmicha.warpmail.net","subject":"Re: [PATCH] git-am: Keep index in case of abort with dirty index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-26T19:23:48Z","receivedAt":"2009-02-26T19:23:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> git am --abort resets the index unconditionally. But in case a previous\n> git am exited due to a dirty index it is preferable to keep that index.\n> Make it so.\n>\n> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n> ---\n> Something like this?\n\nThanks; I think it is a good start.\n\n>  git-am.sh |   12 +++++++++---\n>  1 files changed, 9 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-am.sh b/git-am.sh\n> index 8bcb206..7013fea 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -230,8 +230,10 @@ then\n>  \t\t;;\n>  \t,t)\n>  \t\tgit rerere clear\n> -\t\tgit read-tree --reset -u HEAD ORIG_HEAD\n> -\t\tgit reset ORIG_HEAD\n> +\t\ttest -f \"$dotest/dirtyindex\" || {\n> +\t\t\tgit read-tree --reset -u HEAD ORIG_HEAD\n> +\t\t\tgit reset ORIG_HEAD\n> +\t\t}\n>  \t\trm -fr \"$dotest\"\n>  \t\texit ;;\n>  \tesac\n> @@ -287,7 +289,11 @@ fi\n>  case \"$resolved\" in\n>  '')\n>  \tfiles=$(git diff-index --cached --name-only HEAD --) || exit\n> -\ttest \"$files\" && die \"Dirty index: cannot apply patches (dirty: $files)\"\n> +\tif test \"$files\"\n> +\tthen\n> +\t\t: >\"$dotest/dirtyindex\"\n> +\t\tdie \"Dirty index: cannot apply patches (dirty: $files)\"\n> +\tfi\n>  esac\n>  \n>  if test \"$(cat \"$dotest/utf8\")\" = t\n\nThis certainly would catch this case:\n\n\t$ git add hello.c\n        $ git am -3 patch.mbox\n        ... oops, I had already added my changes\n        $ git am --abort\n\nBut I think there should be some other code that resets \"dirtyindex\" flag\nfile to deal with a case like this:\n\n\t... start from a clean index\n\t$ git am -3 patch.mbox\n        ... applies a first few cleanly and creates commits\n        ... then stops with a conflict\n        $ edit hello.c\n\t$ git add hello.c\n        ... conflict resolved and this is good\n        $ git am\n        ... oops, I meant --resolved\n        $ git am --resolved\n        ... goes a bit more and then gets another conflict\n        ... after examining the situation, decide the whole series\n        ... is not worth it\n        $ git am --abort\n\n\nI guess you would probably want this single liner on top of your patch\n(not tested if it fixes the above sequence, though).\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 7013fea..351b4f8 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -237,6 +237,7 @@ then\n \t\trm -fr \"$dotest\"\n \t\texit ;;\n \tesac\n+\trm -f \"$dotest/dirtyindex\"\n else\n \t# Make sure we are not given --skip, --resolved, nor --abort\n \ttest \"$skip$resolved$abort\" = \"\" ||\n"},{"id":"106352","messageId":"7v8wntrogi.fsf@gitster.siamese.dyndns.org","threadId":"18018","inReplyTo":"1235641973-18307-1-git-send-email-git@drmicha.warpmail.net","subject":"[PATCH] git-am: make --abort less dangerous","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-26T19:24:29Z","receivedAt":"2009-02-26T19:24:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When you are in the middle of \"git rebase\", \"git am --abort\" by mistake\nwould have referred to nonexistent ORIG_HEAD and barfed, or worse yet, used\na stale ORIG_HEAD and taken you to an unexpected commit.\n\nAlso the option parsing did not reject \"git am --abort --skip\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * An independent fix but textually depends on your patch.\n\n git-am.sh |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 351b4f8..d339075 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -221,6 +221,9 @@ then\n \tresume=yes\n \n \tcase \"$skip,$abort\" in\n+\tt,t)\n+\t\tdie \"Please make up your mind. --skip or --abort?\"\n+\t\t;;\n \tt,)\n \t\tgit rerere clear\n \t\tgit read-tree --reset -u HEAD HEAD\n@@ -229,6 +232,10 @@ then\n \t\tgit update-ref ORIG_HEAD $orig_head\n \t\t;;\n \t,t)\n+\t\tif test -f \"$dotest/rebasing\"\n+\t\tthen\n+\t\t\texec git rebase --abort\n+\t\tfi\n \t\tgit rerere clear\n \t\ttest -f \"$dotest/dirtyindex\" || {\n \t\t\tgit read-tree --reset -u HEAD ORIG_HEAD\n"}]}