Re: git apply --directory broken for new files

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 12, 2008, 03:18 UTC
Message-ID: <7vk5ceijqo.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080927015422.GA31783@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 10 quoted lines

> apply --directory: handle creation and deletion patches
>
> We carefully verify that the input to git-apply is sane, including
> cross-checking that the filenames we see in "+++" headers match what was
> provided on the command line of "diff --git". When --directory is used,
> however, we ended up comparing the unadorned name to one with the
> prepended root, causing us to complain about a mismatch.
>
> We simply need to prepend the root directory, if any, when pulling the
> name out of the git header.

Thanks.

c4730f3 (Teach "git apply" to prepend a prefix with "--root=<root>", 2008-07-01) did a half-baked job to teach find_name() which is used to parse traditional diff and also is used to set patch->old_name and patch->new_name by gitdiff_verify_name() when parsing "copy from", "copy to", "rename from", and "rename to". The caller of git_header_name() uses the return value to set patch->def_name that is used when "deleted file" and "new file" are parsed, which should have been taught this trick by the same commit.

However,...

Show 18 quoted lines

> diff --git a/builtin-apply.c b/builtin-apply.c
> index 2ab4aba..f9070d5 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -787,6 +787,13 @@ static char *git_header_name(char *line, int llen)
>  					break;
>  			}
>  			if (second[len] == '\n' && !memcmp(name, second, len)) {
> +				if (root) {
> +					char *ret = xmalloc(root_len + len + 1);
> +					strcpy(ret, root);
> +					memcpy(ret + root_len, name, len);
> +					ret[root_len + len] = '\0';
> +					return ret;
> +				}
>  				return xmemdupz(name, len);
>  			}
>  		}

I suspect this is only half of the story, because the code to parse:

 	diff --git "a/f\244o/bar.c" "b/f\244o/bar.c"

in the same function before the part you patched needs similar treatment. There are two return of strbuf_detach(&first, NULL) in the if(){} block, and the return value needs to be prefixed with the value of --directory when given.

It would be easier to do this --directory prefixing in the sole caller of git_header_name(), though.