{"thread":{"id":"40845","subject":"Odd issue with Git-am","startedAt":"2015-11-20T21:02:42Z","lastAt":"2015-11-20T21:22:34Z","messageCount":2,"participants":["alan@clueserver.org","Stefan Beller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"273559","messageId":"7df90d19cfa7e987a23a22b5cd90fe6a.squirrel@clueserver.org","threadId":"40845","inReplyTo":null,"subject":"Odd issue with Git-am","fromName":"","fromEmail":"alan@clueserver.org","sentAt":"2015-11-20T21:02:42Z","receivedAt":"2015-11-20T21:02:42Z","isPatch":false,"sender":{"key":"alan@clueserver.org","avatar":null},"body":"The following describes bad behavior, but it is bad behavior that git-am\ndoes not flag as bad. It just drops data silently.\n\nI have a developer who has a patch that I am importing into git with\ngit-am. (Currently they have a quilt-like setup that is full of bad and\nincomplete patches.)\n\nAt some point in the past, someone hand edited the patch and added two\nlines. They did not, however, change the @@ references in the patch for\nthe line count.\n\nThe patch added a file. The line that contained the length of the file was\n\"@@ -0,0 +0,1155 @@\" instead of \"@@ -0,0 +0,1157 @@\". The result was that\nwhen the patch was applied it silently dropped the last two lines of the\nfile.\n\nMy assumption is that it should either apply the full file and/or throw an\nerror. This just drops data silently.\n\nYes people should not be editing patches by hand. This migration is part\nof the effort to get them to stop doing that.\n\nShouldn't git-am detect that the patch data and the meta data do not match\nand warn the user or am I just being too damn picky here?\n"},{"id":"273561","messageId":"CAGZ79kagFM9aEobUJhr3hyiH3tWjU2=HMHs=PsBMcks_FMpByw@mail.gmail.com","threadId":"40845","inReplyTo":"7df90d19cfa7e987a23a22b5cd90fe6a.squirrel@clueserver.org","subject":"Re: Odd issue with Git-am","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-11-20T21:22:34Z","receivedAt":"2015-11-20T21:22:34Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Nov 20, 2015 at 1:02 PM, wrote:\n> The following describes bad behavior, but it is bad behavior that git-am\n> does not flag as bad. It just drops data silently.\n>\n> I have a developer who has a patch that I am importing into git with\n> git-am. (Currently they have a quilt-like setup that is full of bad and\n> incomplete patches.)\n>\n> At some point in the past, someone hand edited the patch and added two\n> lines. They did not, however, change the @@ references in the patch for\n> the line count.\n>\n> The patch added a file. The line that contained the length of the file was\n> \"@@ -0,0 +0,1155 @@\" instead of \"@@ -0,0 +0,1157 @@\". The result was that\n> when the patch was applied it silently dropped the last two lines of the\n> file.\n>\n> My assumption is that it should either apply the full file and/or throw an\n> error. This just drops data silently.\n>\n> Yes people should not be editing patches by hand. This migration is part\n> of the effort to get them to stop doing that.\n>\n> Shouldn't git-am detect that the patch data and the meta data do not match\n> and warn the user or am I just being too damn picky here?\n>\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\nCCing Paul who rewrote am recently.\nCCing Junio as he explained a similar issue to me once upon a time.\n\nCopying from a random mailing list submission:\n\n Subject: [PATCH 2/2] fsck: treat a NUL in a tag header as an error\n\n We check the return value of verify_header() for commits already, so do\n the same for tags as well.\n\n Signed-off-by: Rene Scharfe \n ---\n fsck.c | 3 ++-\n t/t1450-fsck.sh | 2 +-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\n diff --git a/fsck.c b/fsck.c\n index e41e753..4060f1f 100644\n --- a/fsck.c\n +++ b/fsck.c\n @@ -711,7 +711,8 @@ static int fsck_tag_buffer(struct tag\n*tag, const char *data,\n }\n }\n\n - if (verify_headers(buffer, size, &tag->object, options))\n + ret = verify_headers(buffer, size, &tag->object, options);\n + if (ret)\n goto done;\n\n if (!skip_prefix(buffer, \"object \", &buffer)) {\n diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\n index 6c96953..e66b7cb 100755\n --- a/t/t1450-fsck.sh\n +++ b/t/t1450-fsck.sh\n @@ -288,7 +288,7 @@ test_expect_success 'tag with bad tagger' '\n grep \"error in tag .*: invalid author/committer\" out\n '\n\n -test_expect_failure 'tag with NUL in header' '\n +test_expect_success 'tag with NUL in header' '\n sha=$(git rev-parse HEAD) &&\n q_to_nul >tag-NUL-header <<-EOF &&\n object $sha\n --\n 2.6.3\n\n --\n To unsubscribe from this list: send the line \"unsubscribe git\" in\n the body of a message to majordomo@vger.kernel.org\n More majordomo info at http://vger.kernel.org/majordomo-info.html\n\nHere we have 2 chunks, check where the second chunk ends\n(hint: it's at the line starting with --).\n\nOne of the features of patches (as by git-am) is to ignore the footers,\nsuch as the git version or the mailing list addendum.\n\nYou don't know how such a footer looks like (this one starts with --\nbut that's just coincidence, not by a defined protocol).\n\nSo it's hard to specify in your case when the file ends and what is\nend-of-message stuff. It's dangerous if you're not aware of the pit\nfall.\n"}]}