Re: [PATCH] apply: when -R, also reverse list of sections

From: Jonathan Tan <jonathantanmy@google.com>
Date: Oct 20, 2020, 19:12 UTC
Message-ID: <20201020191206.104924-1-jonathantanmy@google.com>
In-Reply-To: <xmqqy2ktk11t.fsf@gitster.c.googlers.com>

Show 15 quoted lines

> Jonathan Tan <jonathantanmy@google.com> writes:
> 
> > A patch changing a symlink into a file is written with 2 sections (in
> > the code, represented as "struct patch"): firstly, the deletion of the
> > symlink, and secondly, the creation of the file. When applying that
> > patch with -R, the sections are reversed, so we get:
> >
> >  (1) creation of a symlink, then
> >  (2) deletion of a file.
> 
> Good observation.
> 
> But I have to wonder if it breaks the support for (arguably outside
> the Git usecase) input that has more than one patch that touches the
> same path to blindly reverse the order of all patches

Sorry for getting back to this so late.

The only other case I can think of (besides symlink<->file) is directory<->file, and even in that case, I think blindly reversing the order still works.

If a more sophisticated rearrangement was needed, I would think that even applying the patches in the forward direction (that is, without "-R") wouldn't work, since Git is sensitive to the order of the patches. So I don't think we need to support such input (since they wouldn't work in the forward direction anyway).

Show 5 quoted lines

> (instead of
> the obvious implementation of the fix for the above stated problem
> --- i.e. make sure the first patch is a deletion of a symlink and
> what immediately follows is a creation of a regular file, and swap
> them only in such a case).

This would make patch application more robust, but I still appreciate the relative simplicity of the existing approach - the patches you see in the input are all applied in order (or reverse order, in the case of "-R"). You also wouldn't end up in the situation where you think that your input works because it works with your version of Git, but it actually doesn't work with an older version of Git or another implementation. (I haven't researched how other implementation do it, though.)