Odd issue with Git-am

Odd issue with Git-am

[alan]

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?

Re: Odd issue with Git-am

[Stefan Beller]

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.