[PATCH 01 of 13] mergestate: rename addpath() -> addpathonflict() to prevent confusion

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 01 of 13] mergestate: rename addpath() -> addpathonflict() to prevent confusion

Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594286098 -19800
#      Thu Jul 09 14:44:58 2020 +0530
# Node ID 66fb1ff9343a4d73b44c4314d88176e283a7839d
# Parent  33524b6bef53fffcb34708e927c2b53f08fea736
# EXP-Topic mergestate-refactor
mergestate: rename addpath() -> addpathonflict() to prevent confusion

addpath() seems to imply that we are adding a new path/entry to the mergestate.

Differential Revision: https://phab.mercurial-scm.org/D8715

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1264,7 +1264,7 @@ def applyupdates(
         else:
             s(_(b"the remote file has been renamed to %s\n") % f1)
         s(_(b"resolve manually then use 'hg resolve --mark %s'\n") % f)
-        ms.addpath(f, f1, fo)
+        ms.addpathconflict(f, f1, fo)
         progress.increment(item=f)
 
     # When merging in-memory, we can't support worker processes, so set the
diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -540,7 +540,7 @@ class mergestate(object):
         self._stateextras[fd] = {b'ancestorlinknode': hex(fca.node())}
         self._dirty = True
 
-    def addpath(self, path, frename, forigin):
+    def addpathconflict(self, path, frename, forigin):
         """add a new conflicting path to the merge state
         path:    the path that conflicts
         frename: the filename the conflicting file was renamed to

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 02 of 13] mergestate: remove unrequired RECORD_RESOLVED_OTHER record

Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594290002 -19800
#      Thu Jul 09 15:50:02 2020 +0530
# Node ID 9e4a5e41f0ac3fa39a29d5376a1d3922e9221318
# Parent  66fb1ff9343a4d73b44c4314d88176e283a7839d
# EXP-Topic mergestate-refactor
mergestate: remove unrequired RECORD_RESOLVED_OTHER record

This was introduced in last cycle however while working on refactoring
mergestate, I realized it's unncessary.

This will break users who did a merge using previous version, did this kind of
storage and before commiting updated the mercurial version.

Differential Revision: https://phab.mercurial-scm.org/D8716

diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -51,7 +51,6 @@ RECORD_LABELS = b'l'
 RECORD_OVERRIDE = b't'
 RECORD_UNSUPPORTED_MANDATORY = b'X'
 RECORD_UNSUPPORTED_ADVISORY = b'x'
-RECORD_RESOLVED_OTHER = b'R'
 
 MERGE_DRIVER_STATE_UNMARKED = b'u'
 MERGE_DRIVER_STATE_MARKED = b'm'
@@ -220,7 +219,6 @@ class mergestate(object):
                 RECORD_CHANGEDELETE_CONFLICT,
                 RECORD_PATH_CONFLICT,
                 RECORD_MERGE_DRIVER_MERGE,
-                RECORD_RESOLVED_OTHER,
             ):
                 bits = record.split(b'\0')
                 self._state[bits[0]] = bits[1:]
@@ -448,9 +446,7 @@ class mergestate(object):
                     (RECORD_PATH_CONFLICT, b'\0'.join([filename] + v))
                 )
             elif v[0] == MERGE_RECORD_MERGED_OTHER:
-                records.append(
-                    (RECORD_RESOLVED_OTHER, b'\0'.join([filename] + v))
-                )
+                records.append((RECORD_MERGED, b'\0'.join([filename] + v)))
             elif v[1] == nullhex or v[6] == nullhex:
                 # Change/Delete or Delete/Change conflicts. These are stored in
                 # 'C' records. v[1] is the local file, and is nullhex when the

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 03 of 13] mergestate: add comments about couple of record types and minor reorder

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594292904 -19800
#      Thu Jul 09 16:38:24 2020 +0530
# Node ID ed6b80e5ed265851feb59d7855641b5d5254953b
# Parent  9e4a5e41f0ac3fa39a29d5376a1d3922e9221318
# EXP-Topic mergestate-refactor
mergestate: add comments about couple of record types and minor reorder

I am trying to divide the records into certain groups and then have dedicated
objects for them. Taking baby steps in that direction.

Differential Revision: https://phab.mercurial-scm.org/D8717

diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -41,13 +41,17 @@ def _filectxorabsent(hexnode, ctx, f):
 # Merge state record types. See ``mergestate`` docs for more.
 RECORD_LOCAL = b'L'
 RECORD_OTHER = b'O'
+# record extra information about files
+RECORD_FILE_VALUES = b'f'
+# record merge labels
+RECORD_LABELS = b'l'
+
 RECORD_MERGED = b'F'
 RECORD_CHANGEDELETE_CONFLICT = b'C'
 RECORD_MERGE_DRIVER_MERGE = b'D'
 RECORD_PATH_CONFLICT = b'P'
+
 RECORD_MERGE_DRIVER_STATE = b'm'
-RECORD_FILE_VALUES = b'f'
-RECORD_LABELS = b'l'
 RECORD_OVERRIDE = b't'
 RECORD_UNSUPPORTED_MANDATORY = b'X'
 RECORD_UNSUPPORTED_ADVISORY = b'x'

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 04 of 13] mergestate: remove unused unsupported related mergestate records

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594292982 -19800
#      Thu Jul 09 16:39:42 2020 +0530
# Node ID 6879b715405d2ff35deb35eac97104bb82c1aff2
# Parent  ed6b80e5ed265851feb59d7855641b5d5254953b
# EXP-Topic mergestate-refactor
mergestate: remove unused unsupported related mergestate records

I tried to find users of this but was unable to find. Seems like RECORD_OVERRIDE
is doing for what they were used before.

Differential Revision: https://phab.mercurial-scm.org/D8718

diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -53,8 +53,6 @@ RECORD_PATH_CONFLICT = b'P'
 
 RECORD_MERGE_DRIVER_STATE = b'm'
 RECORD_OVERRIDE = b't'
-RECORD_UNSUPPORTED_MANDATORY = b'X'
-RECORD_UNSUPPORTED_ADVISORY = b'x'
 
 MERGE_DRIVER_STATE_UNMARKED = b'u'
 MERGE_DRIVER_STATE_MARKED = b'm'
@@ -115,8 +113,6 @@ class mergestate(object):
     m: the external merge driver defined for this merge plus its run state
        (experimental)
     f: a (filename, dictionary) tuple of optional values for a given file
-    X: unsupported mandatory record type (used in tests)
-    x: unsupported advisory record type (used in tests)
     l: the labels for the parts of the merge.
 
     Merge driver run states (experimental):

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 05 of 13] mergestate: document mergestate records in an organized way

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594294541 -19800
#      Thu Jul 09 17:05:41 2020 +0530
# Node ID c571d56248d51c7ed396871495e9ddc989a8ce45
# Parent  6879b715405d2ff35deb35eac97104bb82c1aff2
# EXP-Topic mergestate-refactor
mergestate: document mergestate records in an organized way

This makes clear which mergestate record is used for what and group them based
on how they are used right now.

Differential Revision: https://phab.mercurial-scm.org/D8719

diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -39,25 +39,40 @@ def _filectxorabsent(hexnode, ctx, f):
 
 
 # Merge state record types. See ``mergestate`` docs for more.
+
+####
+# merge records which records metadata about a current merge
+# exists only once in a mergestate
+#####
 RECORD_LOCAL = b'L'
 RECORD_OTHER = b'O'
-# record extra information about files
-RECORD_FILE_VALUES = b'f'
 # record merge labels
 RECORD_LABELS = b'l'
+# store info about merge driver used and it's state
+RECORD_MERGE_DRIVER_STATE = b'm'
 
+#####
+# record extra information about files, with one entry containing info about one
+# file. Hence, multiple of them can exists
+#####
+RECORD_FILE_VALUES = b'f'
+
+#####
+# merge records which represents state of individual merges of files/folders
+# These are top level records for each entry containing merge related info.
+# Each record of these has info about one file. Hence multiple of them can
+# exists
+#####
 RECORD_MERGED = b'F'
 RECORD_CHANGEDELETE_CONFLICT = b'C'
 RECORD_MERGE_DRIVER_MERGE = b'D'
+# the path was dir on one side of merge and file on another
 RECORD_PATH_CONFLICT = b'P'
 
-RECORD_MERGE_DRIVER_STATE = b'm'
-RECORD_OVERRIDE = b't'
-
-MERGE_DRIVER_STATE_UNMARKED = b'u'
-MERGE_DRIVER_STATE_MARKED = b'm'
-MERGE_DRIVER_STATE_SUCCESS = b's'
-
+#####
+# possible state which a merge entry can have. These are stored inside top-level
+# merge records mentioned just above.
+#####
 MERGE_RECORD_UNRESOLVED = b'u'
 MERGE_RECORD_RESOLVED = b'r'
 MERGE_RECORD_UNRESOLVED_PATH = b'pu'
@@ -67,6 +82,21 @@ MERGE_RECORD_DRIVER_RESOLVED = b'd'
 # of other version. This info is used on commit.
 MERGE_RECORD_MERGED_OTHER = b'o'
 
+#####
+# top level record which stores other unknown records. Multiple of these can
+# exists
+#####
+RECORD_OVERRIDE = b't'
+
+#####
+# possible states which a merge driver can have. These are stored inside a
+# RECORD_MERGE_DRIVER_STATE entry
+#####
+MERGE_DRIVER_STATE_UNMARKED = b'u'
+MERGE_DRIVER_STATE_MARKED = b'm'
+MERGE_DRIVER_STATE_SUCCESS = b's'
+
+
 ACTION_FORGET = b'f'
 ACTION_REMOVE = b'r'
 ACTION_ADD = b'a'

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 06 of 13] mergestate: remove unnecessary recordactions() from mergestate class

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594299054 -19800
#      Thu Jul 09 18:20:54 2020 +0530
# Node ID 8dd9eeb56cd015f1e323fedad6c8d58de2d81bb1
# Parent  c571d56248d51c7ed396871495e9ddc989a8ce45
# EXP-Topic mergestate-refactor
mergestate: remove unnecessary recordactions() from mergestate class

This function is updating dirstate which sounds like not something which a
method on mergestate class should do. Also this just calls another function.
Lets directly call that function and remove this reducing mergestate
responsibility a bit.

There was single caller which is updated.

Differential Revision: https://phab.mercurial-scm.org/D8737

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -6129,7 +6129,8 @@ def resolve(ui, repo, *pats, **opts):
                     raise
 
         ms.commit()
-        ms.recordactions()
+        branchmerge = repo.dirstate.p2() != nullid
+        mergestatemod.recordupdates(repo, ms.actions(), branchmerge, None)
 
         if not didwork and pats:
             hint = None
diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -756,11 +756,6 @@ class mergestate(object):
                 actions[action].append((f, None, b"merge result"))
         return actions
 
-    def recordactions(self):
-        """record remove/add/get actions in the dirstate"""
-        branchmerge = self._repo.dirstate.p2() != nullid
-        recordupdates(self._repo, self.actions(), branchmerge, None)
-
     def queueremove(self, f):
         """queues a file to be removed from the dirstate
 

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 07 of 13] mergestate: rename a helpless variable name to bit helpful one

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594300089 -19800
#      Thu Jul 09 18:38:09 2020 +0530
# Node ID 7265d8b575ff4f3adfc392247f868344eadd43c2
# Parent  8dd9eeb56cd015f1e323fedad6c8d58de2d81bb1
# EXP-Topic mergestate-refactor
mergestate: rename a helpless variable name to bit helpful one

The old variable name `r` makes it ~impossible to understand what does it mean.
One can only understand that after going to callers and hoping that its
documented there.

I also documented return value of the function involved while I was there.

Differential Revision: https://phab.mercurial-scm.org/D8738

diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -619,7 +619,10 @@ class mergestate(object):
         return self._stateextras.setdefault(filename, {})
 
     def _resolve(self, preresolve, dfile, wctx):
-        """rerun merge process for file path `dfile`"""
+        """rerun merge process for file path `dfile`.
+        Returns whether the merge was completed and the return value of merge
+        obtained from filemerge._filemerge().
+        """
         if self[dfile] in (MERGE_RECORD_RESOLVED, MERGE_RECORD_DRIVER_RESOLVED):
             return True, 0
         if self._state[dfile][0] == MERGE_RECORD_MERGED_OTHER:
@@ -660,7 +663,7 @@ class mergestate(object):
                 f.close()
             else:
                 wctx[dfile].remove(ignoremissing=True)
-            complete, r, deleted = filemerge.premerge(
+            complete, merge_ret, deleted = filemerge.premerge(
                 self._repo,
                 wctx,
                 self._local,
@@ -671,7 +674,7 @@ class mergestate(object):
                 labels=self._labels,
             )
         else:
-            complete, r, deleted = filemerge.filemerge(
+            complete, merge_ret, deleted = filemerge.filemerge(
                 self._repo,
                 wctx,
                 self._local,
@@ -681,12 +684,12 @@ class mergestate(object):
                 fca,
                 labels=self._labels,
             )
-        if r is None:
-            # no real conflict
+        if merge_ret is None:
+            # If return value of merge is None, then there are no real conflict
             del self._state[dfile]
             self._stateextras.pop(dfile, None)
             self._dirty = True
-        elif not r:
+        elif not merge_ret:
             self.mark(dfile, MERGE_RECORD_RESOLVED)
 
         if complete:
@@ -708,9 +711,9 @@ class mergestate(object):
                     else:
                         action = ACTION_ADD
                 # else: regular merges (no action necessary)
-            self._results[dfile] = r, action
+            self._results[dfile] = merge_ret, action
 
-        return complete, r
+        return complete, merge_ret
 
     def preresolve(self, dfile, wctx):
         """run premerge process for dfile

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 08 of 13] mergestate: document what mergestate._results is for

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594300418 -19800
#      Thu Jul 09 18:43:38 2020 +0530
# Node ID 3e356397d1b4f0b07465fab86045a38d59029067
# Parent  7265d8b575ff4f3adfc392247f868344eadd43c2
# EXP-Topic mergestate-refactor
mergestate: document what mergestate._results is for

Understanding that dict is important for understanding how mergestate is
performing operations on dirstate.

Differential Revision: https://phab.mercurial-scm.org/D8739

diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -267,6 +267,11 @@ class mergestate(object):
                 self._labels = [l for l in labels if len(l) > 0]
             elif not rtype.islower():
                 unsupported.add(rtype)
+        # contains a mapping of form:
+        # {filename : (merge_return_value, action_to_be_performed}
+        # these are results of re-running merge process
+        # this dict is used to perform actions on dirstate caused by re-running
+        # the merge
         self._results = {}
         self._dirty = False
 

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 09 of 13] merge: refactor code to advise fsmonitor in separate function

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594301530 -19800
#      Thu Jul 09 19:02:10 2020 +0530
# Node ID 15d0b77c5833af7c1b04e29dd4628a66d9518cde
# Parent  3e356397d1b4f0b07465fab86045a38d59029067
# EXP-Topic mergestate-refactor
merge: refactor code to advise fsmonitor in separate function

merge.update() is quite hard to understand, found this an easy win.

The end goal is to have better organized merge and mergestate handling and then
fix some related bugs.

Differential Revision: https://phab.mercurial-scm.org/D8740

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1477,6 +1477,49 @@ def applyupdates(
     return updateresult(updated, merged, removed, unresolved), getfiledata
 
 
+def _advertisefsmonitor(repo, num_gets, p1node):
+    # Advertise fsmonitor when its presence could be useful.
+    #
+    # We only advertise when performing an update from an empty working
+    # directory. This typically only occurs during initial clone.
+    #
+    # We give users a mechanism to disable the warning in case it is
+    # annoying.
+    #
+    # We only allow on Linux and MacOS because that's where fsmonitor is
+    # considered stable.
+    fsmonitorwarning = repo.ui.configbool(b'fsmonitor', b'warn_when_unused')
+    fsmonitorthreshold = repo.ui.configint(
+        b'fsmonitor', b'warn_update_file_count'
+    )
+    try:
+        # avoid cycle: extensions -> cmdutil -> merge
+        from . import extensions
+
+        extensions.find(b'fsmonitor')
+        fsmonitorenabled = repo.ui.config(b'fsmonitor', b'mode') != b'off'
+        # We intentionally don't look at whether fsmonitor has disabled
+        # itself because a) fsmonitor may have already printed a warning
+        # b) we only care about the config state here.
+    except KeyError:
+        fsmonitorenabled = False
+
+    if (
+        fsmonitorwarning
+        and not fsmonitorenabled
+        and p1node == nullid
+        and num_gets >= fsmonitorthreshold
+        and pycompat.sysplatform.startswith((b'linux', b'darwin'))
+    ):
+        repo.ui.warn(
+            _(
+                b'(warning: large working directory being used without '
+                b'fsmonitor enabled; enable fsmonitor to improve performance; '
+                b'see "hg help -e fsmonitor")\n'
+            )
+        )
+
+
 UPDATECHECK_ABORT = b'abort'  # handled at higher layers
 UPDATECHECK_NONE = b'none'
 UPDATECHECK_LINEAR = b'linear'
@@ -1815,46 +1858,9 @@ def update(
             # note that we're in the middle of an update
             repo.vfs.write(b'updatestate', p2.hex())
 
-        # Advertise fsmonitor when its presence could be useful.
-        #
-        # We only advertise when performing an update from an empty working
-        # directory. This typically only occurs during initial clone.
-        #
-        # We give users a mechanism to disable the warning in case it is
-        # annoying.
-        #
-        # We only allow on Linux and MacOS because that's where fsmonitor is
-        # considered stable.
-        fsmonitorwarning = repo.ui.configbool(b'fsmonitor', b'warn_when_unused')
-        fsmonitorthreshold = repo.ui.configint(
-            b'fsmonitor', b'warn_update_file_count'
+        _advertisefsmonitor(
+            repo, len(actions[mergestatemod.ACTION_GET]), p1.node()
         )
-        try:
-            # avoid cycle: extensions -> cmdutil -> merge
-            from . import extensions
-
-            extensions.find(b'fsmonitor')
-            fsmonitorenabled = repo.ui.config(b'fsmonitor', b'mode') != b'off'
-            # We intentionally don't look at whether fsmonitor has disabled
-            # itself because a) fsmonitor may have already printed a warning
-            # b) we only care about the config state here.
-        except KeyError:
-            fsmonitorenabled = False
-
-        if (
-            fsmonitorwarning
-            and not fsmonitorenabled
-            and p1.node() == nullid
-            and len(actions[mergestatemod.ACTION_GET]) >= fsmonitorthreshold
-            and pycompat.sysplatform.startswith((b'linux', b'darwin'))
-        ):
-            repo.ui.warn(
-                _(
-                    b'(warning: large working directory being used without '
-                    b'fsmonitor enabled; enable fsmonitor to improve performance; '
-                    b'see "hg help -e fsmonitor")\n'
-                )
-            )
 
         wantfiledata = updatedirstate and not branchmerge
         stats, getfiledata = applyupdates(

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 10 of 13] merge: document return values of manifestmerge() and calculateupdates()

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594721548 -19800
#      Tue Jul 14 15:42:28 2020 +0530
# Node ID 6a87141b0721d719fcc3b8bb94743eda1a120c17
# Parent  15d0b77c5833af7c1b04e29dd4628a66d9518cde
# EXP-Topic mergestate-refactor
merge: document return values of manifestmerge() and calculateupdates()

In future patches, I want to add one more return value which represents
information which needs to stored and used at commit time.

Differential Revision: https://phab.mercurial-scm.org/D8741

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -558,6 +558,13 @@ def manifestmerge(
     branchmerge and force are as passed in to update
     matcher = matcher to filter file lists
     acceptremote = accept the incoming changes without prompting
+
+    Returns:
+
+    actions: dict of filename as keys and action related info as values
+    diverge: mapping of source name -> list of dest name for divergent renames
+    renamedelete: mapping of source name -> list of destinations for files
+                  deleted on one side and renamed on other.
     """
     if matcher is not None and matcher.always():
         matcher = None
@@ -875,7 +882,17 @@ def calculateupdates(
     matcher=None,
     mergeforce=False,
 ):
-    """Calculate the actions needed to merge mctx into wctx using ancestors"""
+    """
+    Calculate the actions needed to merge mctx into wctx using ancestors
+
+    Uses manifestmerge() to merge manifest and get list of actions required to
+    perform for merging two manifests. If there are multiple ancestors, uses bid
+    merge if enabled.
+
+    Also filters out actions which are unrequired if repository is sparse.
+
+    Returns same 3 element tuple as manifestmerge().
+    """
     # Avoid cycle.
     from . import sparse
 

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 11 of 13] merge: return 'commitinfo' from manifestmerge() and calculateupdates() (API)

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594723868 -19800
#      Tue Jul 14 16:21:08 2020 +0530
# Node ID 9fd8b89eee7e32ce4f3703d423cc320dec64da86
# Parent  6a87141b0721d719fcc3b8bb94743eda1a120c17
# EXP-Topic mergestate-refactor
merge: return 'commitinfo' from manifestmerge() and calculateupdates() (API)

commitinfo will be used to pass information which is required on commit phase
from the merge phase.

One common example is, merge chooses filenode from second parent and we need to
tell commit to choose that. Right now this one and related cases are not very
neatly implement and there is no clear line on how to pass on such information.

Upcoming patches will try to work on in this area and make things easier.

Differential Revision: https://phab.mercurial-scm.org/D8742

diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
--- a/hgext/convert/hg.py
+++ b/hgext/convert/hg.py
@@ -217,7 +217,8 @@ class mercurial_sink(common.converter_si
         """
         anc = [p1ctx.ancestor(p2ctx)]
         # Calculate what files are coming from p2
-        actions, diverge, rename = mergemod.calculateupdates(
+        # TODO: this might be achieved using commitinfo
+        actions, diverge, rename, commitinfo = mergemod.calculateupdates(
             self.repo,
             p1ctx,
             p2ctx,
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -543,12 +543,12 @@ def overridecalculateupdates(
     origfn, repo, p1, p2, pas, branchmerge, force, acceptremote, *args, **kwargs
 ):
     overwrite = force and not branchmerge
-    actions, diverge, renamedelete = origfn(
+    actions, diverge, renamedelete, commitinfo = origfn(
         repo, p1, p2, pas, branchmerge, force, acceptremote, *args, **kwargs
     )
 
     if overwrite:
-        return actions, diverge, renamedelete
+        return actions, diverge, renamedelete, commitinfo
 
     # Convert to dictionary with filename as key and action as value.
     lfiles = set()
@@ -620,7 +620,7 @@ def overridecalculateupdates(
                 actions[lfile] = (b'g', largs, b'replaces standin')
                 actions[standin] = (b'r', None, b'replaced by non-standin')
 
-    return actions, diverge, renamedelete
+    return actions, diverge, renamedelete, commitinfo
 
 
 @eh.wrapfunction(mergestatemod, b'recordupdates')
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -565,6 +565,8 @@ def manifestmerge(
     diverge: mapping of source name -> list of dest name for divergent renames
     renamedelete: mapping of source name -> list of destinations for files
                   deleted on one side and renamed on other.
+    commitinfo: dict containing data which should be used on commit
+                contains a filename -> info mapping
     """
     if matcher is not None and matcher.always():
         matcher = None
@@ -578,6 +580,10 @@ def manifestmerge(
     branch_copies1 = copies.branch_copies()
     branch_copies2 = copies.branch_copies()
     diverge = {}
+    # information from merge which is needed at commit time
+    # for example choosing filelog of which parent to commit
+    # TODO: use specific constants in future for this mapping
+    commitinfo = {}
     if followcopies:
         branch_copies1, branch_copies2, diverge = copies.mergecopies(
             repo, wctx, p2, pa
@@ -671,6 +677,8 @@ def manifestmerge(
                             (fl2, False),
                             b'remote is newer',
                         )
+                        if branchmerge:
+                            commitinfo[f] = b'other'
                 elif nol and n2 == a:  # remote only changed 'x'
                     actions[f] = (
                         mergestatemod.ACTION_EXEC,
@@ -685,6 +693,8 @@ def manifestmerge(
                         (fl1, False),
                         b'remote is newer',
                     )
+                    if branchmerge:
+                        commitinfo[f] = b'other'
                 else:  # both changed something
                     actions[f] = (
                         mergestatemod.ACTION_MERGE,
@@ -845,7 +855,7 @@ def manifestmerge(
     renamedelete = branch_copies1.renamedelete
     renamedelete.update(branch_copies2.renamedelete)
 
-    return actions, diverge, renamedelete
+    return actions, diverge, renamedelete, commitinfo
 
 
 def _resolvetrivial(repo, wctx, mctx, ancestor, actions):
@@ -891,13 +901,13 @@ def calculateupdates(
 
     Also filters out actions which are unrequired if repository is sparse.
 
-    Returns same 3 element tuple as manifestmerge().
+    Returns same 4 element tuple as manifestmerge().
     """
     # Avoid cycle.
     from . import sparse
 
     if len(ancestors) == 1:  # default
-        actions, diverge, renamedelete = manifestmerge(
+        actions, diverge, renamedelete, commitinfo = manifestmerge(
             repo,
             wctx,
             mctx,
@@ -927,7 +937,7 @@ def calculateupdates(
         diverge, renamedelete = None, None
         for ancestor in ancestors:
             repo.ui.note(_(b'\ncalculating bids for ancestor %s\n') % ancestor)
-            actions, diverge1, renamedelete1 = manifestmerge(
+            actions, diverge1, renamedelete1, commitinfo = manifestmerge(
                 repo,
                 wctx,
                 mctx,
@@ -1010,7 +1020,7 @@ def calculateupdates(
     )
     _resolvetrivial(repo, wctx, mctx, ancestors[0], actions)
 
-    return prunedactions, diverge, renamedelete
+    return prunedactions, diverge, renamedelete, commitinfo
 
 
 def _getcwd():
@@ -1734,7 +1744,7 @@ def update(
             followcopies = False
 
         ### calculate phase
-        actionbyfile, diverge, renamedelete = calculateupdates(
+        actionbyfile, diverge, renamedelete, commitinfo = calculateupdates(
             repo,
             wc,
             p2,

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 12 of 13] merge: pass commitinfo to applyupdates() and get it stored in mergestate

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594724512 -19800
#      Tue Jul 14 16:31:52 2020 +0530
# Node ID 4021b2b860103442ac0d36b0cc5176ec74c92a6a
# Parent  9fd8b89eee7e32ce4f3703d423cc320dec64da86
# EXP-Topic mergestate-refactor
merge: pass commitinfo to applyupdates() and get it stored in mergestate

This patch passes the commitinfo calulcated in manifestmerge() to applyupdates()
so that it can be read there and stored in mergestate. On commit, we can read
mergestate for such information and act accordingly.

This patch also makes ACTION_GET_OTHER_AND_STORE not required anymore. Next
patch will remove the messy code surrounding it.

Differential Revision: https://phab.mercurial-scm.org/D8743

diff --git a/hgext/remotefilelog/__init__.py b/hgext/remotefilelog/__init__.py
--- a/hgext/remotefilelog/__init__.py
+++ b/hgext/remotefilelog/__init__.py
@@ -479,7 +479,7 @@ def storewrapper(orig, requirements, pat
 
 # prefetch files before update
 def applyupdates(
-    orig, repo, actions, wctx, mctx, overwrite, wantfiledata, labels=None
+    orig, repo, actions, wctx, mctx, overwrite, wantfiledata, **opts
 ):
     if isenabled(repo):
         manifest = mctx.manifest()
@@ -488,9 +488,7 @@ def applyupdates(
             files.append((f, hex(manifest[f])))
         # batch fetch the needed files from the server
         repo.fileservice.prefetch(files)
-    return orig(
-        repo, actions, wctx, mctx, overwrite, wantfiledata, labels=labels
-    )
+    return orig(repo, actions, wctx, mctx, overwrite, wantfiledata, **opts)
 
 
 # Prefetch merge checkunknownfiles
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1199,12 +1199,21 @@ def emptyactions():
 
 
 def applyupdates(
-    repo, actions, wctx, mctx, overwrite, wantfiledata, labels=None
+    repo,
+    actions,
+    wctx,
+    mctx,
+    overwrite,
+    wantfiledata,
+    labels=None,
+    commitinfo=None,
 ):
     """apply the merge action list to the working directory
 
     wctx is the working copy context
     mctx is the context to be merged into the working copy
+    commitinfo is a mapping of information which needs to be stored somewhere
+               (probably mergestate) so that it can be used at commit time.
 
     Return a tuple of (counts, filedata), where counts is a tuple
     (updated, merged, removed, unresolved) that describes how many
@@ -1219,6 +1228,15 @@ def applyupdates(
         repo, wctx.p1().node(), mctx.node(), labels
     )
 
+    if commitinfo is None:
+        commitinfo = {}
+
+    for f, op in pycompat.iteritems(commitinfo):
+        # the other side of filenode was choosen while merging, store this in
+        # mergestate so that it can be reused on commit
+        if op == b'other':
+            ms.addmergedother(f)
+
     # add ACTION_GET_OTHER_AND_STORE to mergestate
     for e in actions[mergestatemod.ACTION_GET_OTHER_AND_STORE]:
         ms.addmergedother(e[0])
@@ -1891,7 +1909,14 @@ def update(
 
         wantfiledata = updatedirstate and not branchmerge
         stats, getfiledata = applyupdates(
-            repo, actions, wc, p2, overwrite, wantfiledata, labels=labels
+            repo,
+            actions,
+            wc,
+            p2,
+            overwrite,
+            wantfiledata,
+            labels=labels,
+            commitinfo=commitinfo,
         )
 
         if updatedirstate:

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 13 of 13] merge: remove no longer required ACTION_GET_OTHER_AND_STORE

Pulkit Goyal
In reply to this post by Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1594725028 -19800
#      Tue Jul 14 16:40:28 2020 +0530
# Node ID a5812036d3a33b466854204be2128d8aa1a11b0f
# Parent  4021b2b860103442ac0d36b0cc5176ec74c92a6a
# EXP-Topic mergestate-refactor
merge: remove no longer required ACTION_GET_OTHER_AND_STORE

In 1b8fd4af33189c84feadb47c74d659ec31cde3b9 I (ab)used merge actions to pass
info from manifestmerge() to applyupdates() and store info in mergestate.

In previous patches, we introduced a separate return value from manifestmerge()
and calculateupdates() and an argument to applyupdates() which achieved the same
thing.

Let's remove this no longer required messy code.

Differential Revision: https://phab.mercurial-scm.org/D8744

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -671,9 +671,7 @@ def manifestmerge(
                         )
                     else:
                         actions[f] = (
-                            mergestatemod.ACTION_GET_OTHER_AND_STORE
-                            if branchmerge
-                            else mergestatemod.ACTION_GET,
+                            mergestatemod.ACTION_GET,
                             (fl2, False),
                             b'remote is newer',
                         )
@@ -687,9 +685,7 @@ def manifestmerge(
                     )
                 elif nol and n1 == a:  # local only changed 'x'
                     actions[f] = (
-                        mergestatemod.ACTION_GET_OTHER_AND_STORE
-                        if branchmerge
-                        else mergestatemod.ACTION_GET,
+                        mergestatemod.ACTION_GET,
                         (fl1, False),
                         b'remote is newer',
                     )
@@ -960,8 +956,6 @@ def calculateupdates(
 
             for f, a in sorted(pycompat.iteritems(actions)):
                 m, args, msg = a
-                if m == mergestatemod.ACTION_GET_OTHER_AND_STORE:
-                    m = mergestatemod.ACTION_GET
                 repo.ui.debug(b' %s: %s -> %s\n' % (f, msg, m))
                 if f in fbids:
                     d = fbids[f]
@@ -1193,7 +1187,6 @@ def emptyactions():
             mergestatemod.ACTION_KEEP,
             mergestatemod.ACTION_PATH_CONFLICT,
             mergestatemod.ACTION_PATH_CONFLICT_RESOLVE,
-            mergestatemod.ACTION_GET_OTHER_AND_STORE,
         )
     }
 
@@ -1237,10 +1230,6 @@ def applyupdates(
         if op == b'other':
             ms.addmergedother(f)
 
-    # add ACTION_GET_OTHER_AND_STORE to mergestate
-    for e in actions[mergestatemod.ACTION_GET_OTHER_AND_STORE]:
-        ms.addmergedother(e[0])
-
     moves = []
     for m, l in actions.items():
         l.sort()
@@ -1783,7 +1772,6 @@ def update(
                     mergestatemod.ACTION_EXEC,
                     mergestatemod.ACTION_REMOVE,
                     mergestatemod.ACTION_PATH_CONFLICT_RESOLVE,
-                    mergestatemod.ACTION_GET_OTHER_AND_STORE,
                 ):
                     msg = _(b"conflicting changes")
                     hint = _(b"commit or update --clean to discard changes")
@@ -1854,10 +1842,6 @@ def update(
                 actions[m] = []
             actions[m].append((f, args, msg))
 
-        # ACTION_GET_OTHER_AND_STORE is a mergestatemod.ACTION_GET + store in mergestate
-        for e in actions[mergestatemod.ACTION_GET_OTHER_AND_STORE]:
-            actions[mergestatemod.ACTION_GET].append(e)
-
         if not util.fscasesensitive(repo.path):
             # check collision between files only in p2 for clean update
             if not branchmerge and (
diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -113,8 +113,6 @@ ACTION_DIR_RENAME_MOVE_LOCAL = b'dm'
 ACTION_KEEP = b'k'
 ACTION_EXEC = b'e'
 ACTION_CREATED_MERGE = b'cm'
-# GET the other/remote side and store this info in mergestate
-ACTION_GET_OTHER_AND_STORE = b'gs'
 
 
 class mergestate(object):

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 02 of 13] mergestate: remove unrequired RECORD_RESOLVED_OTHER record

Yuya Nishihara
In reply to this post by Pulkit Goyal
On Fri, 17 Jul 2020 14:29:23 +0530, Pulkit Goyal wrote:

> # HG changeset patch
> # User Pulkit Goyal <[hidden email]>
> # Date 1594290002 -19800
> #      Thu Jul 09 15:50:02 2020 +0530
> # Node ID 9e4a5e41f0ac3fa39a29d5376a1d3922e9221318
> # Parent  66fb1ff9343a4d73b44c4314d88176e283a7839d
> # EXP-Topic mergestate-refactor
> mergestate: remove unrequired RECORD_RESOLVED_OTHER record
>
> This was introduced in last cycle however while working on refactoring
> mergestate, I realized it's unncessary.
>
> This will break users who did a merge using previous version, did this kind of
> storage and before commiting updated the mercurial version.

Maybe better to document it and what error would be displayed in the release
notes.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 10 of 13] merge: document return values of manifestmerge() and calculateupdates()

Yuya Nishihara
In reply to this post by Pulkit Goyal
On Fri, 17 Jul 2020 14:29:31 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <[hidden email]>
> # Date 1594721548 -19800
> #      Tue Jul 14 15:42:28 2020 +0530
> # Node ID 6a87141b0721d719fcc3b8bb94743eda1a120c17
> # Parent  15d0b77c5833af7c1b04e29dd4628a66d9518cde
> # EXP-Topic mergestate-refactor
> merge: document return values of manifestmerge() and calculateupdates()

Queued up to this patch, thanks.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 11 of 13] merge: return 'commitinfo' from manifestmerge() and calculateupdates() (API)

Yuya Nishihara
In reply to this post by Pulkit Goyal
On Fri, 17 Jul 2020 14:29:32 +0530, Pulkit Goyal wrote:

> # HG changeset patch
> # User Pulkit Goyal <[hidden email]>
> # Date 1594723868 -19800
> #      Tue Jul 14 16:21:08 2020 +0530
> # Node ID 9fd8b89eee7e32ce4f3703d423cc320dec64da86
> # Parent  6a87141b0721d719fcc3b8bb94743eda1a120c17
> # EXP-Topic mergestate-refactor
> merge: return 'commitinfo' from manifestmerge() and calculateupdates() (API)
>
> commitinfo will be used to pass information which is required on commit phase
> from the merge phase.
>
> One common example is, merge chooses filenode from second parent and we need to
> tell commit to choose that. Right now this one and related cases are not very
> neatly implement and there is no clear line on how to pass on such information.

Is this commitinfo an orthogonal concept to the actions? If that's true, this
change makes sense, but if not, I think commitinfo[f] == b'other' can be a third
argument of ACTION_GET.

Just asking because I didn't review the previous attempt.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 11 of 13] merge: return 'commitinfo' from manifestmerge() and calculateupdates() (API)

Pulkit Goyal
On Sun, Jul 19, 2020 at 3:34 PM Yuya Nishihara <[hidden email]> wrote:

>
> On Fri, 17 Jul 2020 14:29:32 +0530, Pulkit Goyal wrote:
> > # HG changeset patch
> > # User Pulkit Goyal <[hidden email]>
> > # Date 1594723868 -19800
> > #      Tue Jul 14 16:21:08 2020 +0530
> > # Node ID 9fd8b89eee7e32ce4f3703d423cc320dec64da86
> > # Parent  6a87141b0721d719fcc3b8bb94743eda1a120c17
> > # EXP-Topic mergestate-refactor
> > merge: return 'commitinfo' from manifestmerge() and calculateupdates() (API)
> >
> > commitinfo will be used to pass information which is required on commit phase
> > from the merge phase.
> >
> > One common example is, merge chooses filenode from second parent and we need to
> > tell commit to choose that. Right now this one and related cases are not very
> > neatly implement and there is no clear line on how to pass on such information.
>
> Is this commitinfo an orthogonal concept to the actions? If that's true, this
> change makes sense, but if not, I think commitinfo[f] == b'other' can be a third
> argument of ACTION_GET.

Yes, this whole work is meant to refactor and draw lines between
various things. commitinfo will be the information which should be
stored in mergestate and then used at commit time. ACTION_* should be
meant for dirstate related things or actions which needs to be
performed resulting in some mergerecords.

>
> Just asking because I didn't review the previous attempt.

Now after spending a good amount of time with this code, it feels to
me that my old approach was a hack, non-extensible one which (ab)used
ACTION_* to achieve a quick bug fix.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 02 of 13] mergestate: remove unrequired RECORD_RESOLVED_OTHER record

Pierre-Yves David-2
In reply to this post by Yuya Nishihara


On 7/19/20 11:11 AM, Yuya Nishihara wrote:

> On Fri, 17 Jul 2020 14:29:23 +0530, Pulkit Goyal wrote:
>> # HG changeset patch
>> # User Pulkit Goyal <[hidden email]>
>> # Date 1594290002 -19800
>> #      Thu Jul 09 15:50:02 2020 +0530
>> # Node ID 9e4a5e41f0ac3fa39a29d5376a1d3922e9221318
>> # Parent  66fb1ff9343a4d73b44c4314d88176e283a7839d
>> # EXP-Topic mergestate-refactor
>> mergestate: remove unrequired RECORD_RESOLVED_OTHER record
>>
>> This was introduced in last cycle however while working on refactoring
>> mergestate, I realized it's unncessary.
>>
>> This will break users who did a merge using previous version, did this kind of
>> storage and before commiting updated the mercurial version.
>
> Maybe better to document it and what error would be displayed in the release
> notes.

It is probably not even worse breaking user of this. We could have a set
of LEGACY record that we can safely ignore.

--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel