On Fri, Nov 20, 2015 at 1:02 PM, <alan@xxxxxxxxxxxxxx> wrote: > The following describes bad behavior, but it is bad behavior that git-am > does not flag as bad. It just drops data silently. > > I have a developer who has a patch that I am importing into git with > git-am. (Currently they have a quilt-like setup that is full of bad and > incomplete patches.) > > At some point in the past, someone hand edited the patch and added two > lines. They did not, however, change the @@ references in the patch for > the line count. > > The patch added a file. The line that contained the length of the file was > "@@ -0,0 +0,1155 @@" instead of "@@ -0,0 +0,1157 @@". The result was that > when the patch was applied it silently dropped the last two lines of the > file. > > My assumption is that it should either apply the full file and/or throw an > error. This just drops data silently. > > Yes people should not be editing patches by hand. This migration is part > of the effort to get them to stop doing that. > > Shouldn't git-am detect that the patch data and the meta data do not match > and warn the user or am I just being too damn picky here? > > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html CCing Paul who rewrote am recently. CCing Junio as he explained a similar issue to me once upon a time. Copying from a random mailing list submission: Subject: [PATCH 2/2] fsck: treat a NUL in a tag header as an error We check the return value of verify_header() for commits already, so do the same for tags as well. Signed-off-by: Rene Scharfe <l.s.r@xxxxxx> --- fsck.c | 3 ++- t/t1450-fsck.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/fsck.c b/fsck.c index e41e753..4060f1f 100644 --- a/fsck.c +++ b/fsck.c @@ -711,7 +711,8 @@ static int fsck_tag_buffer(struct tag *tag, const char *data, } } - if (verify_headers(buffer, size, &tag->object, options)) + ret = verify_headers(buffer, size, &tag->object, options); + if (ret) goto done; if (!skip_prefix(buffer, "object ", &buffer)) { diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 6c96953..e66b7cb 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -288,7 +288,7 @@ test_expect_success 'tag with bad tagger' ' grep "error in tag .*: invalid author/committer" out ' -test_expect_failure 'tag with NUL in header' ' +test_expect_success 'tag with NUL in header' ' sha=$(git rev-parse HEAD) && q_to_nul >tag-NUL-header <<-EOF && object $sha -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html Here we have 2 chunks, check where the second chunk ends (hint: it's at the line starting with --). One of the features of patches (as by git-am) is to ignore the footers, such as the git version or the mailing list addendum. You don't know how such a footer looks like (this one starts with -- but that's just coincidence, not by a defined protocol). So it's hard to specify in your case when the file ends and what is end-of-message stuff. It's dangerous if you're not aware of the pit fall. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html