Re: [PATCH v2 1/4] apply: reject input that touches outside $cwd

[Jeff King <peff@peff.net>]

On Mon, Feb 02, 2015 at 03:27:27PM -0800, Junio C Hamano wrote:

> By default, a patch that affects outside the working area is
> rejected as a mistake (or a mischief); Git itself does not create
> such a patch, unless the user bends backwards and specifies a
> non-standard prefix to "git diff" and friends.
> 
> When `git apply` is used without either `--index` or `--cached`
> option as a "better GNU patch", the user can pass `--unsafe-paths`
> option to override this safety check.  This cannot be used to escape
> outside the working tree when using `--index` or `--cached` to apply
> the patch to the index.
> 
> The new test was stolen from Jeff King with slight enhancements.

I notice that this includes the symlink-crossing tests, marked as
failures. Reading the series, I know what is going to happen later, but
do you want to leave a note like:

  Note that we also add tests for leaving the working directory by
  crossing symlink boundaries, which is not addressed in this patch.
  That is a separate issue caused following symlinks, which will come
  later.

or something to help later readers of "git log"?

> +--unsafe-paths::
> +	By default, a patch that affects outside the working area is
> +	rejected as a mistake (or a mischief); Git itself never
> +	creates such a patch unless the user bends backwards and
> +	specifies nonstandard prefix to "git diff" and friends.

Minor wordsmithing: the usual idiom is "bend over backwards", and you
probably want s/specifies/& a/.

> ++
> +When `git apply` is used without either `--index` or `--cached`
> +option as a "better GNU patch", the user can pass `--unsafe-paths`
> +option to override this safety check.

Similarly, probably every instance of "foo option" would read better as
"the foo option".

> This cannot be used to escape
> +outside the working tree when using `--index` or `--cached` to apply
> +the patch to the index.

I had trouble figuring out what this meant. Would it be simpler to just
say:

  This option has no effect when `--index` or `--cached` is in use.

Or is there some other subtlety that you are trying to convey that I am
missing?

-Peff