Patchwork [7,of,8] merge: check for dir rename dest before adding ACTION_KEEP

login
register
mail settings
Submitter Pulkit Goyal
Date Sept. 14, 2020, 11:15 a.m.
Message ID <a6378fd3a90effdd150f.1600082123@workspace>
Download mbox | patch
Permalink /patch/47161/
State Accepted
Headers show

Comments

Pulkit Goyal - Sept. 14, 2020, 11:15 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1599034669 -19800
#      Wed Sep 02 13:47:49 2020 +0530
# Node ID a6378fd3a90effdd150f94f4da8266687276e0e4
# Parent  21f520670eb08917eeb5fcf11c75cc1dfb242530
# EXP-Topic merge-newnode
merge: check for dir rename dest before adding ACTION_KEEP

A previous patch in the series blindly uses `ACTION_KEEP` if the file is not
present on both remote and ancestor. This was wrong.

We tries to detect directory renames and in some graft cases, it can be possible
that file is not present on both sides but is created in rename destination of
other directory which exists on remote. In such cases, we need to merge the
rename source from remote with rename dest from local.

This patch makes sure we checks for such rename cases before falling back to
`ACTION_KEEP`.

Also, we moved the adding of action for the destination file while processing
the destination file which makes things much simpler as an action for file will
only be added while processing it. And we added a `ACTION_KEEP_ABSENT` for the
rename source because otherwise an action for that was missing.

Differential Revision: https://phab.mercurial-scm.org/D8977
Yuya Nishihara - Sept. 16, 2020, 11:33 a.m.
On Mon, 14 Sep 2020 16:45:23 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1599034669 -19800
> #      Wed Sep 02 13:47:49 2020 +0530
> # Node ID a6378fd3a90effdd150f94f4da8266687276e0e4
> # Parent  21f520670eb08917eeb5fcf11c75cc1dfb242530
> # EXP-Topic merge-newnode
> merge: check for dir rename dest before adding ACTION_KEEP

Apparently this breaks test-graft-rename.t. I've queued first 6 patches.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -933,12 +933,41 @@  def manifestmerge(
                         f, mergestatemod.ACTION_REMOVE, None, b'other deleted',
                     )
             else:  # file not in ancestor, not in remote
-                mresult.addfile(
-                    f,
-                    mergestatemod.ACTION_KEEP,
-                    None,
-                    b'ancestor missing, remote missing',
-                )
+                rename_found = False
+                for source, dest in branch_copies1.dirmove.items():
+                    if f.startswith(dest):
+                        # this directory in which this file is created
+                        # is moved from somewhere else
+                        # we should check if the source file exists on
+                        # remote side
+                        sf = source + f[len(dest) :]
+                        if sf in m2:
+                            rename_found = True
+                            if m2[sf] == ma[sf]:
+                                mresult.addfile(
+                                    f,
+                                    mergestatemod.ACTION_KEEP,
+                                    None,
+                                    b'file is rename dest and rename source'
+                                    b' did not change',
+                                )
+                            else:
+                                mresult.addfile(
+                                    f,
+                                    mergestatemod.ACTION_MERGE,
+                                    (f, sf, sf, False, pa.node()),
+                                    b'local directory rename - respect move '
+                                    b'from %s' % sf,
+                                )
+                            break
+
+                if not rename_found:
+                    mresult.addfile(
+                        f,
+                        mergestatemod.ACTION_KEEP,
+                        None,
+                        b'ancestor missing, remote missing',
+                    )
 
         elif n2:  # file exists only on remote side
             if f in copied1:
@@ -1010,12 +1039,16 @@  def manifestmerge(
                         df = branch_copies1.dirmove[d] + f[len(d) :]
                         break
                 if df is not None and df in m1:
+                    # this file is not present in local however its a source
+                    # of rename for another file. keep this file not
+                    # present in local. Action for rename dest is written
+                    # while processing it
                     mresult.addfile(
-                        df,
-                        mergestatemod.ACTION_MERGE,
-                        (df, f, f, False, pa.node()),
-                        b'local directory rename - respect move '
-                        b'from %s' % f,
+                        f,
+                        mergestatemod.ACTION_KEEP_ABSENT,
+                        None,
+                        b'remote directory is rename source - respect move '
+                        b'to %s' % df,
                     )
                 elif acceptremote:
                     mresult.addfile(