Re: Optimizing writes to unchanged files during merges?

From: Elijah Newren <newren@gmail.com>
Date: Apr 16, 2018, 23:03 UTC
Message-ID: <CABPp-BHa47a-nH6dZ+LdGuf2XdYTFHyhHVv39dGeuEux10DY_A@mail.gmail.com>
In-Reply-To: <xmqqbmekylgc.fsf@gitster-ct.c.googlers.com>

On Sun, Apr 15, 2018 at 6:44 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

> One thing that makes me curious is what happens (and what we want to
> happen) when such a "we already have the changes the side branch
> tries to bring in" path has local (i.e. not yet in the index)
> changes.  For a dirty file that trivially merges (e.g. a path we
> modified since our histories forked, while the other side didn't do
> anything, has local changes in the working tree), we try hard to
> make the merge succeed while keeping the local changes, and we
> should be able to do the same in this case, too.
>
> Your illustration patch that reads from the working tree to compare
> with the contents we are about to write out of course won't cope
> with this case ;-).  If we do things in the index like the approach
> to fix was_tracked(), I suspect that we would just say "as long as
> the index and the HEAD matches, i.e. we are keeping the path as it is
> found in HEAD as the merge result, we do not touch the working tree"
> and leave such a local modification alone.

I could see it going either way (fail early, or succeed because we don't need to overwrite anything), but my suspicion is also towards letting the merge succeed.

It looks like my patches need a little more fixing up (the was_dirty() calls would be mixing both the old and the new index -- it used the original index via was_tracked(), but used the current index via cache_file_exists(); it should be more consistent). I'm fixing that up, will add this as another testcase to t6046, and will submit a newer RFC in a day or so.