Patchwork [2,of,2,V2] merge: make 'cd' and 'dc' actions store the same arguments as 'm'

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 14, 2015, 6:46 a.m.
Message ID <33003ae0eb069549729f.1447483603@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11402/
State Accepted
Headers show

Comments

Siddharth Agarwal - Nov. 14, 2015, 6:46 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1447483389 28800
#      Fri Nov 13 22:43:09 2015 -0800
# Node ID 33003ae0eb069549729f0b47dc5b198594392931
# Parent  88a697eb28dfa94038ac89543cd40ee65b5ca00b
merge: make 'cd' and 'dc' actions store the same arguments as 'm'

We're going to treat these conflicts similarly to merge conflicts, and this
change to the way we store things in memory makes future code a lot simpler.
Martin von Zweigbergk - Nov. 15, 2015, 7:50 a.m.
On Fri, Nov 13, 2015 at 10:48 PM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1447483389 28800
> #      Fri Nov 13 22:43:09 2015 -0800
> # Node ID 33003ae0eb069549729f0b47dc5b198594392931
> # Parent  88a697eb28dfa94038ac89543cd40ee65b5ca00b
> merge: make 'cd' and 'dc' actions store the same arguments as 'm'
>
> We're going to treat these conflicts similarly to merge conflicts, and this
> change to the way we store things in memory makes future code a lot
> simpler.
>

Is it mostly the filemerge code that will be shared between regular merge
conflicts and change/delete conflicts? How much of the filemerge code will
be shared? I can see that the tool selection code and the backup code would
be shared. Is your "autoresolve" bookmark what I should pull from to see
the current future?

Relatedly, I said in another email that I had been wondering about how you
were planning on dealing with BC. What I meant there was how you were
planning on dealing with BC while calling the tool (I thought that that's
what you were referring to before I had finished reading the email). I
suppose you're planning on updating the internal tools so they can deal
with merges where one side is missing. How about external tools? Will there
be a new config for that (e.g. "merge-tools.mytool.canhandlemissing")?


>
> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py
> +++ b/hgext/largefiles/overrides.py
> @@ -481,6 +481,9 @@ def overridecalculateupdates(origfn, rep
>          (lm, largs, lmsg) = actions.get(lfile, (None, None, None))
>          (sm, sargs, smsg) = actions.get(standin, (None, None, None))
>          if sm in ('g', 'dc') and lm != 'r':
> +            if sm == 'dc':
> +                f1, f2, fa, move, anc = sargs
> +                sargs = (p2[f2].flags(),)
>              # Case 1: normal file in the working copy, largefile in
>              # the second parent
>              usermsg = _('remote turned local normal file %s into a
> largefile\n'
> @@ -496,6 +499,9 @@ def overridecalculateupdates(origfn, rep
>                  else:
>                      actions[standin] = ('r', None, 'replaced by
> non-standin')
>          elif lm in ('g', 'dc') and sm != 'r':
> +            if lm == 'dc':
> +                f1, f2, fa, move, anc = largs
> +                largs = (p2[f2].flags(),)
>              # Case 2: largefile in the working copy, normal file in
>              # the second parent
>              usermsg = _('remote turned local largefile %s into a normal
> file\n'
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -623,7 +623,8 @@ def manifestmerge(repo, wctx, p2, pa, br
>                      if acceptremote:
>                          actions[f] = ('r', None, "remote delete")
>                      else:
> -                        actions[f] = ('cd', None,  "prompt
> changed/deleted")
> +                        actions[f] = ('cd', (f, None, f, False,
> pa.node()),
> +                                      "prompt changed/deleted")
>                  elif n1[20:] == 'a':
>                      # This extra 'a' is added by working copy manifest to
> mark
>                      # the file as locally added. We should forget it
> instead of
> @@ -673,7 +674,8 @@ def manifestmerge(repo, wctx, p2, pa, br
>                  if acceptremote:
>                      actions[f] = ('c', (fl2,), "remote recreating")
>                  else:
> -                    actions[f] = ('dc', (fl2,), "prompt deleted/changed")
> +                    actions[f] = ('dc', (None, f, f, False, pa.node()),
> +                                  "prompt deleted/changed")
>
>      return actions, diverge, renamedelete
>
> @@ -1264,7 +1266,8 @@ def update(repo, node, branchmerge, forc
>                  actions['a'].append((f, None, "prompt keep"))
>
>          for f, args, msg in sorted(actions['dc']):
> -            flags, = args
> +            f1, f2, fa, move, anc = args
> +            flags = p2[f2].flags()
>              if repo.ui.promptchoice(
>                  _("remote changed %s which local deleted\n"
>                    "use (c)hanged version or leave (d)eleted?"
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - Nov. 15, 2015, 9:36 p.m.
On 11/14/15 23:50, Martin von Zweigbergk wrote:
>
>
> On Fri, Nov 13, 2015 at 10:48 PM Siddharth Agarwal <sid0@fb.com 
> <mailto:sid0@fb.com>> wrote:
>
>     # HG changeset patch
>     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>     # Date 1447483389 28800
>     #      Fri Nov 13 22:43:09 2015 -0800
>     # Node ID 33003ae0eb069549729f0b47dc5b198594392931
>     # Parent  88a697eb28dfa94038ac89543cd40ee65b5ca00b
>     merge: make 'cd' and 'dc' actions store the same arguments as 'm'
>
>     We're going to treat these conflicts similarly to merge conflicts,
>     and this
>     change to the way we store things in memory makes future code a
>     lot simpler.
>
>
> Is it mostly the filemerge code that will be shared between regular 
> merge conflicts and change/delete conflicts?

A bit of that, and the code dealing with merges in applyupdates(). The 
latter is what these patches make easier.

> How much of the filemerge code will be shared? I can see that the tool 
> selection code and the backup code would be shared. Is your 
> "autoresolve" bookmark what I should pull from to see the current future?

Yeah. http://42.netv6.net/sid0-wip/hg/shortlog/autoresolve

>
> Relatedly, I said in another email that I had been wondering about how 
> you were planning on dealing with BC. What I meant there was how you 
> were planning on dealing with BC while calling the tool (I thought 
> that that's what you were referring to before I had finished reading 
> the email).

I'm mostly not dealing with that at the moment.

> I suppose you're planning on updating the internal tools so they can 
> deal with merges where one side is missing.

Just the nomerge ones (:local, :other, :prompt and :fail).

> How about external tools? Will there be a new config for that (e.g. 
> "merge-tools.mytool.canhandlemissing")?

Potentially in the future, but my current plan is to simply not use 
external tools and prompt instead when an external tool is explicitly 
selected. Choosing whether to prompt or to fail was what my email was about.

- Siddharth

>
>     diff --git a/hgext/largefiles/overrides.py
>     b/hgext/largefiles/overrides.py
>     --- a/hgext/largefiles/overrides.py
>     +++ b/hgext/largefiles/overrides.py
>     @@ -481,6 +481,9 @@ def overridecalculateupdates(origfn, rep
>              (lm, largs, lmsg) = actions.get(lfile, (None, None, None))
>              (sm, sargs, smsg) = actions.get(standin, (None, None, None))
>              if sm in ('g', 'dc') and lm != 'r':
>     +            if sm == 'dc':
>     +                f1, f2, fa, move, anc = sargs
>     +                sargs = (p2[f2].flags(),)
>                  # Case 1: normal file in the working copy, largefile in
>                  # the second parent
>                  usermsg = _('remote turned local normal file %s into
>     a largefile\n'
>     @@ -496,6 +499,9 @@ def overridecalculateupdates(origfn, rep
>                      else:
>                          actions[standin] = ('r', None, 'replaced by
>     non-standin')
>              elif lm in ('g', 'dc') and sm != 'r':
>     +            if lm == 'dc':
>     +                f1, f2, fa, move, anc = largs
>     +                largs = (p2[f2].flags(),)
>                  # Case 2: largefile in the working copy, normal file in
>                  # the second parent
>                  usermsg = _('remote turned local largefile %s into a
>     normal file\n'
>     diff --git a/mercurial/merge.py b/mercurial/merge.py
>     --- a/mercurial/merge.py
>     +++ b/mercurial/merge.py
>     @@ -623,7 +623,8 @@ def manifestmerge(repo, wctx, p2, pa, br
>                          if acceptremote:
>                              actions[f] = ('r', None, "remote delete")
>                          else:
>     -                        actions[f] = ('cd', None,  "prompt
>     changed/deleted")
>     +                        actions[f] = ('cd', (f, None, f, False,
>     pa.node()),
>     +                                      "prompt changed/deleted")
>                      elif n1[20:] == 'a':
>                          # This extra 'a' is added by working copy
>     manifest to mark
>                          # the file as locally added. We should forget
>     it instead of
>     @@ -673,7 +674,8 @@ def manifestmerge(repo, wctx, p2, pa, br
>                      if acceptremote:
>                          actions[f] = ('c', (fl2,), "remote recreating")
>                      else:
>     -                    actions[f] = ('dc', (fl2,), "prompt
>     deleted/changed")
>     +                    actions[f] = ('dc', (None, f, f, False,
>     pa.node()),
>     +                                  "prompt deleted/changed")
>
>          return actions, diverge, renamedelete
>
>     @@ -1264,7 +1266,8 @@ def update(repo, node, branchmerge, forc
>                      actions['a'].append((f, None, "prompt keep"))
>
>              for f, args, msg in sorted(actions['dc']):
>     -            flags, = args
>     +            f1, f2, fa, move, anc = args
>     +            flags = p2[f2].flags()
>                  if repo.ui.promptchoice(
>                      _("remote changed %s which local deleted\n"
>                        "use (c)hanged version or leave (d)eleted?"
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@selenic.com <mailto:Mercurial-devel@selenic.com>
>     https://selenic.com/mailman/listinfo/mercurial-devel
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Martin von Zweigbergk - Nov. 16, 2015, 6:35 p.m.
On Fri, Nov 13, 2015 at 10:48 PM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1447483389 28800
> #      Fri Nov 13 22:43:09 2015 -0800
> # Node ID 33003ae0eb069549729f0b47dc5b198594392931
> # Parent  88a697eb28dfa94038ac89543cd40ee65b5ca00b
> merge: make 'cd' and 'dc' actions store the same arguments as 'm'
>

I have pushed these to the clowncopter, thanks.

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -481,6 +481,9 @@  def overridecalculateupdates(origfn, rep
         (lm, largs, lmsg) = actions.get(lfile, (None, None, None))
         (sm, sargs, smsg) = actions.get(standin, (None, None, None))
         if sm in ('g', 'dc') and lm != 'r':
+            if sm == 'dc':
+                f1, f2, fa, move, anc = sargs
+                sargs = (p2[f2].flags(),)
             # Case 1: normal file in the working copy, largefile in
             # the second parent
             usermsg = _('remote turned local normal file %s into a largefile\n'
@@ -496,6 +499,9 @@  def overridecalculateupdates(origfn, rep
                 else:
                     actions[standin] = ('r', None, 'replaced by non-standin')
         elif lm in ('g', 'dc') and sm != 'r':
+            if lm == 'dc':
+                f1, f2, fa, move, anc = largs
+                largs = (p2[f2].flags(),)
             # Case 2: largefile in the working copy, normal file in
             # the second parent
             usermsg = _('remote turned local largefile %s into a normal file\n'
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -623,7 +623,8 @@  def manifestmerge(repo, wctx, p2, pa, br
                     if acceptremote:
                         actions[f] = ('r', None, "remote delete")
                     else:
-                        actions[f] = ('cd', None,  "prompt changed/deleted")
+                        actions[f] = ('cd', (f, None, f, False, pa.node()),
+                                      "prompt changed/deleted")
                 elif n1[20:] == 'a':
                     # This extra 'a' is added by working copy manifest to mark
                     # the file as locally added. We should forget it instead of
@@ -673,7 +674,8 @@  def manifestmerge(repo, wctx, p2, pa, br
                 if acceptremote:
                     actions[f] = ('c', (fl2,), "remote recreating")
                 else:
-                    actions[f] = ('dc', (fl2,), "prompt deleted/changed")
+                    actions[f] = ('dc', (None, f, f, False, pa.node()),
+                                  "prompt deleted/changed")
 
     return actions, diverge, renamedelete
 
@@ -1264,7 +1266,8 @@  def update(repo, node, branchmerge, forc
                 actions['a'].append((f, None, "prompt keep"))
 
         for f, args, msg in sorted(actions['dc']):
-            flags, = args
+            f1, f2, fa, move, anc = args
+            flags = p2[f2].flags()
             if repo.ui.promptchoice(
                 _("remote changed %s which local deleted\n"
                   "use (c)hanged version or leave (d)eleted?"