Re: [PATCH v2 4/4] apply: do not touch a file beyond a symbolic link

2015-02-03 Thread Eric Sunshine
On Tue, Feb 3, 2015 at 4:01 PM, Junio C Hamano wrote: > Jeff King writes: >> diff --git a/t/t4122-apply-symlink-inside.sh >> b/t/t4122-apply-symlink-inside.sh >> index 942c5cb..fbba8dd 100755 >> --- a/t/t4122-apply-symlink-inside.sh >> +++ b/t/t4122-apply-symlink-inside.sh >> @@ -112,6 +113,20 @

Re: [PATCH v2 4/4] apply: do not touch a file beyond a symbolic link

2015-02-03 Thread Junio C Hamano
Jeff King writes: > Here's the test addition I came up with, because it didn't look like we > were covering this case. Thanks. > diff --git a/t/t4122-apply-symlink-inside.sh b/t/t4122-apply-symlink-inside.sh > index 942c5cb..fbba8dd 100755 > --- a/t/t4122-apply-symlink-inside.sh > +++ b/t/t412

Re: [PATCH v2 4/4] apply: do not touch a file beyond a symbolic link

2015-02-02 Thread Jeff King
On Mon, Feb 02, 2015 at 05:56:55PM -0800, Junio C Hamano wrote: > > I think this means we'll be > > overly cautious with a patch that does: > > > > 1. add foo as a symlink > > > > 2. remove foo > > > > 3. add foo/bar > > > > which is perfectly OK > > No, such a patchset is broken. > > A va

Re: [PATCH v2 4/4] apply: do not touch a file beyond a symbolic link

2015-02-02 Thread Junio C Hamano
Jeff King writes: > On Mon, Feb 02, 2015 at 03:27:30PM -0800, Junio C Hamano wrote: > >> +static struct string_list symlink_changes; > > I notice we don't duplicate strings for this list. Are the paths we > register here always stable? I think they should be, as they are part of > the "struct pat

Re: [PATCH v2 4/4] apply: do not touch a file beyond a symbolic link

2015-02-02 Thread Jeff King
On Mon, Feb 02, 2015 at 03:27:30PM -0800, Junio C Hamano wrote: > +static struct string_list symlink_changes; I notice we don't duplicate strings for this list. Are the paths we register here always stable? I think they should be, as they are part of the "struct patch". > +#define SYMLINK_GOES_A

[PATCH v2 4/4] apply: do not touch a file beyond a symbolic link

2015-02-02 Thread Junio C Hamano
Because Git tracks symbolic links as symbolic links, a path that has a symbolic link in its leading part (e.g. path/to/dir/file, where path/to/dir is a symbolic link to somewhere else, be it inside or outside the working tree) can never appear in a patch that validly applies, unless the same patch