{"thread":{"id":"39542","subject":"[PATCH 0/6] am --skip/--abort: improve index/worktree cleanup","startedAt":"2015-06-06T11:46:06Z","lastAt":"2015-06-09T09:18:14Z","messageCount":13,"participants":["Paul Tan","Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"263095","messageId":"1433591172-27077-1-git-send-email-pyokagan@gmail.com","threadId":"39542","inReplyTo":null,"subject":"[PATCH 0/6] am --skip/--abort: improve index/worktree cleanup","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-06T11:46:06Z","receivedAt":"2015-06-06T11:46:06Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Currently git-am attempts to clean up the index/worktree when skipping or\naborting, but does not do it very well:\n\n* While it discards conflicted index entries, it does not remove any other\n  modifications made to the index due to a previous threeway merge.\n\n* It expects HEAD/ORIG_HEAD to exist, and thus does not clean up the index when\n  on an unborn branch.\n\nThis patch series addresses the above two general problems by making the calls\nto git-read-tree aware of the differences between our current index and\nHEAD/ORIG_HEAD, and by explictly defining what happens when we are on an unborn\nbranch.\n\n\nPaul Tan (6):\n  am --skip: revert changes introduced by failed 3way merge\n  am -3: support 3way merge on unborn branch\n  am --skip: support skipping while on unborn branch\n  am --abort: revert changes introduced by failed 3way merge\n  am --abort: support aborting to unborn branch\n  am --abort: keep unrelated commits on unborn branch\n\n git-am.sh           | 31 ++++++++++++++------\n t/t4151-am-abort.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 104 insertions(+), 8 deletions(-)\n\n-- \n2.1.4\n"},{"id":"263096","messageId":"1433591172-27077-2-git-send-email-pyokagan@gmail.com","threadId":"39542","inReplyTo":"1433591172-27077-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 1/6] am --skip: revert changes introduced by failed 3way merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-06T11:46:07Z","receivedAt":"2015-06-06T11:46:07Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Even when a merge conflict occurs with am --3way, the index will be\nmodified with the results of any succesfully merged files (such as a new\nfile). These changes to the index will not be reverted with a\n\"git read-tree --reset -u HEAD HEAD\", as git read-tree will not be aware\nof how the current index differs from HEAD.\n\nTo fix this, we first reset any conflicting entries from the index. The\nresulting index will contain the results of successfully merged files.\nWe write the index to a tree, then use git read-tree -m to fast-forward\nthe \"index tree\" back to HEAD, thus undoing all the changes from the\nfailed merge.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n git-am.sh           |  7 ++++++-\n t/t4151-am-abort.sh | 11 +++++++++++\n 2 files changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 761befb..df3c8f4 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -69,6 +69,8 @@ then\n \tcmdline=\"$cmdline -3\"\n fi\n \n+empty_tree=4b825dc642cb6eb9a060e54bf8d69288fbee4904\n+\n sq () {\n \tgit rev-parse --sq-quote \"$@\"\n }\n@@ -502,7 +504,10 @@ then\n \t\t;;\n \tt,)\n \t\tgit rerere clear\n-\t\tgit read-tree --reset -u HEAD HEAD\n+\t\thead_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree) &&\n+\t\tgit read-tree --reset -u $head_tree $head_tree &&\n+\t\tindex_tree=$(git write-tree) &&\n+\t\tgit read-tree -m -u $index_tree $head_tree\n \t\torig_head=$(cat \"$GIT_DIR/ORIG_HEAD\")\n \t\tgit reset HEAD\n \t\tgit update-ref ORIG_HEAD $orig_head\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex 8d90634..e15acc8 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -63,6 +63,17 @@ do\n \n done\n \n+test_expect_success 'am -3 --skip removes otherfile-4' '\n+\tgit reset --hard initial &&\n+\ttest_must_fail git am -3 0003-*.patch &&\n+\ttest 3 -eq $(git ls-files -u | wc -l) &&\n+\ttest 4 = \"$(cat otherfile-4)\" &&\n+\tgit am --skip &&\n+\ttest_cmp_rev initial HEAD &&\n+\ttest -z $(git ls-files -u) &&\n+\ttest_path_is_missing otherfile-4\n+'\n+\n test_expect_success 'am --abort will keep the local commits intact' '\n \ttest_must_fail git am 0004-*.patch &&\n \ttest_commit unrelated &&\n-- \n2.1.4\n"},{"id":"263097","messageId":"1433591172-27077-3-git-send-email-pyokagan@gmail.com","threadId":"39542","inReplyTo":"1433591172-27077-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 2/6] am -3: support 3way merge on unborn branch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-06T11:46:08Z","receivedAt":"2015-06-06T11:46:08Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"While on an unborn branch, git am -3 will fail to do a threeway merge as\nit references HEAD as \"our tree\", but HEAD does not point to a valid\ntree.\n\nFix this by using an empty tree as \"our tree\" when we are on an unborn\nbranch.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n git-am.sh           | 3 ++-\n t/t4151-am-abort.sh | 9 +++++++++\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex df3c8f4..c847b58 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -179,7 +179,8 @@ It does not apply to blobs recorded in its index.\")\"\n     then\n \t    GIT_MERGE_VERBOSITY=0 && export GIT_MERGE_VERBOSITY\n     fi\n-    git-merge-recursive $orig_tree -- HEAD $his_tree || {\n+    our_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree)\n+    git-merge-recursive $orig_tree -- $our_tree $his_tree || {\n \t    git rerere $allow_rerere_autoupdate\n \t    die \"$(gettext \"Failed to merge in the changes.\")\"\n     }\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex e15acc8..b618ee0 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -83,4 +83,13 @@ test_expect_success 'am --abort will keep the local commits intact' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'am -3 stops on conflict on unborn branch' '\n+\tgit checkout -f --orphan orphan &&\n+\tgit reset &&\n+\trm -f otherfile-4 &&\n+\ttest_must_fail git am -3 0003-*.patch &&\n+\ttest 2 -eq $(git ls-files -u | wc -l) &&\n+\ttest 4 = \"$(cat otherfile-4)\"\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"263098","messageId":"1433591172-27077-4-git-send-email-pyokagan@gmail.com","threadId":"39542","inReplyTo":"1433591172-27077-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 3/6] am --skip: support skipping while on unborn branch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-06T11:46:09Z","receivedAt":"2015-06-06T11:46:09Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"When git am --skip is run, git am will copy HEAD's tree entries to the\nindex with \"git reset HEAD\". However, on an unborn branch, HEAD does not\npoint to a tree, so \"git reset HEAD\" will fail.\n\nFix this by treating HEAD as en empty tree when we are on an unborn\nbranch.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n git-am.sh           |  4 +---\n t/t4151-am-abort.sh | 10 ++++++++++\n 2 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex c847b58..67f4f25 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -509,9 +509,7 @@ then\n \t\tgit read-tree --reset -u $head_tree $head_tree &&\n \t\tindex_tree=$(git write-tree) &&\n \t\tgit read-tree -m -u $index_tree $head_tree\n-\t\torig_head=$(cat \"$GIT_DIR/ORIG_HEAD\")\n-\t\tgit reset HEAD\n-\t\tgit update-ref ORIG_HEAD $orig_head\n+\t\tgit read-tree $head_tree\n \t\t;;\n \t,t)\n \t\tif test -f \"$dotest/rebasing\"\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex b618ee0..ea4a49e 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -92,4 +92,14 @@ test_expect_success 'am -3 stops on conflict on unborn branch' '\n \ttest 4 = \"$(cat otherfile-4)\"\n '\n \n+test_expect_success 'am -3 --skip clears index on unborn branch' '\n+\ttest_path_is_dir .git/rebase-apply &&\n+\techo tmpfile >tmpfile &&\n+\tgit add tmpfile &&\n+\tgit am --skip &&\n+\ttest -z \"$(git ls-files)\" &&\n+\ttest_path_is_missing otherfile-4 &&\n+\ttest_path_is_missing tmpfile\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"263100","messageId":"1433591172-27077-5-git-send-email-pyokagan@gmail.com","threadId":"39542","inReplyTo":"1433591172-27077-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 4/6] am --abort: revert changes introduced by failed 3way merge","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-06T11:46:10Z","receivedAt":"2015-06-06T11:46:10Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Even when a merge conflict occurs with am --3way, the index will be\nmodified with the results of any successfully merged files. These\nchanges to the index will not be reverted with a\n\"git read-tree --reset -u HEAD ORIG_HEAD\", as git read-tree will not be\naware of how the current index differs from HEAD or ORIG_HEAD.\n\nTo fix this, we first reset any conflicting entries in the index. The\nresulting index will contain the results of successfully merged files\nintroduced by the failed merge. We write this index to a tree, and then\nuse git read-tree to fast-forward this \"index tree\" back to ORIG_HEAD,\nthus undoing all the changes from the failed merge.\n\nWhen we are on an unborn branch, HEAD and ORIG_HEAD will not point to\nvalid trees. In this case, use an empty tree.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n git-am.sh           |  6 +++++-\n t/t4151-am-abort.sh | 23 +++++++++++++++++++++++\n 2 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 67f4f25..e4fe3ed 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -519,7 +519,11 @@ then\n \t\tgit rerere clear\n \t\tif safe_to_abort\n \t\tthen\n-\t\t\tgit read-tree --reset -u HEAD ORIG_HEAD\n+\t\t\thead_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree) &&\n+\t\t\tgit read-tree --reset -u $head_tree $head_tree &&\n+\t\t\tindex_tree=$(git write-tree) &&\n+\t\t\torig_head=$(git rev-parse --verify -q ORIG_HEAD || echo $empty_tree) &&\n+\t\t\tgit read-tree -m -u $index_tree $orig_head\n \t\t\tgit reset ORIG_HEAD\n \t\tfi\n \t\trm -fr \"$dotest\"\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex ea4a49e..2366042 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -70,6 +70,17 @@ test_expect_success 'am -3 --skip removes otherfile-4' '\n \ttest 4 = \"$(cat otherfile-4)\" &&\n \tgit am --skip &&\n \ttest_cmp_rev initial HEAD &&\n+\ttest -z \"$(git ls-files -u)\" &&\n+\ttest_path_is_missing otherfile-4\n+'\n+\n+test_expect_success 'am -3 --abort removes otherfile-4' '\n+\tgit reset --hard initial &&\n+\ttest_must_fail git am -3 0003-*.patch &&\n+\ttest 3 -eq $(git ls-files -u | wc -l) &&\n+\ttest 4 = \"$(cat otherfile-4)\" &&\n+\tgit am --abort &&\n+\ttest_cmp_rev initial HEAD &&\n \ttest -z $(git ls-files -u) &&\n \ttest_path_is_missing otherfile-4\n '\n@@ -102,4 +113,16 @@ test_expect_success 'am -3 --skip clears index on unborn branch' '\n \ttest_path_is_missing tmpfile\n '\n \n+test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' '\n+\tgit checkout -f --orphan orphan &&\n+\tgit reset &&\n+\trm -f otherfile-4 file-1 &&\n+\ttest_must_fail git am -3 0003-*.patch &&\n+\ttest 2 -eq $(git ls-files -u | wc -l) &&\n+\ttest 4 = \"$(cat otherfile-4)\" &&\n+\tgit am --abort &&\n+\ttest -z \"$(git ls-files -u)\" &&\n+\ttest_path_is_missing otherfile-4\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"263101","messageId":"1433591172-27077-6-git-send-email-pyokagan@gmail.com","threadId":"39542","inReplyTo":"1433591172-27077-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 5/6] am --abort: support aborting to unborn branch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-06T11:46:11Z","receivedAt":"2015-06-06T11:46:11Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"When git-am is first run on an unborn branch, no ORIG_HEAD is created.\nAs such, any applied commits will remain even after a git am --abort.\n\nTo be consistent with the behavior of git am --abort when it is not run\nfrom an unborn branch, we empty the index, and then destroy the branch\npointed to by HEAD if there is no ORIG_HEAD.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n git-am.sh           |  9 ++++++++-\n t/t4151-am-abort.sh | 17 +++++++++++++++++\n 2 files changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex e4fe3ed..95f90a0 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -524,7 +524,14 @@ then\n \t\t\tindex_tree=$(git write-tree) &&\n \t\t\torig_head=$(git rev-parse --verify -q ORIG_HEAD || echo $empty_tree) &&\n \t\t\tgit read-tree -m -u $index_tree $orig_head\n-\t\t\tgit reset ORIG_HEAD\n+\t\t\tif git rev-parse --verify -q ORIG_HEAD >/dev/null 2>&1\n+\t\t\tthen\n+\t\t\t\tgit reset ORIG_HEAD\n+\t\t\telse\n+\t\t\t\tgit read-tree $empty_tree\n+\t\t\t\tcurr_branch=$(git symbolic-ref HEAD 2>/dev/null) &&\n+\t\t\t\tgit update-ref -d $curr_branch\n+\t\t\tfi\n \t\tfi\n \t\trm -fr \"$dotest\"\n \t\texit ;;\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex 2366042..b2a7fc5 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -14,6 +14,7 @@ test_expect_success setup '\n \tgit add file-1 file-2 &&\n \tgit commit -m initial &&\n \tgit tag initial &&\n+\tgit format-patch --stdout --root initial >initial.patch &&\n \tfor i in 2 3 4 5 6\n \tdo\n \t\techo $i >>file-1 &&\n@@ -125,4 +126,20 @@ test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' '\n \ttest_path_is_missing otherfile-4\n '\n \n+test_expect_success 'am -3 --abort on unborn branch removes applied commits' '\n+\tgit checkout -f --orphan orphan &&\n+\tgit reset &&\n+\trm -f otherfile-4 otherfile-2 file-1 file-2 &&\n+\ttest_must_fail git am -3 initial.patch 0003-*.patch &&\n+\ttest 3 -eq $(git ls-files -u | wc -l) &&\n+\ttest 4 = \"$(cat otherfile-4)\" &&\n+\tgit am --abort &&\n+\ttest -z \"$(git ls-files -u)\" &&\n+\ttest_path_is_missing otherfile-4 &&\n+\ttest_path_is_missing file-1 &&\n+\ttest_path_is_missing file-2 &&\n+\ttest 0 -eq $(git log --oneline 2>/dev/null | wc -l) &&\n+\ttest refs/heads/orphan = \"$(git symbolic-ref HEAD)\"\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"263099","messageId":"1433591172-27077-7-git-send-email-pyokagan@gmail.com","threadId":"39542","inReplyTo":"1433591172-27077-1-git-send-email-pyokagan@gmail.com","subject":"[PATCH 6/6] am --abort: keep unrelated commits on unborn branch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-06T11:46:12Z","receivedAt":"2015-06-06T11:46:12Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure\nand warn, 2010-12-21), git-am would refuse to rewind HEAD if commits\nwere made since the last git-am failure. This check was implemented in\nsafe_to_abort(), which checked to see if HEAD's hash matched the\nabort-safety file.\n\nHowever, this check was skipped if the abort-safety file was empty,\nwhich can happen if git-am failed while on an unborn branch. As such, if\nany commits were made since then, they would be discarded. Fix this by\ncarrying on the abort safety check even if the abort-safety file is\nempty.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n git-am.sh           |  2 +-\n t/t4151-am-abort.sh | 11 +++++++++++\n 2 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 95f90a0..4324bb1 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -87,7 +87,7 @@ safe_to_abort () {\n \t\treturn 1\n \tfi\n \n-\tif ! test -s \"$dotest/abort-safety\"\n+\tif ! test -f \"$dotest/abort-safety\"\n \tthen\n \t\treturn 0\n \tfi\ndiff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\nindex b2a7fc5..833e7b2 100755\n--- a/t/t4151-am-abort.sh\n+++ b/t/t4151-am-abort.sh\n@@ -142,4 +142,15 @@ test_expect_success 'am -3 --abort on unborn branch removes applied commits' '\n \ttest refs/heads/orphan = \"$(git symbolic-ref HEAD)\"\n '\n \n+test_expect_success 'am --abort on unborn branch will keep local commits intact' '\n+\tgit checkout -f --orphan orphan &&\n+\tgit reset &&\n+\ttest_must_fail git am 0004-*.patch &&\n+\ttest_commit unrelated2 &&\n+\tgit rev-parse HEAD >expect &&\n+\tgit am --abort &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"263257","messageId":"CAGZ79kZj1vFj9VFAzJdv-nE=Lqcbz2BXXrao1KPdrYw_yYeyhA@mail.gmail.com","threadId":"39542","inReplyTo":"1433591172-27077-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 0/6] am --skip/--abort: improve index/worktree cleanup","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-08T17:04:42Z","receivedAt":"2015-06-08T17:04:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Jun 6, 2015 at 4:46 AM, Paul Tan <pyokagan@gmail.com> wrote:\n> Currently git-am attempts to clean up the index/worktree when skipping or\n> aborting, but does not do it very well:\n>\n> * While it discards conflicted index entries, it does not remove any other\n>   modifications made to the index due to a previous threeway merge.\n>\n> * It expects HEAD/ORIG_HEAD to exist, and thus does not clean up the index when\n>   on an unborn branch.\n>\n> This patch series addresses the above two general problems by making the calls\n> to git-read-tree aware of the differences between our current index and\n> HEAD/ORIG_HEAD, and by explictly defining what happens when we are on an unborn\n> branch.\n>\n>\n> Paul Tan (6):\n>   am --skip: revert changes introduced by failed 3way merge\n>   am -3: support 3way merge on unborn branch\n>   am --skip: support skipping while on unborn branch\n>   am --abort: revert changes introduced by failed 3way merge\n>   am --abort: support aborting to unborn branch\n>   am --abort: keep unrelated commits on unborn branch\n\nThe whole series looks good to me.\n\nThanks,\nStefan\n\n>\n>  git-am.sh           | 31 ++++++++++++++------\n>  t/t4151-am-abort.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 104 insertions(+), 8 deletions(-)\n>\n> --\n> 2.1.4\n>\n"},{"id":"263297","messageId":"xmqqd2169d3q.fsf@gitster.dls.corp.google.com","threadId":"39542","inReplyTo":"1433591172-27077-5-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 4/6] am --abort: revert changes introduced by failed 3way merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-08T20:09:13Z","receivedAt":"2015-06-08T20:09:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Even when a merge conflict occurs with am --3way, the index will be\n> modified with the results of any successfully merged files. These\n> changes to the index will not be reverted with a\n> \"git read-tree --reset -u HEAD ORIG_HEAD\", as git read-tree will not be\n> aware of how the current index differs from HEAD or ORIG_HEAD.\n>\n> To fix this, we first reset any conflicting entries in the index. The\n> resulting index will contain the results of successfully merged files\n> introduced by the failed merge. We write this index to a tree, and then\n> use git read-tree to fast-forward this \"index tree\" back to ORIG_HEAD,\n> thus undoing all the changes from the failed merge.\n>\n> When we are on an unborn branch, HEAD and ORIG_HEAD will not point to\n> valid trees. In this case, use an empty tree.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>  git-am.sh           |  6 +++++-\n>  t/t4151-am-abort.sh | 23 +++++++++++++++++++++++\n>  2 files changed, 28 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-am.sh b/git-am.sh\n> index 67f4f25..e4fe3ed 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -519,7 +519,11 @@ then\n>  \t\tgit rerere clear\n>  \t\tif safe_to_abort\n>  \t\tthen\n> -\t\t\tgit read-tree --reset -u HEAD ORIG_HEAD\n> +\t\t\thead_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree) &&\n> +\t\t\tgit read-tree --reset -u $head_tree $head_tree &&\n> +\t\t\tindex_tree=$(git write-tree) &&\n> +\t\t\torig_head=$(git rev-parse --verify -q ORIG_HEAD || echo $empty_tree) &&\n> +\t\t\tgit read-tree -m -u $index_tree $orig_head\n>  \t\t\tgit reset ORIG_HEAD\n\nWhat would the last \"reset\" do when ORIG_HEAD is invalid?  In other\nwords, does it make sense to worry about the case where ORIG_HEAD\ndoes not exist?  At which point in the flow does it get written using\nwhat information?\n\n>  \t\tfi\n>  \t\trm -fr \"$dotest\"\n> diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\n> index ea4a49e..2366042 100755\n> --- a/t/t4151-am-abort.sh\n> +++ b/t/t4151-am-abort.sh\n> @@ -70,6 +70,17 @@ test_expect_success 'am -3 --skip removes otherfile-4' '\n>  \ttest 4 = \"$(cat otherfile-4)\" &&\n>  \tgit am --skip &&\n>  \ttest_cmp_rev initial HEAD &&\n> +\ttest -z \"$(git ls-files -u)\" &&\n> +\ttest_path_is_missing otherfile-4\n> +'\n> +\n> +test_expect_success 'am -3 --abort removes otherfile-4' '\n> +\tgit reset --hard initial &&\n> +\ttest_must_fail git am -3 0003-*.patch &&\n> +\ttest 3 -eq $(git ls-files -u | wc -l) &&\n> +\ttest 4 = \"$(cat otherfile-4)\" &&\n> +\tgit am --abort &&\n> +\ttest_cmp_rev initial HEAD &&\n>  \ttest -z $(git ls-files -u) &&\n>  \ttest_path_is_missing otherfile-4\n>  '\n> @@ -102,4 +113,16 @@ test_expect_success 'am -3 --skip clears index on unborn branch' '\n>  \ttest_path_is_missing tmpfile\n>  '\n>  \n> +test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' '\n> +\tgit checkout -f --orphan orphan &&\n> +\tgit reset &&\n> +\trm -f otherfile-4 file-1 &&\n> +\ttest_must_fail git am -3 0003-*.patch &&\n> +\ttest 2 -eq $(git ls-files -u | wc -l) &&\n> +\ttest 4 = \"$(cat otherfile-4)\" &&\n> +\tgit am --abort &&\n> +\ttest -z \"$(git ls-files -u)\" &&\n> +\ttest_path_is_missing otherfile-4\n> +'\n> +\n>  test_done\n"},{"id":"263298","messageId":"xmqq8ubu9d1a.fsf@gitster.dls.corp.google.com","threadId":"39542","inReplyTo":"1433591172-27077-6-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 5/6] am --abort: support aborting to unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-08T20:10:41Z","receivedAt":"2015-06-08T20:10:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> When git-am is first run on an unborn branch, no ORIG_HEAD is created.\n> As such, any applied commits will remain even after a git am --abort.\n\nI think this answered my question on 4/6; that may indicate that\nthese two are either done in a wrong order or perhaps should be a\nsingle change.\n\n\n\n>\n> To be consistent with the behavior of git am --abort when it is not run\n> from an unborn branch, we empty the index, and then destroy the branch\n> pointed to by HEAD if there is no ORIG_HEAD.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>  git-am.sh           |  9 ++++++++-\n>  t/t4151-am-abort.sh | 17 +++++++++++++++++\n>  2 files changed, 25 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-am.sh b/git-am.sh\n> index e4fe3ed..95f90a0 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -524,7 +524,14 @@ then\n>  \t\t\tindex_tree=$(git write-tree) &&\n>  \t\t\torig_head=$(git rev-parse --verify -q ORIG_HEAD || echo $empty_tree) &&\n>  \t\t\tgit read-tree -m -u $index_tree $orig_head\n> -\t\t\tgit reset ORIG_HEAD\n> +\t\t\tif git rev-parse --verify -q ORIG_HEAD >/dev/null 2>&1\n> +\t\t\tthen\n> +\t\t\t\tgit reset ORIG_HEAD\n> +\t\t\telse\n> +\t\t\t\tgit read-tree $empty_tree\n> +\t\t\t\tcurr_branch=$(git symbolic-ref HEAD 2>/dev/null) &&\n> +\t\t\t\tgit update-ref -d $curr_branch\n> +\t\t\tfi\n>  \t\tfi\n>  \t\trm -fr \"$dotest\"\n>  \t\texit ;;\n> diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\n> index 2366042..b2a7fc5 100755\n> --- a/t/t4151-am-abort.sh\n> +++ b/t/t4151-am-abort.sh\n> @@ -14,6 +14,7 @@ test_expect_success setup '\n>  \tgit add file-1 file-2 &&\n>  \tgit commit -m initial &&\n>  \tgit tag initial &&\n> +\tgit format-patch --stdout --root initial >initial.patch &&\n>  \tfor i in 2 3 4 5 6\n>  \tdo\n>  \t\techo $i >>file-1 &&\n> @@ -125,4 +126,20 @@ test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' '\n>  \ttest_path_is_missing otherfile-4\n>  '\n>  \n> +test_expect_success 'am -3 --abort on unborn branch removes applied commits' '\n> +\tgit checkout -f --orphan orphan &&\n> +\tgit reset &&\n> +\trm -f otherfile-4 otherfile-2 file-1 file-2 &&\n> +\ttest_must_fail git am -3 initial.patch 0003-*.patch &&\n> +\ttest 3 -eq $(git ls-files -u | wc -l) &&\n> +\ttest 4 = \"$(cat otherfile-4)\" &&\n> +\tgit am --abort &&\n> +\ttest -z \"$(git ls-files -u)\" &&\n> +\ttest_path_is_missing otherfile-4 &&\n> +\ttest_path_is_missing file-1 &&\n> +\ttest_path_is_missing file-2 &&\n> +\ttest 0 -eq $(git log --oneline 2>/dev/null | wc -l) &&\n> +\ttest refs/heads/orphan = \"$(git symbolic-ref HEAD)\"\n> +'\n> +\n>  test_done\n"},{"id":"263299","messageId":"xmqq4mmi9cwb.fsf@gitster.dls.corp.google.com","threadId":"39542","inReplyTo":"1433591172-27077-7-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH 6/6] am --abort: keep unrelated commits on unborn branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-08T20:13:40Z","receivedAt":"2015-06-08T20:13:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure\n> and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits\n> were made since the last git-am failure. This check was implemented in\n> safe_to_abort(), which checked to see if HEAD's hash matched the\n> abort-safety file.\n>\n> However, this check was skipped if the abort-safety file was empty,\n> which can happen if git-am failed while on an unborn branch.\n\nShouldn't we then be checking that the HEAD is still unborn if this\nfile is found and says that there was no history in the beginning,\nin order to give the \"am on top of unborn\" workflow the same degree\nof safety?  Or is the fact that the file is empty sufficient not to\nmatch any valid commit name that is 40-hex?\n\n> As such, if\n> any commits were made since then, they would be discarded. Fix this by\n> carrying on the abort safety check even if the abort-safety file is\n> empty.\n>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n>  git-am.sh           |  2 +-\n>  t/t4151-am-abort.sh | 11 +++++++++++\n>  2 files changed, 12 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-am.sh b/git-am.sh\n> index 95f90a0..4324bb1 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -87,7 +87,7 @@ safe_to_abort () {\n>  \t\treturn 1\n>  \tfi\n>  \n> -\tif ! test -s \"$dotest/abort-safety\"\n> +\tif ! test -f \"$dotest/abort-safety\"\n>  \tthen\n>  \t\treturn 0\n>  \tfi\n> diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh\n> index b2a7fc5..833e7b2 100755\n> --- a/t/t4151-am-abort.sh\n> +++ b/t/t4151-am-abort.sh\n> @@ -142,4 +142,15 @@ test_expect_success 'am -3 --abort on unborn branch removes applied commits' '\n>  \ttest refs/heads/orphan = \"$(git symbolic-ref HEAD)\"\n>  '\n>  \n> +test_expect_success 'am --abort on unborn branch will keep local commits intact' '\n> +\tgit checkout -f --orphan orphan &&\n> +\tgit reset &&\n> +\ttest_must_fail git am 0004-*.patch &&\n> +\ttest_commit unrelated2 &&\n> +\tgit rev-parse HEAD >expect &&\n> +\tgit am --abort &&\n> +\tgit rev-parse HEAD >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n"},{"id":"263331","messageId":"CACRoPnT4CcJxQtTCzbOVRhHmN4MvzBx1S6x53CRuwFF9e+N4SQ@mail.gmail.com","threadId":"39542","inReplyTo":"xmqq4mmi9cwb.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 6/6] am --abort: keep unrelated commits on unborn branch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-09T08:54:54Z","receivedAt":"2015-06-09T08:54:54Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Tue, Jun 9, 2015 at 4:13 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure\n>> and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits\n>> were made since the last git-am failure. This check was implemented in\n>> safe_to_abort(), which checked to see if HEAD's hash matched the\n>> abort-safety file.\n>>\n>> However, this check was skipped if the abort-safety file was empty,\n>> which can happen if git-am failed while on an unborn branch.\n>\n> Shouldn't we then be checking that the HEAD is still unborn if this\n> file is found and says that there was no history in the beginning,\n> in order to give the \"am on top of unborn\" workflow the same degree\n> of safety?\n\nWe do already check to see if the HEAD is still unborn:\n\n        abort_safety=$(cat \"$dotest/abort-safety\")\n        if test \"z$(git rev-parse --verify -q HEAD)\" = \"z$abort_safety\"\n        then\n            return 0\n        fi\n        gettextln \"You seem to have moved HEAD since the last 'am' failure.\n    Not rewinding to ORIG_HEAD\" >&2\n\nIf HEAD is unborn, then git rev-parse will not print anything, so we\nwould be comparing an empty string to an empty string.\n\nThanks,\nPaul\n"},{"id":"263333","messageId":"CACRoPnQft7p1QrVwYaKcBnyoAkZLm0uOaf84E+jrdDUb8myweg@mail.gmail.com","threadId":"39542","inReplyTo":"xmqq8ubu9d1a.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 5/6] am --abort: support aborting to unborn branch","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-06-09T09:18:14Z","receivedAt":"2015-06-09T09:18:14Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Tue, Jun 9, 2015 at 4:10 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Paul Tan <pyokagan@gmail.com> writes:\n>\n>> When git-am is first run on an unborn branch, no ORIG_HEAD is created.\n>> As such, any applied commits will remain even after a git am --abort.\n>\n> I think this answered my question on 4/6; that may indicate that\n> these two are either done in a wrong order or perhaps should be a\n> single change.\n\nAh right, patch 4/6 was too sneaky in that it tried to do the \"support\nunborn branch\" thing as well, which should only be handled in this\npatch.\n\nI still think the patches should be separate though since they are\nconceptually different. 4/6 modifies the \"index clean up\" function,\nwhile 5/6 modifies the \"reset HEAD\" function.\n\nThanks,\nPaul\n"}]}