Re: [PATCH] Add a test-case for git-apply trying to add an ending line

[Junio C Hamano <junkio@xxxxxxx>]

Hmmmmm.

To git-apply, this patch:

        diff --git a/file b/file
        --- a/file
        +++ b/file
        @@ -1,2 +1,3 @@
         a
         b
        +c

currently means "I want to append a line c immediately after the
lines that have a and then b".  Nothing else.  And applying it
to

	a
        b
        c

produces

	a
        b
        c
        c

The first c is what your patch added, and the second c is what
was originally there.

I do not think this is necessarily a bug.  You _could_ make the
EOF a special case (i.e. you _could_ interpret the patch that it
also says, with "@@ -1,2", that "the result of this patch _must_
end with this line I just added"), and if you are going to do
that, you would also need a symmetric special case for the
beginning of the file, but I do not think it is the right thing
to do in general.

Realistically, you would have something like this:

        diff --git a/apply.c b/apply.c
        index 0ed9d13..f99c6fe 100644
        --- a/apply.c
        +++ b/apply.c
        @@ -2297,3 +2297,8 @@ int main(int argc, char **argv)

                return 0; /* end of main */
         }
        +
        +static void this_function_is_unused(void)
        +{
        +	printf("hello, world\n");
        +}

You added a useless function at the end of the file.

While you prepared the above patch, the upstream updated the
same file to end like this:

                return 0; /* end of main */
        }

        static int some_new_function(void)
        {
                return 314;
        }

Now, git-apply would produce this file if you apply the above
patch:

                return 0; /* end of main */
        }

        static void this_function_is_unused(void)
        {
                printf("hello, world\n");
        }

        static int some_new_function(void)
        {
                return 314;
        }

I think this current behaviour is more desirable than special
casing the end of file and refusing to apply.

In this particular case, expecting failure like your new test
does is somewhat understandable, but if you change the test case
to start with this file, you would realize that your expectation
is not the only valid understanding of what is really happening:

	echo 'a' >file
	echo 'b' >>file
	echo 'd' >>file

Applying test-patch to this would result in

	a
        b
        c
        d

which I think is more useful behaviour.

-
: 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