{"thread":{"id":"23813","subject":"[PATCH] git-am: Ignore whitespace before patches","startedAt":"2010-05-15T17:23:08Z","lastAt":"2010-08-12T21:34:51Z","messageCount":11,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Jon Seymour","Jay Soffian","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"141738","messageId":"1273944188-9472-1-git-send-email-avarab@gmail.com","threadId":"23813","inReplyTo":null,"subject":"[PATCH] git-am: Ignore whitespace before patches","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-05-15T17:23:08Z","receivedAt":"2010-05-15T17:23:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change git-am to ignore whitespace (as defined by sh's read) at the\nbeginning of patches.\n\nEmpty lines are wont to creep in at the beginning of patches, here's\nan example from a raw Gmail attachment:\n\n    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 20  |                |\n    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 0a  |               .|\n    52 65 74 75 72 6e 2d 50  61 74 68 3a 20 3c 61 76  |Return-Path: <av|\n\nWhitespace is also likely to appear if the user copy/pastes the patch\naround, e.g. via a pastebin, or any any number of other cases. This\nharms nothing and makes git-am's detection more fault tolerant.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n git-am.sh     |   16 +++++++++++++++-\n t/t4150-am.sh |   30 ++++++++++++++++++++++++++++++\n 2 files changed, 45 insertions(+), 1 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex 1056075..1b4baa8 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -172,7 +172,21 @@ check_patch_format () {\n \t# otherwise, check the first few lines of the first patch to try\n \t# to detect its format\n \t{\n-\t\tread l1\n+\t\twhile read -r line\n+\t\tdo\n+\t\t\tcase \"$line\" in\n+\t\t\t\t\"\")\n+\t\t\t\t\t# Just skip whitespace\n+\t\t\t\t\tcontinue\n+\t\t\t\t\t;;\n+\t\t\t\t*)\n+\t\t\t\t\t# First non-empty line\n+\t\t\t\t\tl1=$line\n+\t\t\t\t\tbreak\n+\t\t\t\t\t;;\n+\t\t\tesac\n+\t\tdone\n+\n \t\tread l2\n \t\tread l3\n \t\tcase \"$l1\" in\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 810b04b..3d089de 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -318,6 +318,36 @@ test_expect_success 'am without --committer-date-is-author-date' '\n \ttest \"$at\" != \"$ct\"\n '\n \n+test_expect_success 'am applying a patch that begins with an empty line' '\n+\tgit checkout first &&\n+\ttest_tick &&\n+\techo > patch1-white &&\n+\tcat patch1 >> patch1-white &&\n+\tgit am patch1-white &&\n+\tgit cat-file commit HEAD | sed -e \"/^\\$/q\" >head1 &&\n+\tat=$(sed -ne \"/^author /s/.*> //p\" head1) &&\n+\tct=$(sed -ne \"/^committer /s/.*> //p\" head1) &&\n+\ttest \"$at\" != \"$ct\"\n+'\n+\n+test_expect_success 'am applying a patch that begins with many empty lines' '\n+\tgit checkout first &&\n+\ttest_tick &&\n+\techo \"   \" > patch1-white2 &&\n+\techo \"  \" >> patch1-white2 &&\n+\techo \" \" >> patch1-white2 &&\n+\techo \"\" >> patch1-white2 &&\n+\techo \" \" >> patch1-white2 &&\n+\techo \"  \" >> patch1-white2 &&\n+\techo \"   \" >> patch1-white2 &&\n+\tcat patch1 >> patch1-white2 &&\n+\tgit am patch1-white2 &&\n+\tgit cat-file commit HEAD | sed -e \"/^\\$/q\" >head1 &&\n+\tat=$(sed -ne \"/^author /s/.*> //p\" head1) &&\n+\tct=$(sed -ne \"/^committer /s/.*> //p\" head1) &&\n+\ttest \"$at\" != \"$ct\"\n+'\n+\n # This checks for +0000 because TZ is set to UTC and that should\n # show up when the current time is used. The date in message is set\n # by test_tick that uses -0700 timezone; if this feature does not\n-- \n1.7.1.84.gd92f8\n"},{"id":"143350","messageId":"AANLkTikIlGeIEDdCo35us_Ovfmw3Cj4TQwCgDWAf1ecB@mail.gmail.com","threadId":"23813","inReplyTo":"1273944188-9472-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH] git-am: Ignore whitespace before patches","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-09T15:35:03Z","receivedAt":"2010-06-09T15:35:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, May 15, 2010 at 17:23, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> Change git-am to ignore whitespace (as defined by sh's read) at the\n> beginning of patches.\n>\n> Empty lines are wont to creep in at the beginning of patches, here's\n> an example from a raw Gmail attachment:\n>\n>    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 20  |                |\n>    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 0a  |               .|\n>    52 65 74 75 72 6e 2d 50  61 74 68 3a 20 3c 61 76  |Return-Path: <av|\n>\n> Whitespace is also likely to appear if the user copy/pastes the patch\n> around, e.g. via a pastebin, or any any number of other cases. This\n> harms nothing and makes git-am's detection more fault tolerant.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  git-am.sh     |   16 +++++++++++++++-\n>  t/t4150-am.sh |   30 ++++++++++++++++++++++++++++++\n>  2 files changed, 45 insertions(+), 1 deletions(-)\n>\n> diff --git a/git-am.sh b/git-am.sh\n> index 1056075..1b4baa8 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -172,7 +172,21 @@ check_patch_format () {\n>        # otherwise, check the first few lines of the first patch to try\n>        # to detect its format\n>        {\n> -               read l1\n> +               while read -r line\n> +               do\n> +                       case \"$line\" in\n> +                               \"\")\n> +                                       # Just skip whitespace\n> +                                       continue\n> +                                       ;;\n> +                               *)\n> +                                       # First non-empty line\n> +                                       l1=$line\n> +                                       break\n> +                                       ;;\n> +                       esac\n> +               done\n> +\n>                read l2\n>                read l3\n>                case \"$l1\" in\n> diff --git a/t/t4150-am.sh b/t/t4150-am.sh\n> index 810b04b..3d089de 100755\n> --- a/t/t4150-am.sh\n> +++ b/t/t4150-am.sh\n> @@ -318,6 +318,36 @@ test_expect_success 'am without --committer-date-is-author-date' '\n>        test \"$at\" != \"$ct\"\n>  '\n>\n> +test_expect_success 'am applying a patch that begins with an empty line' '\n> +       git checkout first &&\n> +       test_tick &&\n> +       echo > patch1-white &&\n> +       cat patch1 >> patch1-white &&\n> +       git am patch1-white &&\n> +       git cat-file commit HEAD | sed -e \"/^\\$/q\" >head1 &&\n> +       at=$(sed -ne \"/^author /s/.*> //p\" head1) &&\n> +       ct=$(sed -ne \"/^committer /s/.*> //p\" head1) &&\n> +       test \"$at\" != \"$ct\"\n> +'\n> +\n> +test_expect_success 'am applying a patch that begins with many empty lines' '\n> +       git checkout first &&\n> +       test_tick &&\n> +       echo \"   \" > patch1-white2 &&\n> +       echo \"  \" >> patch1-white2 &&\n> +       echo \" \" >> patch1-white2 &&\n> +       echo \"\" >> patch1-white2 &&\n> +       echo \" \" >> patch1-white2 &&\n> +       echo \"  \" >> patch1-white2 &&\n> +       echo \"   \" >> patch1-white2 &&\n> +       cat patch1 >> patch1-white2 &&\n> +       git am patch1-white2 &&\n> +       git cat-file commit HEAD | sed -e \"/^\\$/q\" >head1 &&\n> +       at=$(sed -ne \"/^author /s/.*> //p\" head1) &&\n> +       ct=$(sed -ne \"/^committer /s/.*> //p\" head1) &&\n> +       test \"$at\" != \"$ct\"\n> +'\n> +\n>  # This checks for +0000 because TZ is set to UTC and that should\n>  # show up when the current time is used. The date in message is set\n>  # by test_tick that uses -0700 timezone; if this feature does not\n> --\n> 1.7.1.84.gd92f8\n>\n>\n\nAdding Giuseppe Bilotta who wrote the original code I'm modifying to\nthe CC list.\n\nIt would be nice to get an ack or tested-by for this trivial patch. It\nmakes git-am & GMail integration much easier. Thanks.\n"},{"id":"143452","messageId":"7vd3vyn84j.fsf@alter.siamese.dyndns.org","threadId":"23813","inReplyTo":"1273944188-9472-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH] git-am: Ignore whitespace before patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-10T16:51:40Z","receivedAt":"2010-06-10T16:51:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> Change git-am to ignore whitespace (as defined by sh's read) at the\n> beginning of patches.\n>\n> Empty lines are wont to creep in at the beginning of patches, here's\n> an example from a raw Gmail attachment:\n>\n>     20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 20  |                |\n>     20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 0a  |               .|\n>     52 65 74 75 72 6e 2d 50  61 74 68 3a 20 3c 61 76  |Return-Path: <av|\n>\n> Whitespace is also likely to appear if the user copy/pastes the patch\n> around, e.g. via a pastebin, or any any number of other cases. This\n> harms nothing and makes git-am's detection more fault tolerant.\n\nActually cut-and-paste is often a major source of whitespace breakage\n(including tabs silently being expanded), and I personally think a patch\nlike this to encourage the practice is going in a wrong direction.\n"},{"id":"143453","messageId":"AANLkTikgWsumvqwicvOW8keGgvrgak8D6HXCGQ3wqHG6@mail.gmail.com","threadId":"23813","inReplyTo":"7vd3vyn84j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-am: Ignore whitespace before patches","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-10T17:03:57Z","receivedAt":"2010-06-10T17:03:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jun 10, 2010 at 16:51, Junio C Hamano <gitster@pobox.com> wrote:\n> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>> Change git-am to ignore whitespace (as defined by sh's read) at the\n>> beginning of patches.\n>>\n>> Empty lines are wont to creep in at the beginning of patches, here's\n>> an example from a raw Gmail attachment:\n>>\n>>     20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 20  |                |\n>>     20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 0a  |               .|\n>>     52 65 74 75 72 6e 2d 50  61 74 68 3a 20 3c 61 76  |Return-Path: <av|\n>>\n>> Whitespace is also likely to appear if the user copy/pastes the patch\n>> around, e.g. via a pastebin, or any any number of other cases. This\n>> harms nothing and makes git-am's detection more fault tolerant.\n>\n> Actually cut-and-paste is often a major source of whitespace breakage\n> (including tabs silently being expanded), and I personally think a patch\n> like this to encourage the practice is going in a wrong direction.\n\nIt doesn't encourage that copy/paste. It's just tangentally mentioned\nin the commit message since it's a plausable use case.\n\nWhat it does is enable the GMail -> download -> git-am workflow. GMail\n(and doubtless countless other) E-Mail providers introduce whitespace\nat the beginning of raw E-Mail messages, while otherwise leaving them\nintact.\n"},{"id":"147825","messageId":"1281556645-23361-1-git-send-email-avarab@gmail.com","threadId":"23813","inReplyTo":"1273944188-9472-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2] git-am: Ignore whitespace before patches","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-11T19:57:25Z","receivedAt":"2010-08-11T19:57:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change git-am to ignore whitespace (as defined by sh's read) at the\nbeginning of patches.\n\nThis makes git-am work with patches downloaded from the GMail web\ninterface, here's an example from a raw Gmail attachment produced with\n`hexdump -C':\n\n    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 20  |                |\n    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 0a  |               .|\n    52 65 74 75 72 6e 2d 50  61 74 68 3a 20 3c 61 76  |Return-Path: <av|\n\nHaving to tell GMail users that they must manually edit their patches\nbefore git-am will accept them (as this article does:\nhttp://evag.evn.am/git/git-and-gmail) isn't optimal.\n\nThis change is probably useful for other things than GMail patch\ndownloads, whitespace is also likely to appear if the user copy/pastes\nthe patch around, e.g. via a pastebin, or any any number of other\ncases. This change harms nothing and makes git-am's detection more\nfault tolerant.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nI originally sent this on July 8 but it was never picked up. Junio commented:\n\n>> Whitespace is also likely to appear if the user copy/pastes the patch\n>> around, e.g. via a pastebin, or any any number of other cases. This\n>> harms nothing and makes git-am's detection more fault tolerant.\n>\n> Actually cut-and-paste is often a major source of whitespace breakage\n> (including tabs silently being expanded), and I personally think a patch\n> like this to encourage the practice is going in a wrong direction.\n\nI disagree and think git-am should be smarter. Any human looking at\nsomething like a GMail mail.txt download will clearly see that it's a\npatch, but git-am is pedantic and doesn't skip past whitespace at the\nbeginning of the file.\n\nI think it should have more smarts and less pedanticness, and I run\ninto this bug every time I download a patch via GMail.\n\nSo please pick it up, thanks.\n\n git-am.sh     |   16 +++++++++++++++-\n t/t4150-am.sh |   30 ++++++++++++++++++++++++++++++\n 2 files changed, 45 insertions(+), 1 deletions(-)\n\ndiff --git a/git-am.sh b/git-am.sh\nindex e7f008c..4ed8544 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -173,7 +173,21 @@ check_patch_format () {\n \t# otherwise, check the first few lines of the first patch to try\n \t# to detect its format\n \t{\n-\t\tread l1\n+\t\twhile read -r line\n+\t\tdo\n+\t\t\tcase \"$line\" in\n+\t\t\t\t\"\")\n+\t\t\t\t\t# Just skip whitespace\n+\t\t\t\t\tcontinue\n+\t\t\t\t\t;;\n+\t\t\t\t*)\n+\t\t\t\t\t# First non-empty line\n+\t\t\t\t\tl1=$line\n+\t\t\t\t\tbreak\n+\t\t\t\t\t;;\n+\t\t\tesac\n+\t\tdone\n+\n \t\tread l2\n \t\tread l3\n \t\tcase \"$l1\" in\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 810b04b..3d089de 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -318,6 +318,36 @@ test_expect_success 'am without --committer-date-is-author-date' '\n \ttest \"$at\" != \"$ct\"\n '\n \n+test_expect_success 'am applying a patch that begins with an empty line' '\n+\tgit checkout first &&\n+\ttest_tick &&\n+\techo > patch1-white &&\n+\tcat patch1 >> patch1-white &&\n+\tgit am patch1-white &&\n+\tgit cat-file commit HEAD | sed -e \"/^\\$/q\" >head1 &&\n+\tat=$(sed -ne \"/^author /s/.*> //p\" head1) &&\n+\tct=$(sed -ne \"/^committer /s/.*> //p\" head1) &&\n+\ttest \"$at\" != \"$ct\"\n+'\n+\n+test_expect_success 'am applying a patch that begins with many empty lines' '\n+\tgit checkout first &&\n+\ttest_tick &&\n+\techo \"   \" > patch1-white2 &&\n+\techo \"  \" >> patch1-white2 &&\n+\techo \" \" >> patch1-white2 &&\n+\techo \"\" >> patch1-white2 &&\n+\techo \" \" >> patch1-white2 &&\n+\techo \"  \" >> patch1-white2 &&\n+\techo \"   \" >> patch1-white2 &&\n+\tcat patch1 >> patch1-white2 &&\n+\tgit am patch1-white2 &&\n+\tgit cat-file commit HEAD | sed -e \"/^\\$/q\" >head1 &&\n+\tat=$(sed -ne \"/^author /s/.*> //p\" head1) &&\n+\tct=$(sed -ne \"/^committer /s/.*> //p\" head1) &&\n+\ttest \"$at\" != \"$ct\"\n+'\n+\n # This checks for +0000 because TZ is set to UTC and that should\n # show up when the current time is used. The date in message is set\n # by test_tick that uses -0700 timezone; if this feature does not\n-- \n1.7.2.1.295.gdf931\n"},{"id":"147836","messageId":"AANLkTin2DM-q7acST-x_Oz7jU-hODy8se6sngGxTDi_9@mail.gmail.com","threadId":"23813","inReplyTo":"1281556645-23361-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2] git-am: Ignore whitespace before patches","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2010-08-11T22:50:13Z","receivedAt":"2010-08-11T22:50:13Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Thu, Aug 12, 2010 at 5:57 AM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> Change git-am to ignore whitespace (as defined by sh's read) at the\n> beginning of patches.\n>\n> This makes git-am work with patches downloaded from the GMail web\n> interface, here's an example from a raw Gmail attachment produced with\n> `hexdump -C':\n>\n>    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 20  |                |\n>    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 0a  |               .|\n>    52 65 74 75 72 6e 2d 50  61 74 68 3a 20 3c 61 76  |Return-Path: <av|\n>\n> Having to tell GMail users that they must manually edit their patches\n> before git-am will accept them (as this article does:\n> http://evag.evn.am/git/git-and-gmail) isn't optimal.\n\nThis is a good point. Current behaviour discourages testing of patches\nas delivered by e-mail since the GMail user is more likely to overlook\nactual whitespace errors in a patch because they come to expect this\nusual failure.\n\njon.\n"},{"id":"147838","messageId":"AANLkTind7ZtTS+Fi-tOSE7ExgD1ubhpLuxYJgWYms2aE@mail.gmail.com","threadId":"23813","inReplyTo":"AANLkTin2DM-q7acST-x_Oz7jU-hODy8se6sngGxTDi_9@mail.gmail.com","subject":"Re: [PATCH v2] git-am: Ignore whitespace before patches","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-11T22:55:14Z","receivedAt":"2010-08-11T22:55:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Aug 11, 2010 at 22:50, Jon Seymour <jon.seymour@gmail.com> wrote:\n> On Thu, Aug 12, 2010 at 5:57 AM, Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> Change git-am to ignore whitespace (as defined by sh's read) at the\n>> beginning of patches.\n>>\n>> This makes git-am work with patches downloaded from the GMail web\n>> interface, here's an example from a raw Gmail attachment produced with\n>> `hexdump -C':\n>>\n>>    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 20  |                |\n>>    20 20 20 20 20 20 20 20  20 20 20 20 20 20 20 0a  |               .|\n>>    52 65 74 75 72 6e 2d 50  61 74 68 3a 20 3c 61 76  |Return-Path: <av|\n>>\n>> Having to tell GMail users that they must manually edit their patches\n>> before git-am will accept them (as this article does:\n>> http://evag.evn.am/git/git-and-gmail) isn't optimal.\n>\n> This is a good point. Current behaviour discourages testing of patches\n> as delivered by e-mail since the GMail user is more likely to overlook\n> actual whitespace errors in a patch because they come to expect this\n> usual failure.\n\nJust to clarify, git-am doesn't print a whitespace error on GMail\npatches currently, the detection just fails:\n\n    $ git am ~/Desktop/mail.txt\n    Patch format detection failed.\n\nBut with my patch:\n\n    $ git am ~/Desktop/mail.txt\n    Applying: git-am: Ignore whitespace before patches\n"},{"id":"147930","messageId":"AANLkTinDHvwHLZfj6DDPtV39Z2xhDZREiqwdt5cjiaLP@mail.gmail.com","threadId":"23813","inReplyTo":"1281556645-23361-1-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2] git-am: Ignore whitespace before patches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-08-12T19:13:23Z","receivedAt":"2010-08-12T19:13:23Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Aug 11, 2010 at 3:57 PM, Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>> Actually cut-and-paste is often a major source of whitespace breakage\n>> (including tabs silently being expanded), and I personally think a patch\n>> like this to encourage the practice is going in a wrong direction.\n>\n> I disagree and think git-am should be smarter. Any human looking at\n> something like a GMail mail.txt download will clearly see that it's a\n> patch, but git-am is pedantic and doesn't skip past whitespace at the\n> beginning of the file.\n\nThe point of git-am being pedantic is to prevent the original patch\nfrom being applied w/silent corruption (e.g., tabs-to-spaces).\n\nPerhaps, before making git-am less strict, we should modify\nformat-patch to include a sha1 of the diff output so that corruption\ncan be reliably detected by git-am.\n\nJust a thought.\n\nj.\n"},{"id":"147934","messageId":"AANLkTikGaqj1QWtWEy3k=_E8q=bKNKjgJwHCQTUpnZmo@mail.gmail.com","threadId":"23813","inReplyTo":"AANLkTinDHvwHLZfj6DDPtV39Z2xhDZREiqwdt5cjiaLP@mail.gmail.com","subject":"Re: [PATCH v2] git-am: Ignore whitespace before patches","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-12T20:04:26Z","receivedAt":"2010-08-12T20:04:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Aug 12, 2010 at 19:13, Jay Soffian <jaysoffian@gmail.com> wrote:\n> On Wed, Aug 11, 2010 at 3:57 PM, Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>>> Actually cut-and-paste is often a major source of whitespace breakage\n>>> (including tabs silently being expanded), and I personally think a patch\n>>> like this to encourage the practice is going in a wrong direction.\n>>\n>> I disagree and think git-am should be smarter. Any human looking at\n>> something like a GMail mail.txt download will clearly see that it's a\n>> patch, but git-am is pedantic and doesn't skip past whitespace at the\n>> beginning of the file.\n>\n> The point of git-am being pedantic is to prevent the original patch\n> from being applied w/silent corruption (e.g., tabs-to-spaces).\n\nThe git-am code doesn't strike me as particularly pedantic. It'll fail\nto spot any number of common pitfalls like patches being pasted inline\n(recently discussed on list), double encoding, invalid author names /\nE-Mail addresses etc. (which can e.g. happen when applying patches\nfrom RT).\n\nThe parsing code just didn't think of this issue, I'm not aware of any\ncorruption that begins with whitespace being added to the beginning of\na patch, but I am aware of a non-corruption (GMail) that does that.\n\n> Perhaps, before making git-am less strict\n\nI don't think it's less strict with this patch, just more intelligent.\n\n> we should modify format-patch to include a sha1 of the diff output\n> so that corruption can be reliably detected by git-am.\n\nThere's a lot we could do in this department, and there was a previous\ndiscussion on list about schemas like that (can't find it now).\n\nWe could do an ad-hoc checksum, but including more than the SHA would\nbe better, e.g.:\n\n    --\n    cs:<7 char SHA1> t:<NUM TABS> s:<NUM SPACES> c:<NUM CHARS ([^\n\\t])> ln:<NUM LINES>\n\nThat'd allow git-am to print more intelligent error messages than just\n\"ok/not ok\", e.g.:\n\n    * \"your patch is $x lines, but the patch thinks it's $y, something\n       may have gone wrong with wrapping\"\n\n    * \"You have 0 tabs, but the patch thinks it has 20\"\n\netc., but that's a project for another day.\n"},{"id":"147939","messageId":"20100812202457.GC2029@burratino","threadId":"23813","inReplyTo":"AANLkTinDHvwHLZfj6DDPtV39Z2xhDZREiqwdt5cjiaLP@mail.gmail.com","subject":"Re: [PATCH v2] git-am: Ignore whitespace before patches","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-12T20:24:57Z","receivedAt":"2010-08-12T20:24:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jay Soffian wrote:\n\n> Perhaps, before making git-am less strict, we should modify\n> format-patch to include a sha1 of the diff output so that corruption\n> can be reliably detected by git-am.\n\nI seem to remember a discussion about hand-munging and rebasing of\npatches suggesting such verification might be a bad idea[1].  I dunno.\n\n[1] but all a search turned up is this:\nhttp://thread.gmane.org/gmane.comp.version-control.git/136008/focus=136234\n"},{"id":"147951","messageId":"7vocd7qzus.fsf@alter.siamese.dyndns.org","threadId":"23813","inReplyTo":"20100812202457.GC2029@burratino","subject":"Re: [PATCH v2] git-am: Ignore whitespace before patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-12T21:34:51Z","receivedAt":"2010-08-12T21:34:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Jay Soffian wrote:\n>\n>> Perhaps, before making git-am less strict, we should modify\n>> format-patch to include a sha1 of the diff output so that corruption\n>> can be reliably detected by git-am.\n>\n> I seem to remember a discussion about hand-munging and rebasing of\n> patches suggesting such verification might be a bad idea[1].  I dunno.\n>\n> [1] but all a search turned up is this:\n> http://thread.gmane.org/gmane.comp.version-control.git/136008/focus=136234\n\nYou meant this perhaps.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/138084/focus=138100\n"}]}