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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 30, 2015, 20:11 UTC
Message-ID: <xmqq1tmcc9l9.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqq61bocao1.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 33 quoted lines

> Jeff King <peff@peff.net> 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.