Re: Odd issue with Git-am

[Stefan Beller <sbeller@google.com>]

On Fri, Nov 20, 2015 at 1:02 PM,  <alan@clueserver.org> 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@vger.kernel.org
> 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@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.