Re: Optimizing writes to unchanged files during merges?

[Linus Torvalds <torvalds@linux-foundation.org>]

[ Talking to myself ]

On Thu, Apr 12, 2018 at 4:41 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Oddly, that *already* has the check:
>
>         if (mfi.clean && !df_conflict_remains &&
>             oid_eq(&mfi.oid, a_oid) && mfi.mode == a_mode) {
>                 int path_renamed_outside_HEAD;
>                 output(o, 3, _("Skipped %s (merged same as existing)"), path);
>
> but that doesn't seem to actually trigger for some reason.

Actually, that check triggers just fine.

> But the code really seems to have the _intention_ of skipping the case
> where the result ended up the same as the source.
>
> Maybe I'm missing something.

The later check that does

                /*
                 * The content merge resulted in the same file contents we
                 * already had.  We can return early if those file contents
                 * are recorded at the correct path (which may not be true
                 * if the merge involves a rename).
                 */
                path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
                if (!path_renamed_outside_HEAD) {

will see that 'path2' is NULL, and not trigger this early out case,
and then this all falls back to the normal cases after all.

So I think that 'path_renamed_outside_HEAD' logic is somehow broken.

Did it perhaps mean to say

                path_renamed_outside_HEAD = path2 && !strcmp(path, path2);

instead?

See commit 5b448b853 ("merge-recursive: When we detect we can skip an
update, actually skip it") which really implies we want to actually
skip it (but then we don't anyway).

Also see commit b2c8c0a76 ("merge-recursive: When we detect we can
skip an update, actually skip it") which was an earlier version, and
which *actually* skipped it, but it was reverted because of that
rename issue.

Adding Elijah Newren to the cc, because he was working on this back
then, an dis still around, and still working on merging ;)

               Linus