Re: git-am applies half of a patch

From: Linus Torvalds <torvalds@osdl.org>
Date: Jan 9, 2007, 16:08 UTC
Message-ID: <Pine.LNX.4.64.0701090803070.3661@woody.osdl.org>
In-Reply-To: <7vtzz0mpzk.fsf@assigned-by-dhcp.cox.net>

Oh well, I should read my whole mbox ..

On Tue, 9 Jan 2007, Junio C Hamano wrote:

Show 12 quoted lines

>
> ---
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 1c35837..918822b 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -811,7 +811,7 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
>  			struct fragment dummy;
>  			if (parse_fragment_header(line, len, &dummy) < 0)
>  				continue;
> -			error("patch fragment without header at line %d: %.*s", linenr, (int)len-1, line);
> +			return error("patch fragment without header at line %d: %.*s", linenr, (int)len-1, line);

I don't think this really does anything, Junio.

Yes, it will now return an error value from "find_header()", but that's strictly normal: we _always_ get an error value from find_header() at the very end of a patch ("there's no more header").

And the caller (parse_chunk()) just ends up returning the error value, and the caller of THAT just ends up breaking out of the loop that does the patching, ie it does

	..
	while (size > 0) {
		..
		nr = parse_chunk(buffer + offset, size, patch);
		if (nr < 0)
			break;
		..

so you really need to make the "return error(..)" thing be a "die()" instead.

Alternatively, you could just say that _any_ garbage at the end of a patch is fatal, and make apply_patch() just die on a negative error from parse_chunk(), but garbage at the end of a patch is strictly normal (things like signatures or just even empty lines), so that wouldn't really work either.

In other words, I really don't think your patch does anything (except if the garbage is at the BEGINNING of a patch), and that you should take mine instead.

But test it.

		Linus