Subject: Re: [PATCH] apply: refuse touching a file beyond symlink

From: Junio C Hamano <[email-masked]>
Date: Fri, 30 Jan 2015 12:11:30 -0800

Junio C Hamano <[email-masked]> writes:

> Jeff King <[email-masked]> writes:
>
>>> +	if (!patch->is_delete && path_is_beyond_symlink(patch->new_name))
>>> +		return error(_("affected file '%s' is beyond a symbolic link"),
>>> +			     patch->new_name);
>>
>> Why does this not kick in when deleting a file? If it is not OK to
>> add across a symlink, why is it OK to delete?
>
> Hmph, adding
>
> 	if (patch->is_delete &&	path_is_beyond_symlink(patch->old_name))
> 		return error(_("deleted file '%s' is beyond a symlink"),
> 				patch->old_name);
>
> seems to break t4114.11, which wants to apply this patch to a tree
> that does not have a symbolic link but a directory at 'foo/'.
>
> diff --git a/foo b/foo
> new file mode 120000
> index 0000000..ba0e162
> --- /dev/null
> +++ b/foo
> @@ -0,0 +1 @@
> +bar
> \ No newline at end of file
> diff --git a/foo/baz b/foo/baz
> deleted file mode 100644
> index 682c76b..0000000
> --- a/foo/baz
> +++ /dev/null
> @@ -1 +0,0 @@
> -if only I knew

I am not sure how to fix this, without completely ripping out the
misguided "We should be able to concatenate outputs from multiple
invocations of 'git diff' into a single file and apply the result
with a single invocation of 'git apply'" change I grudgingly
accepted long time ago (7a07841c (git-apply: handle a patch that
touches the same path more than once better, 2008-06-27).

"git diff" output is designed each patch to apply independently to
the preimage to produce the postimage, and that allows patches to
two files can be swapped via -Oorderfile mechanism, and also "X was
created by copying from Y and Y is modified in place" will result in
X with the contents of Y in the preimage (i.e. before the in-place
modification of Y in the same patch) regardless of the order of X
and Y in the "git diff" output.  The above input used by t4114.11
expects to remove 'foo/baz' (leaving an empty directory foo as an
result but we do not track directories so it can be nuked to make
room if other patch in the same input wants to put something else,
either a regular file or a symbolic link, there) and create a blob
at 'foo', and such an input should apply regardless of the order of
patches in it.

The in_fn_table[] stuff broke that design completely.