{"thread":{"id":"34147","subject":"[PATCH v3 0/2] Fix am with stray $dotest directory","startedAt":"2013-06-15T12:43:10Z","lastAt":"2013-06-18T14:44:40Z","messageCount":8,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"220937","messageId":"1371300192-6222-1-git-send-email-artagnon@gmail.com","threadId":"34147","inReplyTo":null,"subject":"[PATCH v3 0/2] Fix am with stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-15T12:43:10Z","receivedAt":"2013-06-15T12:43:10Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The test in [1/2] was too loose in the previous iteration: guard it\nwith \"test -z $rebasing\". Also fix a couple of minor problems pointed\nout by Junio (extra indentation, $-unescaped).\n\nThanks.\n\nRamkumar Ramachandra (2):\n am: handle stray $dotest directory\n t/am: use test_path_is_missing() where appropriate\n\n git-am.sh | 17 +++++++++++++++++\n t/t4150-am.sh | 40 +++++++++++++++++++++++-----------------\n 2 files changed, 40 insertions(+), 17 deletions(-)\n\n-- \n1.8.3.1.383.g8881048.dirty\n"},{"id":"220938","messageId":"1371300192-6222-2-git-send-email-artagnon@gmail.com","threadId":"34147","inReplyTo":"1371300192-6222-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v3 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-15T12:43:11Z","receivedAt":"2013-06-15T12:43:11Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The following bug has been observed:\n\n $ git am # no input file\n ^C\n $ git am --abort\n Resolve operation not in progress, we are not resuming.\n\nThis happens because the following test fails:\n\n test -d \"$dotest\" && test -f \"$dotest/last\" && test -f \"$dotest/next\"\n\nand the codepath for an \"am in-progress\" is not executed. It falls back\nto the codepath that treats this as a \"fresh execution\". Before\nrr/rebase-autostash, this condition was\n\n test -d \"$dotest\"\n\nIt would incorrectly execute the \"normal\" am --abort codepath:\n\n git read-tree --reset -u HEAD ORIG_HEAD\n git reset ORIG_HEAD\n\nby incorrectly assuming that an am is \"in progress\" (i.e. ORIG_HEAD\netc. was written during the previous execution).\n\nNotice that\n\n $ git am\n ^C\n\nexecutes nothing of significance, is equivalent to\n\n $ mkdir .git/rebase-apply\n\nTherefore, the correct solution is to treat .git/rebase-apply as a\n\"stray directory\" and remove it on --abort in the fresh-execution\ncodepath. Also ensure that we're not called with --rebasing from\ngit-rebase--am.sh; in that case, it is the responsibility of the caller\nto handle and stray directories.\n\nWhile at it, tell the user to run \"git am --abort\" to get rid of the\nstray $dotest directory, if she attempts anything else.\n\nReported-by: Junio C Hamano \nSigned-off-by: Ramkumar Ramachandra \n---\n git-am.sh | 17 +++++++++++++++++\n t/t4150-am.sh | 6 ++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 1cf3d1d..91a2bcc 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -506,6 +506,23 @@ then\n \tesac\n \trm -f \"$dotest/dirtyindex\"\n else\n+\t# Possible stray $dotest directory in the independent-run\n+\t# case; in the --rebasing case, it is upto the caller\n+\t# (git-rebase--am) to take care of stray directories.\n+\tif test -d \"$dotest\" && test -z \"$rebasing\"\n+\tthen\n+\tcase \"$skip,$resolved,$abort\" in\n+\t,,t)\n+\t\trm -fr \"$dotest\"\n+\t\texit 0\n+\t\t;;\n+\t*)\n+\t\tdie \"$(eval_gettext \"Stray \\$dotest directory found.\n+Use \\\"git am --abort\\\" to remove it.\")\"\n+\t\t;;\n+\tesac\n+\tfi\n+\n \t# Make sure we are not given --skip, --resolved, nor --abort\n \ttest \"$skip$resolved$abort\" = \"\" ||\n \t\tdie \"$(gettext \"Resolve operation not in progress, we are not resuming.\")\"\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 12f6b02..6c2cc3e 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -363,6 +363,12 @@ test_expect_success 'am --skip works' '\n \ttest_cmp expected another\n '\n \n+test_expect_success 'am --abort removes a stray directory' '\n+\tmkdir .git/rebase-apply &&\n+\tgit am --abort &&\n+\ttest_path_is_missing .git/rebase-apply\n+'\n+\n test_expect_success 'am --resolved works' '\n \techo goodbye >expected &&\n \trm -fr .git/rebase-apply &&\n-- \n1.8.3.1.383.g8881048.dirty\n"},{"id":"220939","messageId":"1371300192-6222-3-git-send-email-artagnon@gmail.com","threadId":"34147","inReplyTo":"1371300192-6222-1-git-send-email-artagnon@gmail.com","subject":"[PATCH v3 2/2] t/am: use test_path_is_missing() where appropriate","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-15T12:43:12Z","receivedAt":"2013-06-15T12:43:12Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Replace instances of ! test -d with test_path_is_missing.\n\nSigned-off-by: Ramkumar Ramachandra \n---\n t/t4150-am.sh | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 6c2cc3e..5edb79a 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -147,7 +147,7 @@ test_expect_success 'am applies patch correctly' '\n \tgit checkout first &&\n \ttest_tick &&\n \tgit am actual &&\n \tgrep \"Re: Re: Re: \\[PATCH 1/5 v2\\] \\[foo\\] third\" actual\n '\n@@ -268,7 +268,7 @@ test_expect_success 'am --keep-non-patch really keeps the non-patch part' '\n \tgit reset --hard &&\n \tgit checkout HEAD^ &&\n \tgit am --keep-non-patch patch4 &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \tgit cat-file commit HEAD >actual &&\n \tgrep \"^\\[foo\\] third\" actual\n '\n@@ -283,7 +283,7 @@ test_expect_success 'am -3 falls back to 3-way merge' '\n \ttest_tick &&\n \tgit commit -m \"copied stuff\" &&\n \tgit am -3 lorem-move.patch &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \tgit diff --exit-code lorem\n '\n \n@@ -297,7 +297,7 @@ test_expect_success 'am -3 -p0 can read --no-prefix patch' '\n \ttest_tick &&\n \tgit commit -m \"copied stuff\" &&\n \tgit am -3 -p0 lorem-zero.patch &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \tgit diff --exit-code lorem\n '\n \n@@ -307,7 +307,7 @@ test_expect_success 'am can rename a file' '\n \tgit reset --hard &&\n \tgit checkout lorem^0 &&\n \tgit am rename.patch &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \tgit update-index --refresh &&\n \tgit diff --exit-code rename\n '\n@@ -318,7 +318,7 @@ test_expect_success 'am -3 can rename a file' '\n \tgit reset --hard &&\n \tgit checkout lorem^0 &&\n \tgit am -3 rename.patch &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \tgit update-index --refresh &&\n \tgit diff --exit-code rename\n '\n@@ -329,7 +329,7 @@ test_expect_success 'am -3 can rename a file after falling back to 3-way merge'\n \tgit reset --hard &&\n \tgit checkout lorem^0 &&\n \tgit am -3 rename-add.patch &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \tgit update-index --refresh &&\n \tgit diff --exit-code rename\n '\n@@ -358,7 +358,7 @@ test_expect_success 'am pauses on conflict' '\n test_expect_success 'am --skip works' '\n \techo goodbye >expected &&\n \tgit am --skip &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \tgit diff --exit-code lorem2^^ -- file &&\n \ttest_cmp expected another\n '\n@@ -379,7 +379,7 @@ test_expect_success 'am --resolved works' '\n \techo resolved >>file &&\n \tgit add file &&\n \tgit am --resolved &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \ttest_cmp expected another\n '\n \n@@ -388,7 +388,7 @@ test_expect_success 'am takes patches from a Pine mailbox' '\n \tgit reset --hard &&\n \tgit checkout first &&\n \tcat pine patch1 | git am &&\n-\t! test -d .git/rebase-apply &&\n+\ttest_path_is_missing .git/rebase-apply &&\n \tgit diff --exit-code master^..HEAD\n '\n \n@@ -397,7 +397,7 @@ test_expect_success 'am fails on mail without patch' '\n \tgit reset --hard &&\n \ttest_must_fail git am >failmail &&\n \ttest_must_fail git am writes:\n\n> else\n> +\t# Possible stray $dotest directory in the independent-run\n> +\t# case; in the --rebasing case, it is upto the caller\n> +\t# (git-rebase--am) to take care of stray directories.\n> +\tif test -d \"$dotest\" && test -z \"$rebasing\"\n\nThe $rebasing variable is set only when the command line of \"git am\"\nhas --rebasing, i.e. only when it is driven by \"git rebase\". So\nthis will not affect what happens to \"git am --abort\" that is run by\nthe end user. That sounds like a safe thing to do as far as \"am\" is\nconcerned.\n\nWill replace what has been queued on 'pu'. Thanks.\n"},{"id":"221079","messageId":"7vy5a8ei6u.fsf@alter.siamese.dyndns.org","threadId":"34147","inReplyTo":"7v38shjw01.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 1/2] am: handle stray $dotest directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-17T17:54:49Z","receivedAt":"2013-06-17T17:54:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Will replace what has been queued on 'pu'.\n\n... after fixing an indentation error, that is.\n"},{"id":"221152","messageId":"CALkWK0kq6+K4fotWq1Qz9BrqOG56NezJqnfH9Xrg1vh6AS9bnw@mail.gmail.com","threadId":"34147","inReplyTo":"7vy5a8ei6u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-18T07:05:39Z","receivedAt":"2013-06-18T07:05:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n>> Will replace what has been queued on 'pu'.\n>\n> ... after fixing an indentation error, that is.\n\nWhere did the error occur? I thought I taught my Emacs to always\nindent properly.\n"},{"id":"221226","messageId":"7va9mna3tm.fsf@alter.siamese.dyndns.org","threadId":"34147","inReplyTo":"CALkWK0kq6+K4fotWq1Qz9BrqOG56NezJqnfH9Xrg1vh6AS9bnw@mail.gmail.com","subject":"Re: [PATCH v3 1/2] am: handle stray $dotest directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-18T14:31:01Z","receivedAt":"2013-06-18T14:31:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Junio C Hamano wrote:\n>>> Will replace what has been queued on 'pu'.\n>>\n>> ... after fixing an indentation error, that is.\n>\n> Where did the error occur?\n\nI think you can compare what is in 'pu' and what you sent out\nwithout asking.\n"},{"id":"221232","messageId":"CALkWK0kQwyM4UD_mVkHKdVT-j+gxgg3quZN70jnE3tUO7m6bzA@mail.gmail.com","threadId":"34147","inReplyTo":"7va9mna3tm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-18T14:44:40Z","receivedAt":"2013-06-18T14:44:40Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n>> Where did the error occur?\n>\n> I think you can compare what is in 'pu' and what you sent out\n> without asking.\n\nAh, I sent out the series before I taught Emacs to indent properly.\n\nThanks.\n"}]}