Odd issue with Git-am

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?

On Fri, Nov 20, 2015 at 1:02 PM,  <alan@clueserver.org> wrote:

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@web.de>
        ---
         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@vger.kernel.org
        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.