{"thread":{"id":"34134","subject":"[PATCH v2 0/2] Fix am with stray $dotest directory","startedAt":"2013-06-14T07:47:36Z","lastAt":"2013-06-16T06:36:08Z","messageCount":16,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"220784","messageId":"1371196058-23948-1-git-send-email-artagnon@gmail.com","threadId":"34134","inReplyTo":null,"subject":"[PATCH v2 0/2] Fix am with stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T07:47:36Z","receivedAt":"2013-06-14T07:47:36Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"[1/2] is now equipped to handle any am invocation in the presence of a\nstray $dotest directory.\n\n[2/2] is a \"while we're there\".\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 | 14 ++++++++++++++\n t/t4150-am.sh | 40 +++++++++++++++++++++++-----------------\n 2 files changed, 37 insertions(+), 17 deletions(-)\n\n-- \n1.8.3.1.380.g67e7d64.dirty\n"},{"id":"220785","messageId":"1371196058-23948-2-git-send-email-artagnon@gmail.com","threadId":"34134","inReplyTo":"1371196058-23948-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T07:47:37Z","receivedAt":"2013-06-14T07:47:37Z","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.\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 | 14 ++++++++++++++\n t/t4150-am.sh | 6 ++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 1cf3d1d..37ee18b 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -506,6 +506,20 @@ then\n \tesac\n \trm -f \"$dotest/dirtyindex\"\n else\n+\t# Possible stray $dotest directory\n+\tif test -d \"$dotest\"; then\n+\tcase \"$skip,$resolved,$abort\" in\n+\t\t,,t)\n+\t\t\trm -fr \"$dotest\"\n+\t\t\texit 0\n+\t\t\t;;\n+\t\t*)\n+\t\t\tdie \"$(eval_gettext \"Stray $dotest directory found.\n+Use \\\"git am --abort\\\" to remove it.\")\"\n+\t\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.380.g67e7d64.dirty\n"},{"id":"220786","messageId":"1371196058-23948-3-git-send-email-artagnon@gmail.com","threadId":"34134","inReplyTo":"1371196058-23948-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/2] t/am: use test_path_is_missing() where appropriate","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T07:47:38Z","receivedAt":"2013-06-14T07:47:38Z","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> 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>\n> This happens because the following test fails:\n>\n> test -d \"$dotest\" && test -f \"$dotest/last\" && test -f \"$dotest/next\"\n>\n> and the codepath for an \"am in-progress\" is not executed. It falls back\n> to the codepath that treats this as a \"fresh execution\". Before\n> rr/rebase-autostash, this condition was\n>\n> test -d \"$dotest\"\n>\n> It would incorrectly execute the \"normal\" am --abort codepath:\n>\n> git read-tree --reset -u HEAD ORIG_HEAD\n> git reset ORIG_HEAD\n>\n> by incorrectly assuming that an am is \"in progress\" (i.e. ORIG_HEAD\n> etc. was written during the previous execution).\n>\n> Notice that\n>\n> $ git am\n> ^C\n>\n> executes nothing of significance, is equivalent to\n>\n> $ mkdir .git/rebase-apply\n>\n> Therefore, the correct solution is to treat .git/rebase-apply as a\n> \"stray directory\" and remove it on --abort in the fresh-execution\n> codepath.\n>\n> While at it, tell the user to run \"git am --abort\" to get rid of the\n> stray $dotest directory, if she attempts anything else.\n>\n> Reported-by: Junio C Hamano \n> Signed-off-by: Ramkumar Ramachandra \n> ---\n> git-am.sh | 14 ++++++++++++++\n> t/t4150-am.sh | 6 ++++++\n> 2 files changed, 20 insertions(+)\n>\n> diff --git a/git-am.sh b/git-am.sh\n> index 1cf3d1d..37ee18b 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -506,6 +506,20 @@ then\n> \tesac\n> \trm -f \"$dotest/dirtyindex\"\n> else\n> +\t# Possible stray $dotest directory\n> +\tif test -d \"$dotest\"; then\n> +\tcase \"$skip,$resolved,$abort\" in\n> +\t\t,,t)\n> +\t\t\trm -fr \"$dotest\"\n> +\t\t\texit 0\n> +\t\t\t;;\n> +\t\t*)\n> +\t\t\tdie \"$(eval_gettext \"Stray $dotest directory found.\n> +Use \\\"git am --abort\\\" to remove it.\")\"\n> +\t\t\t;;\n> +\tesac\n\nThese two case arms are indented one level too deep (will locally\ntouch up).\n\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.\")\"\n> diff --git a/t/t4150-am.sh b/t/t4150-am.sh\n> index 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"},{"id":"220872","messageId":"7vhah0raps.fsf@alter.siamese.dyndns.org","threadId":"34134","inReplyTo":"1371196058-23948-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-14T15:08:47Z","receivedAt":"2013-06-14T15:08:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> +\t\t\tdie \"$(eval_gettext \"Stray $dotest directory found.\n> +Use \\\"git am --abort\\\" to remove it.\")\"\n\n$dotest, or \\$dotest?\n"},{"id":"220875","messageId":"CALkWK0mF7s7Zfp4VTekT9AhHNhd80Jj3GT8SaKVs4gAATACsBA@mail.gmail.com","threadId":"34134","inReplyTo":"7vli6crau5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T15:11:12Z","receivedAt":"2013-06-14T15:11:12Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> These two case arms are indented one level too deep (will locally\n> touch up).\n\nThanks. Can you tell me how to get shell-script-mode to indent the\ncase statement properly? (I used the default indentation)\n"},{"id":"220876","messageId":"CALkWK0m6r4yRfCLb3dejdY+1KJgdOHPfrvPXbftNucApXFx+0A@mail.gmail.com","threadId":"34134","inReplyTo":"7vhah0raps.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T15:11:59Z","receivedAt":"2013-06-14T15:11:59Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> $dotest, or \\$dotest?\n\nWorks fine for me like this. Why do we escape the dollar in the other strings?\n"},{"id":"220878","messageId":"7v61xgraba.fsf@alter.siamese.dyndns.org","threadId":"34134","inReplyTo":"7vli6crau5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-14T15:17:29Z","receivedAt":"2013-06-14T15:17:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> +\t# Possible stray $dotest directory\n>> +\tif test -d \"$dotest\"; then\n>> +\tcase \"$skip,$resolved,$abort\" in\n>> +\t\t,,t)\n>> +\t\t\trm -fr \"$dotest\"\n>> +\t\t\texit 0\n>> +\t\t\t;;\n>> +\t\t*)\n>> +\t\t\tdie \"$(eval_gettext \"Stray $dotest directory found.\n>> +Use \\\"git am --abort\\\" to remove it.\")\"\n>> +\t\t\t;;\n>> +\tesac\n>\n> These two case arms are indented one level too deep (will locally\n> touch up).\n\nAnd then the message triggers at the second test in t3420 when\napplied on top of 587947750bd7 (rebase: implement --[no-]autostash\nand rebase.autostash, 2013-05-12) or 45acb7592825 (Merge branch\n'rr/rebase-autostash', 2013-06-11).\n"},{"id":"220879","messageId":"7v1u84ra77.fsf@alter.siamese.dyndns.org","threadId":"34134","inReplyTo":"CALkWK0m6r4yRfCLb3dejdY+1KJgdOHPfrvPXbftNucApXFx+0A@mail.gmail.com","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-14T15:19:56Z","receivedAt":"2013-06-14T15:19:56Z","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>> $dotest, or \\$dotest?\n>\n> Works fine for me like this. Why do we escape the dollar in the other strings?\n\nThe reason would become clear once you think what string you are\nfeeding eval_gettext with if you do not escape. The translators\ntranslate a fixed string (possibly with placeholders) to a fixed\ntranslated string (possibly with placeholders).\n\n\teval_gettext \"Stray $dotest directory found. ...\"\n\nwould allow the shell to expand $dotest before eval_gettext sees it,\nwhich would mean the string is no longer a constant.\n"},{"id":"220885","messageId":"CALkWK0m6iiDAmbxG9wrpgxJ3=nM816U7RM519Mu62qSgDwJbaQ@mail.gmail.com","threadId":"34134","inReplyTo":"7v61xgraba.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T16:13:21Z","receivedAt":"2013-06-14T16:13:21Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> And then the message triggers at the second test in t3420 when\n> applied on top of 587947750bd7 (rebase: implement --[no-]autostash\n> and rebase.autostash, 2013-05-12) or 45acb7592825 (Merge branch\n> 'rr/rebase-autostash', 2013-06-11).\n\nWhat was triggered? (I didn't understand what you said)\n"},{"id":"220886","messageId":"CALkWK0n+KSrehL3tZ557c2hfKna6p_jVGL+a2az5V+h4=DstJQ@mail.gmail.com","threadId":"34134","inReplyTo":"7v1u84ra77.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-14T16:15:24Z","receivedAt":"2013-06-14T16:15:24Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> The reason would become clear once you think what string you are\n> feeding eval_gettext with if you do not escape. The translators\n> translate a fixed string (possibly with placeholders) to a fixed\n> translated string (possibly with placeholders).\n>\n> eval_gettext \"Stray $dotest directory found. ...\"\n>\n> would allow the shell to expand $dotest before eval_gettext sees it,\n> which would mean the string is no longer a constant.\n\nAh. I was scratching my head wondering why $dotest needed to be\ntranslated (it's just a path).\n"},{"id":"220892","messageId":"7vip1gpqjp.fsf@alter.siamese.dyndns.org","threadId":"34134","inReplyTo":"CALkWK0m6iiDAmbxG9wrpgxJ3=nM816U7RM519Mu62qSgDwJbaQ@mail.gmail.com","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-14T17:09:46Z","receivedAt":"2013-06-14T17:09:46Z","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>> And then the message triggers at the second test in t3420 when\n>> applied on top of 587947750bd7 (rebase: implement --[no-]autostash\n>> and rebase.autostash, 2013-05-12) or 45acb7592825 (Merge branch\n>> 'rr/rebase-autostash', 2013-06-11).\n>\n> What was triggered? (I didn't understand what you said)\n\nThe patch applied on top of either of these commits will *BREAK* the\nsecond test in t3420.\n"},{"id":"220893","messageId":"7vehc4pq1c.fsf@alter.siamese.dyndns.org","threadId":"34134","inReplyTo":"CALkWK0n+KSrehL3tZ557c2hfKna6p_jVGL+a2az5V+h4=DstJQ@mail.gmail.com","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-14T17:20:47Z","receivedAt":"2013-06-14T17:20:47Z","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>> The reason would become clear once you think what string you are\n>> feeding eval_gettext with if you do not escape. The translators\n>> translate a fixed string (possibly with placeholders) to a fixed\n>> translated string (possibly with placeholders).\n>>\n>> eval_gettext \"Stray $dotest directory found. ...\"\n>>\n>> would allow the shell to expand $dotest before eval_gettext sees it,\n>> which would mean the string is no longer a constant.\n>\n> Ah. I was scratching my head wondering why $dotest needed to be\n> translated (it's just a path).\n\n\"Stray\" and \"directory found\" are to be translated. You can have\ninfinite possibilities to $dotest that is computed at runtime, so\nthat should be kept as a variable, untranslated in the message\ntemplate.\n"},{"id":"220904","messageId":"7vbo78o6ut.fsf@alter.siamese.dyndns.org","threadId":"34134","inReplyTo":"7vip1gpqjp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-14T19:00:26Z","receivedAt":"2013-06-14T19:00:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ramkumar Ramachandra writes:\n>\n>> Junio C Hamano wrote:\n>>> And then the message triggers at the second test in t3420 when\n>>> applied on top of 587947750bd7 (rebase: implement --[no-]autostash\n>>> and rebase.autostash, 2013-05-12) or 45acb7592825 (Merge branch\n>>> 'rr/rebase-autostash', 2013-06-11).\n>>\n>> What was triggered? (I didn't understand what you said)\n>\n> The patch applied on top of either of these commits will *BREAK* the\n> second test in t3420.\n\nHere is how \"sh -x t3420-*.sh -i\" ends:\n\n ...\n ++ git rebase unrelated-onto-branch\n Created autostash: cdca6ca\n HEAD is now at 0c4d2f1 third commit\n First, rewinding head to replay your work on top of it...\n Stray /srv/project/git/git.git/t/trash\n directory.t3420-rebase-autostash/.git/rebase-apply directory found.\n Use \"git am --abort\" to remove it.\n + eval_ret=1\n + test -z t\n + test 1 = 0\n + test -n ''\n + test t = t\n + test -n ''\n + return 1\n + test_failure_ 'rebase: dirty worktree, non-conflicting rebase' '\n ...\n\nI _think_ the new check you added may be too loose.\n\nA fix-up may look like this.\n\n\n git-am.sh | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 37edfae..3f89cf6 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -505,9 +505,9 @@ then\n \t\texit ;;\n \tesac\n \trm -f \"$dotest/dirtyindex\"\n-else\n-\t# Possible stray $dotest directory\n-\tif test -d \"$dotest\"; then\n+elif test -d \"$dotest\" && ! test -f \"$dotest/autostash\"\n+then\n+\t# stray $dotest directory\n \tcase \"$skip,$resolved,$abort\" in\n \t,,t)\n \t\trm -fr \"$dotest\"\n@@ -518,8 +518,7 @@ else\n Use \\\"git am --abort\\\" to remove it.\")\"\n \t\t;;\n \tesac\n-\tfi\n-\n+else\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.\")\"\n"},{"id":"220936","messageId":"CALkWK0nRtrvd=oX04XfqovYg1dnx3h+aNSNWH6MXnssOBqz=xw@mail.gmail.com","threadId":"34134","inReplyTo":"7vbo78o6ut.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-15T12:35:44Z","receivedAt":"2013-06-15T12:35:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I _think_ the new check you added may be too loose.\n\nYep, I totally forgot about the case when git-am.sh is called from an\nexisting script. In that case, it is upto the caller to handle\nwhatever stray directories; we have no business meddling with that.\n\n> A fix-up may look like this.\n\nNo, don't leak autostash detail.\n\nJust change that condition to\n\n if test -d \"$dotest\" && test -z \"$rebasing\"\n\nand we're done. I'll send in a re-roll.\n"},{"id":"220984","messageId":"CALkWK0k9hhCvYs=SrCi_s=q-MfhFWtkw8q2PYG+Vy-tRq65nsA@mail.gmail.com","threadId":"34134","inReplyTo":"CALkWK0mF7s7Zfp4VTekT9AhHNhd80Jj3GT8SaKVs4gAATACsBA@mail.gmail.com","subject":"Re: [PATCH 1/2] am: handle stray $dotest directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-06-16T06:36:08Z","receivedAt":"2013-06-16T06:36:08Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Can you tell me how to get shell-script-mode to indent the\n> case statement properly? (I used the default indentation)\n\nNever mind; I figured it out:\n\n(setq sh-indent-for-case-label 0)\n(setq sh-indent-for-case-alt '+)\n\nMaybe we should dump the relevant parts of my .emacs somewhere in-tree?\n"}]}