{"thread":{"id":"37495","subject":"[RFC PATCHv3 0/4] am: patch-format","startedAt":"2014-09-05T10:06:47Z","lastAt":"2014-09-06T12:46:32Z","messageCount":13,"participants":["Chris Packham","Johannes Sixt","Junio C Hamano","Stephen Boyd","Torsten Bögershausen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"248893","messageId":"1409911611-20370-1-git-send-email-judge.packham@gmail.com","threadId":"37495","inReplyTo":null,"subject":"[RFC PATCHv3 0/4] am: patch-format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-05T10:06:47Z","receivedAt":"2014-09-05T10:06:47Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"I've re-ordered things in this round. Hopefully the first 3 patches are\nuncontroversial, Junio has expressed reservations about the 4th. I\nhaven't looked at moving the changes into mailsplit yet or I could try\nand update gitk to use something other than diff-tree or I could write\nsome external filter programs.\n\nChris Packham (4):\n am: avoid re-directing stdin twice\n t/am: add test for stgit patch format\n t/am: add tests for hg patch format\n am: add gitk patch format\n\n Documentation/git-am.txt | 3 +-\n git-am.sh | 38 +++++++++++++++++++++++-\n t/t4150-am.sh | 76 ++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 115 insertions(+), 2 deletions(-)\n\n-- \n2.1.0.64.gc343089\n"},{"id":"248894","messageId":"1409911611-20370-2-git-send-email-judge.packham@gmail.com","threadId":"37495","inReplyTo":"1409911611-20370-1-git-send-email-judge.packham@gmail.com","subject":"[RFC PATCHv3 1/4] am: avoid re-directing stdin twice","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-05T10:06:48Z","receivedAt":"2014-09-05T10:06:48Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"In check_patch_format we feed $1 to a block that attempts to determine\nthe patch format. Since we've already redirected $1 to stdin there is no\nneed to redirect it again when we invoke tr. This prevents the following\nerrors when invoking git am\n\n $ git am patch.patch\n tr: write error: Broken pipe\n tr: write error\n Patch format detection failed.\n\nCc: Stephen Boyd \nSigned-off-by: Chris Packham \n---\nNothing new since http://article.gmane.org/gmane.comp.version-control.git/256425\n\n git-am.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex ee61a77..fade7f8 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -250,7 +250,7 @@ check_patch_format () {\n \t\t\t# discarding the indented remainder of folded lines,\n \t\t\t# and see if it looks like that they all begin with the\n \t\t\t# header field names...\n-\t\t\ttr -d '\\015' <\"$1\" |\n+\t\t\ttr -d '\\015' |\n \t\t\tsed -n -e '/^$/q' -e '/^[ \t]/d' -e p |\n \t\t\tsane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n \t\t\tpatch_format=mbox\n-- \n2.1.0.64.gc343089\n"},{"id":"248895","messageId":"1409911611-20370-3-git-send-email-judge.packham@gmail.com","threadId":"37495","inReplyTo":"1409911611-20370-1-git-send-email-judge.packham@gmail.com","subject":"[RFC PATCHv3 2/4] t/am: add test for stgit patch format","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-05T10:06:49Z","receivedAt":"2014-09-05T10:06:49Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"This adds a tests which exercise the detection of the stgit format.\nThere is a current know breakage in that the code that deals with stgit\nin split_patches can't handle reading from stdin.\n\nSigned-off-by: Chris Packham \n---\n t/t4150-am.sh | 27 +++++++++++++++++++++++++++\n 1 file changed, 27 insertions(+)\n\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 5edb79a..dbe3475 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -103,6 +103,15 @@ test_expect_success setup '\n \t\techo \"X-Fake-Field: Line Three\" &&\n \t\tgit format-patch --stdout first | sed -e \"1d\"\n \t} > patch1-ws.eml &&\n+\t{\n+\t\techo \"From: $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>\" &&\n+\t\techo &&\n+\t\tcat msg &&\n+\t\techo &&\n+\t\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" &&\n+\t\techo \"---\" &&\n+\t\tgit diff-tree --stat -p second | sed -e \"1d\"\n+\t} > patch1-stgit.eml &&\n \n \tsed -n -e \"3,\\$p\" msg >file &&\n \tgit add file &&\n@@ -186,6 +195,24 @@ test_expect_success 'am applies patch e-mail with preceding whitespace' '\n \ttest \"$(git rev-parse second^)\" = \"$(git rev-parse HEAD^)\"\n '\n \n+test_expect_success 'am applies patch generated by stgit' '\n+\trm -fr .git/rebase-apply &&\n+\tgit reset --hard &&\n+\tgit checkout first &&\n+\tgit am patch1-stgit.eml &&\n+\ttest_path_is_missing .git/rebase-apply &&\n+\tgit diff --exit-code second\n+'\n+\n+test_expect_failure 'am applies patch using --patch-format=stgit' '\n+\trm -fr .git/rebase-apply &&\n+\tgit reset --hard &&\n+\tgit checkout first &&\n+\tgit am --patch-format=stgit \nSigned-off-by: Chris Packham \n---\nNote. I don't have access to a mercurial repository (plus I know next to\nnothing about it) so the patch I've generated for the test was created\nby looking at the format detection code. If someone can show me an\nactual example of what mercurial produces that'd be helpful.\n\n t/t4150-am.sh | 26 ++++++++++++++++++++++++++\n 1 file changed, 26 insertions(+)\n\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex dbe3475..8ee81cf 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -112,6 +112,14 @@ test_expect_success setup '\n \t\techo \"---\" &&\n \t\tgit diff-tree --stat -p second | sed -e \"1d\"\n \t} > patch1-stgit.eml &&\n+\t{\n+\t\techo \"# HG changeset patch\"\n+\t\techo \"# User $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>\"\n+\t\techo &&\n+\t\tcat msg &&\n+\t\techo \"---\" &&\n+\t\tgit diff-tree --stat -p second | sed -e \"1d\"\n+\t} > patch1-hg.eml &&\n \n \tsed -n -e \"3,\\$p\" msg >file &&\n \tgit add file &&\n@@ -213,6 +221,24 @@ test_expect_failure 'am applies patch using --patch-format=stgit' '\n \tgit diff --exit-code second\n '\n \n+test_expect_success 'am applies patch generated by hg' '\n+\trm -fr .git/rebase-apply &&\n+\tgit reset --hard &&\n+\tgit checkout first &&\n+\tgit am patch1-hg.eml &&\n+\ttest_path_is_missing .git/rebase-apply &&\n+\tgit diff --exit-code second\n+'\n+\n+test_expect_failure 'am applies patch using --patch-format=hg' '\n+\trm -fr .git/rebase-apply &&\n+\tgit reset --hard &&\n+\tgit checkout first &&\n+\tgit am --patch-format=hg \n---\nThis hasn't materially changed from the version Junio expressed\nreservations about[1]. It solves my immediate problem but perhaps this\n(as well as stgit and hg) belong as external filters in a pipeline\nbefore git am. Or maybe mailsplit should absorb the functionality?\n\n[1] - http://article.gmane.org/gmane.comp.version-control.git/256426\n\n Documentation/git-am.txt | 3 ++-\n git-am.sh | 36 ++++++++++++++++++++++++++++++++++++\n t/t4150-am.sh | 23 +++++++++++++++++++++++\n 3 files changed, 61 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-am.txt b/Documentation/git-am.txt\nindex 9adce37..b59d2b3 100644\n--- a/Documentation/git-am.txt\n+++ b/Documentation/git-am.txt\n@@ -101,7 +101,8 @@ default. You can use `--no-utf8` to override this.\n \tBy default the command will try to detect the patch format\n \tautomatically. This option allows the user to bypass the automatic\n \tdetection and specify the patch format that the patch(es) should be\n-\tinterpreted as. Valid formats are mbox, stgit, stgit-series and hg.\n+\tinterpreted as. Valid formats are mbox, stgit, stgit-series, hg and\n+\tgitk.\n \n -i::\n --interactive::\ndiff --git a/git-am.sh b/git-am.sh\nindex fade7f8..5d69c89 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -227,6 +227,9 @@ check_patch_format () {\n \t\t\"# HG changeset patch\")\n \t\t\tpatch_format=hg\n \t\t\t;;\n+\t\t'commit '*)\n+\t\t\tpatch_format=gitk\n+\t\t\t;;\n \t\t*)\n \t\t\t# if the second line is empty and the third is\n \t\t\t# a From, Author or Date entry, this is very\n@@ -357,6 +360,39 @@ split_patches () {\n \t\tthis=\n \t\tmsgnum=\n \t\t;;\n+\tgitk)\n+\t\t# These patches are generates with 'git diff-tree -p --pretty'\n+\t\t# we discard the 'commit' line, after that the first line not\n+\t\t# starting with 'Author:' or 'Date:' is the subject. We also\n+\t\t# need to strip leading whitespace from the message body.\n+\t\tthis=0\n+\t\tfor gitk in \"$@\"\n+\t\tdo\n+\t\t\tthis=$(expr \"$this\" + 1)\n+\t\t\tmsgnum=$(printf \"%0${prec}d\" $this)\n+\t\t\t@@PERL@@ -ne 'BEGIN { $subject = 0; $diff = 0; }\n+\t\t\t\tif (!$diff) { s/^ // ; }\n+\t\t\t\tif ($subject > 1) { print ; }\n+\t\t\t\telsif (/^commit\\s.*$/) { next ; }\n+\t\t\t\telsif (/^\\s+$/) { next ; }\n+\t\t\t\telsif (/^Author:/) { s/Author/From/ ; print ; }\n+\t\t\t\telsif (/^Date:/) { print ;}\n+\t\t\t\telsif (/^diff --git/) { $diff = 1 ; print ;}\n+\t\t\t\telsif ($subject) {\n+\t\t\t\t\t$subject = 2 ;\n+\t\t\t\t\tprint \"\\n\" ;\n+\t\t\t\t\tprint ;\n+\t\t\t\t} else {\n+\t\t\t\t\tprint \"Subject: \", $_ ;\n+\t\t\t\t\t$subject = 1;\n+\t\t\t\t}\n+\t\t\t' <\"$gitk\" >\"$dotest/$msgnum\" || clean_abort\n+\n+\t\tdone\n+\t\techo \"$this\" >\"$dotest/last\"\n+\t\tthis=\n+\t\tmsgnum=\n+\t\t;;\n \t*)\n \t\tif test -n \"$patch_format\"\n \t\tthen\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 8ee81cf..5d4f7be 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -120,6 +120,7 @@ test_expect_success setup '\n \t\techo \"---\" &&\n \t\tgit diff-tree --stat -p second | sed -e \"1d\"\n \t} > patch1-hg.eml &&\n+\tgit diff-tree -p --pretty second >patch1-gitk.eml &&\n \n \tsed -n -e \"3,\\$p\" msg >file &&\n \tgit add file &&\n@@ -239,6 +240,28 @@ test_expect_failure 'am applies patch using --patch-format=hg' '\n \tgit diff --exit-code second\n '\n \n+test_expect_success 'am applies patch generated by gitk' '\n+\trm -fr .git/rebase-apply &&\n+\tgit reset --hard &&\n+\tgit checkout first &&\n+\tgit am patch1-gitk.eml &&\n+\ttest_path_is_missing .git/rebase-apply &&\n+\tgit diff --exit-code second &&\n+\ttest \"$(git rev-parse second)\" = \"$(git rev-parse HEAD)\" &&\n+\ttest \"$(git rev-parse second^)\" = \"$(git rev-parse HEAD^)\"\n+'\n+\n+test_expect_failure 'am applies patch using --patch-format=gitk' '\n+\trm -fr .git/rebase-apply &&\n+\tgit reset --hard &&\n+\tgit checkout first &&\n+\tgit am --patch-format=gitk In check_patch_format we feed $1 to a block that attempts to determine\n> the patch format. Since we've already redirected $1 to stdin there is no\n> need to redirect it again when we invoke tr. This prevents the following\n> errors when invoking git am\n> \n> $ git am patch.patch\n> tr: write error: Broken pipe\n> tr: write error\n> Patch format detection failed.\n> \n> Cc: Stephen Boyd \n> Signed-off-by: Chris Packham \n> ---\n> Nothing new since http://article.gmane.org/gmane.comp.version-control.git/256425\n> \n> git-am.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-am.sh b/git-am.sh\n> index ee61a77..fade7f8 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -250,7 +250,7 @@ check_patch_format () {\n> \t\t\t# discarding the indented remainder of folded lines,\n> \t\t\t# and see if it looks like that they all begin with the\n> \t\t\t# header field names...\n> -\t\t\ttr -d '\\015' <\"$1\" |\n> +\t\t\ttr -d '\\015' |\n> \t\t\tsed -n -e '/^$/q' -e '/^[ \t]/d' -e p |\n> \t\t\tsane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n> \t\t\tpatch_format=mbox\n> \n\nI think this change is wrong. This pipeline checks whether one of the\nlines at the top of the file contains something that looks like an email\nheader. With your change, the first three lines would not be looked at\nbecause they were already consumed earlier.\n\nI wonder why tr (assuming it is *this* instance of tr) dies with a write\nerror instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then\nthe tr invocation inherits this \"ignore SIGPIPE\" setting?\n\nThe only thing your version changes is that tr writes a bit less text\ninto the pipe. Perhaps its just sufficient that the output fits into the\npipe buffer, and no error occurs anymore? Then the new version is not a\nreal fix: make the patch text a bit longer, and the error is back.\n\n-- Hannes\n"},{"id":"248933","messageId":"CAFOYHZBct1CRA+NumVMvbbuELWTRoGL5FkhBfHD2Wk7QZVe1fA@mail.gmail.com","threadId":"37495","inReplyTo":"540A1C7B.80109@kdbg.org","subject":"Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-09-05T21:31:06Z","receivedAt":"2014-09-05T21:31:06Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt wrote:\n> Am 05.09.2014 12:06, schrieb Chris Packham:\n>> In check_patch_format we feed $1 to a block that attempts to determine\n>> the patch format. Since we've already redirected $1 to stdin there is no\n>> need to redirect it again when we invoke tr. This prevents the following\n>> errors when invoking git am\n>>\n>> $ git am patch.patch\n>> tr: write error: Broken pipe\n>> tr: write error\n>> Patch format detection failed.\n>>\n>> Cc: Stephen Boyd \n>> Signed-off-by: Chris Packham \n>> ---\n>> Nothing new since http://article.gmane.org/gmane.comp.version-control.git/256425\n>>\n>> git-am.sh | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/git-am.sh b/git-am.sh\n>> index ee61a77..fade7f8 100755\n>> --- a/git-am.sh\n>> +++ b/git-am.sh\n>> @@ -250,7 +250,7 @@ check_patch_format () {\n>> # discarding the indented remainder of folded lines,\n>> # and see if it looks like that they all begin with the\n>> # header field names...\n>> - tr -d '\\015' <\"$1\" |\n>> + tr -d '\\015' |\n>> sed -n -e '/^$/q' -e '/^[ ]/d' -e p |\n>> sane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n>> patch_format=mbox\n>>\n>\n> I think this change is wrong. This pipeline checks whether one of the\n> lines at the top of the file contains something that looks like an email\n> header. With your change, the first three lines would not be looked at\n> because they were already consumed earlier.\n>\n> I wonder why tr (assuming it is *this* instance of tr) dies with a write\n> error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then\n> the tr invocation inherits this \"ignore SIGPIPE\" setting?\n>\n> The only thing your version changes is that tr writes a bit less text\n> into the pipe. Perhaps its just sufficient that the output fits into the\n> pipe buffer, and no error occurs anymore? Then the new version is not a\n> real fix: make the patch text a bit longer, and the error is back.\n>\n> -- Hannes\n>\n\nI did notice some oddities when attempting to reproduce this issue.\nThey would be explained by the output fitting into the buffer. So yes\nperhaps this solution has just changed enough so that it no longer\ntriggers on the particular patch I was testing with. It still seems a\nbit funny that we start re-reading the input part way through\nprocessing it.\n\nPerhaps putting the tr outside the whole block would be a better solution?\n"},{"id":"248935","messageId":"xmqqoautpw1g.fsf@gitster.dls.corp.google.com","threadId":"37495","inReplyTo":"CAFOYHZBct1CRA+NumVMvbbuELWTRoGL5FkhBfHD2Wk7QZVe1fA@mail.gmail.com","subject":"Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-05T22:00:43Z","receivedAt":"2014-09-05T22:00:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Packham writes:\n\n> On Sat, Sep 6, 2014 at 8:26 AM, Johannes Sixt wrote:\n>> Am 05.09.2014 12:06, schrieb Chris Packham:\n>>> In check_patch_format we feed $1 to a block that attempts to determine\n>>> the patch format. Since we've already redirected $1 to stdin there is no\n>>> need to redirect it again when we invoke tr. This prevents the following\n>>> errors when invoking git am\n>>>\n>>> $ git am patch.patch\n>>> tr: write error: Broken pipe\n>>> tr: write error\n>>> Patch format detection failed.\n>>> ...\n>>\n>> I wonder why tr (assuming it is *this* instance of tr) dies with a write\n>> error instead of from a SIGPIPE. Is SIGPIPE ignored somewhere and then\n>> the tr invocation inherits this \"ignore SIGPIPE\" setting?\n>> ...\n> Perhaps putting the tr outside the whole block would be a better solution?\n\nPerhaps fixing the root cause of your (but not other people's) \"tr\"\nfailing is the right solution, no?\n\nAlso,\n\n>>> - tr -d '\\015' <\"$1\" |\n>>> sed -n -e '/^$/q' -e '/^[ ]/d' -e p |\n>>> sane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n>>> patch_format=mbox\n\nas the tr is at an upsteam of this pipeline, it does not really\nmatter to the outcome if it gives a write-error error message or not\n(the downstream sane_egrep would have decided, based on the data it\nwas given, if the payload was mbox format), so...\n\nAn easier workaround may be to update the sed script downstream of\ntr. It stops reading as soon as it finished to save cycles, and tr\nshould know that it does not have to produce any more output. For a\nbroken tr installation, the sed script could be taught to slurp\neverything in the message body (without passing it to downstream\nsane_egrep, of course), and your \"tr\" would not see a broken pipe.\n\nBut that is still a workaround, not a fix, and an expensive one at\nthat.\n"},{"id":"248936","messageId":"xmqqk35hpvbg.fsf@gitster.dls.corp.google.com","threadId":"37495","inReplyTo":"xmqqoautpw1g.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-05T22:16:19Z","receivedAt":"2014-09-05T22:16:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Also,\n>\n>>>> - tr -d '\\015' <\"$1\" |\n>>>> sed -n -e '/^$/q' -e '/^[ ]/d' -e p |\n>>>> sane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n>>>> patch_format=mbox\n>\n> as the tr is at an upsteam of this pipeline, it does not really\n> matter to the outcome if it gives a write-error error message or not\n> (the downstream sane_egrep would have decided, based on the data it\n> was given, if the payload was mbox format), so...\n>\n> An easier workaround may be to update the sed script downstream of\n> tr. It stops reading as soon as it finished to save cycles, and tr\n> should know that it does not have to produce any more output. For a\n> broken tr installation, the sed script could be taught to slurp\n> everything in the message body (without passing it to downstream\n> sane_egrep, of course), and your \"tr\" would not see a broken pipe.\n>\n> But that is still a workaround, not a fix, and an expensive one at\n> that.\n\nRedoing what e3f67d30 (am: fix patch format detection for\nThunderbird \"Save As\" emails, 2010-01-25) tried to do without\nwasting a fork and a pipe may be a workable improvement.\n\nI see Stephen who wrote the original \"Thunderbird save-as\" is\nalready on the Cc list. How about doing it this way instead?\n\n git-am.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex ee61a77..9db3846 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -250,8 +250,7 @@ check_patch_format () {\n \t\t\t# discarding the indented remainder of folded lines,\n \t\t\t# and see if it looks like that they all begin with the\n \t\t\t# header field names...\n-\t\t\ttr -d '\\015' <\"$1\" |\n-\t\t\tsed -n -e '/^$/q' -e '/^[ \t]/d' -e p |\n+\t\t\tsed -n -e '/^$/q' -e '/^\\r$/q' -e '/^[ \t]/d' -e p <\"$1\" |\n \t\t\tsane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n \t\t\tpatch_format=mbox\n \t\tfi\n"},{"id":"248938","messageId":"xmqqfvg5puws.fsf@gitster.dls.corp.google.com","threadId":"37495","inReplyTo":"xmqqk35hpvbg.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-05T22:25:06Z","receivedAt":"2014-09-05T22:25:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Redoing what e3f67d30 (am: fix patch format detection for\n> Thunderbird \"Save As\" emails, 2010-01-25) tried to do without\n> wasting a fork and a pipe may be a workable improvement.\n>\n> I see Stephen who wrote the original \"Thunderbird save-as\" is\n> already on the Cc list. How about doing it this way instead?\n\nNot that way, but more like this.\n\n git-am.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex ee61a77..32e3039 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -250,8 +250,7 @@ check_patch_format () {\n \t\t\t# discarding the indented remainder of folded lines,\n \t\t\t# and see if it looks like that they all begin with the\n \t\t\t# header field names...\n-\t\t\ttr -d '\\015' <\"$1\" |\n-\t\t\tsed -n -e '/^$/q' -e '/^[ \t]/d' -e p |\n+\t\t\tsed -n -e 's/\\r$//' -e '/^$/q' -e '/^[ \t]/d' -e p |\n \t\t\tsane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n \t\t\tpatch_format=mbox\n \t\tfi\n"},{"id":"248939","messageId":"CALaEz9Xbk_sAAJ0wNCgC9Rzr=E9Ke0H3YEwGr1_4VNgv0AwYhw@mail.gmail.com","threadId":"37495","inReplyTo":"xmqqfvg5puws.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2014-09-05T23:18:57Z","receivedAt":"2014-09-05T23:18:57Z","isPatch":false,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"(replying from webmail interface)\n\nOn Fri, Sep 5, 2014 at 3:25 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Redoing what e3f67d30 (am: fix patch format detection for\n>> Thunderbird \"Save As\" emails, 2010-01-25) tried to do without\n>> wasting a fork and a pipe may be a workable improvement.\n>>\n>> I see Stephen who wrote the original \"Thunderbird save-as\" is\n>> already on the Cc list. How about doing it this way instead?\n>\n\nIt was so long ago I can't even remember writing that patch. But I\ngoogled the thread from 4.5 years ago and I see that you suggested we\nuse tr because \\r is not portable[1].\n\n> Not that way, but more like this.\n>\n> git-am.sh | 3 +--\n> 1 file changed, 1 insertion(+), 2 deletions(-)\n>\n> diff --git a/git-am.sh b/git-am.sh\n> index ee61a77..32e3039 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -250,8 +250,7 @@ check_patch_format () {\n> # discarding the indented remainder of folded lines,\n> # and see if it looks like that they all begin with the\n> # header field names...\n> - tr -d '\\015' <\"$1\" |\n> - sed -n -e '/^$/q' -e '/^[ ]/d' -e p |\n> + sed -n -e 's/\\r$//' -e '/^$/q' -e '/^[ ]/d' -e p |\n> sane_egrep -v '^[!-9;-~]+:' >/dev/null ||\n> patch_format=mbox\n> fi\n\n[1] http://git.661346.n2.nabble.com/PATCH-am-fix-patch-format-detection-for-Thunderbird-quot-Save-As-quot-emails-td4184273.html\n"},{"id":"248941","messageId":"xmqqbnqtp5he.fsf@gitster.dls.corp.google.com","threadId":"37495","inReplyTo":"CALaEz9Xbk_sAAJ0wNCgC9Rzr=E9Ke0H3YEwGr1_4VNgv0AwYhw@mail.gmail.com","subject":"Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-06T07:34:21Z","receivedAt":"2014-09-06T07:34:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephen Boyd writes:\n\n>>> I see Stephen who wrote the original \"Thunderbird save-as\" is\n>>> already on the Cc list. How about doing it this way instead?\n>\n> It was so long ago I can't even remember writing that patch. But I\n> googled the thread from 4.5 years ago and I see that you suggested we\n> use tr because \\r is not portable[1].\n\nHmph. That's unfortunate that this may be one of those things that\neven though it is in POSIX the real world prevents us from using it.\n\nI wonder if things changed over the past four years, though. Can\nfolks on OSX or BSD do a quick check?\n\nThanks.\n"},{"id":"248977","messageId":"540B0228.1070201@web.de","threadId":"37495","inReplyTo":"xmqqbnqtp5he.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC PATCHv3 1/4] am: avoid re-directing stdin twice","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-09-06T12:46:32Z","receivedAt":"2014-09-06T12:46:32Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-09-06 09.34, Junio C Hamano wrote:\n> Stephen Boyd writes:\n> \n>>>> I see Stephen who wrote the original \"Thunderbird save-as\" is\n>>>> already on the Cc list. How about doing it this way instead?\n>>\n>> It was so long ago I can't even remember writing that patch. But I\n>> googled the thread from 4.5 years ago and I see that you suggested we\n>> use tr because \\r is not portable[1].\n> \n> Hmph. That's unfortunate that this may be one of those things that\n> even though it is in POSIX the real world prevents us from using it.\n> \n> I wonder if things changed over the past four years, though. Can\n> folks on OSX or BSD do a quick check?\n>\nI may have missed the discussion, does this help?\n\"\\r\" can be used with tr, but not with sed:\n\n\ntb@macosx:/tmp> cat ./xx.sh \n#!/bin/sh\nwhich tr\nprintf \"AB\\rCD\\n\" | tr 'A\\r\\n\\BCD' 'aRNbcd' | xxd\nprintf \"E\\rE\" | tr -d '\\r' | xxd\nwhich sed\nprintf \"AB\\rCD\\n\" | sed -e 's/\\r/R/g' | xxd\nprintf \"E\\rE\" | sed -e 's/\\r//g' | xxd\n\ntb@macosx:/tmp> ./xx.sh \n/usr/bin/tr\n0000000: 6162 5263 644e abRcdN\n0000000: 4545 EE\n/usr/bin/sed\n0000000: 4142 0d43 440a AB.CD.\n0000000: 450d 450a E.E.\ntb@macosx:/tmp> \n"}]}