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

login
register
mail settings
Submitter Pulkit Goyal
Date July 17, 2020, 8:59 a.m.
Message ID <9fd8b89eee7e32ce4f37.1594976372@workspace>
Download mbox | patch
Permalink /patch/46785/
State Accepted
Headers show

Comments

Pulkit Goyal - July 17, 2020, 8:59 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# 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
Yuya Nishihara - July 19, 2020, 10:02 a.m.
On Fri, 17 Jul 2020 14:29:32 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # 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.
Pulkit Goyal - July 30, 2020, 8:39 a.m.
On Sun, Jul 19, 2020 at 3:34 PM Yuya Nishihara <yuya@tcha.org> wrote:
>
> On Fri, 17 Jul 2020 14:29:32 +0530, Pulkit Goyal wrote:
> > # HG changeset patch
> > # User Pulkit Goyal <7895pulkit@gmail.com>
> > # 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.

Patch

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,