Re: git add -p breaks after split on change at the top of the file

From: Jeff King <peff@peff.net>
Date: Aug 17, 2017, 09:15 UTC
Message-ID: <20170817091503.vsd4lncogphy5gxx@sigill.intra.peff.net>
In-Reply-To: <20170817090308.yh3jnztgfmkwtqpr@sigill.intra.peff.net>

On Thu, Aug 17, 2017 at 05:03:08AM -0400, Jeff King wrote:

Show 9 quoted lines

> But that does the opposite of what we want: it makes this case work when
> --allow-overlap isn't specified. I think my first attempt is probably
> closer to the right direction (but we'd want to have it kick in only
> when --allow-overlap is specified; well formed patches shouldn't overlap
> but we'd want to barf loudly if they do).
> 
> I'll stop here for now before digging any further. I'm not all that
> familiar with the apply code, so I have a feeling Junio's comments may
> stop me from going down an unproductive dead end. :)

OK, last email, I promise. :)

My first attempt which turns off match_beginning for the overlapping hunk really does feel wrong. It would blindly match a similar hunk later in the file. So if you did something like:

  1. The first hunk deletes context line "x".

  2. The second overlapping hunk claims to start at the beginning of the
     file (line 1), and then adds a line after "x".

  3. Later in the file, we have another line "x".

Then the second hunk should be rejected, and not find the unrelated "x" from (3). But with my patch, I suspect it would be found (because we dropped the requirement to find it at the beginning).

Of course this is a pretty ridiculous input in the first place. In theory it _could_ be figured out, but overlapping hunks like this are always going to cause problems (in this case, context from the second hunk became a removal, and the second hunk no longer applies).

So maybe it's not worth caring about. But I think the most robust solution would involve checking the lines between try_lno and the start to make sure they were all added. I just don't know that we have an easy way of checking that. Perhaps if we recorded the cumulative number of lines added in previous hunks, and used that as our starting point (instead of "line 0").

I also suspect that match_end has a similar problem, but I couldn't trigger it in practice (perhaps it's impossible, or perhaps my brain is just not up to the challenge today).

-Peff