Patchwork [5,of,7,mergedriver] merge.applyupdates: use counters from mergestate

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 21, 2015, 1:04 a.m.
Message ID <9f9a937bb5af83115f1c.1448067840@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11557/
State Accepted
Commit a421debae31d9864037ed0ec641200dc08960e95
Delegated to: Martin von Zweigbergk
Headers show

Comments

Siddharth Agarwal - Nov. 21, 2015, 1:04 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1448066259 28800
#      Fri Nov 20 16:37:39 2015 -0800
# Node ID 9f9a937bb5af83115f1c3349fd2058bb82fe4486
# Parent  45f8add0d3082f025c7f3e67b1e28cd78680e72f
# Available At http://42.netv6.net/sid0-wip/hg/
#              hg pull http://42.netv6.net/sid0-wip/hg/ -r 9f9a937bb5af
merge.applyupdates: use counters from mergestate

This eliminates a whole bunch of duplicate code and allows us to update the
removed count for change/delete conflicts where the delete action was chosen.
Martin von Zweigbergk - Nov. 23, 2015, 5:32 a.m.
On Fri, Nov 20, 2015 at 5:05 PM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1448066259 28800
> #      Fri Nov 20 16:37:39 2015 -0800
> # Node ID 9f9a937bb5af83115f1c3349fd2058bb82fe4486
> # Parent  45f8add0d3082f025c7f3e67b1e28cd78680e72f
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r 9f9a937bb5af
> merge.applyupdates: use counters from mergestate
>
> This eliminates a whole bunch of duplicate code and allows us to update the
> removed count for change/delete conflicts where the delete action was
> chosen.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -944,7 +944,7 @@ def applyupdates(repo, actions, wctx, mc
>      describes how many files were affected by the update.
>      """
>
> -    updated, merged, removed, unresolved = 0, 0, 0, 0
> +    updated, merged, removed = 0, 0, 0
>      ms = mergestate.clean(repo, wctx.p1().node(), mctx.node())
>      moves = []
>      for m, l in actions.items():
> @@ -1084,15 +1084,7 @@ def applyupdates(repo, actions, wctx, mc
>              continue
>          audit(f)
>          complete, r = ms.preresolve(f, wctx, labels=labels)
>

Can 'preresolve' now stop returning the 'r' value now? I saw that it's also
used in commands.py:5677, but maybe there's now a better way to do that
just there was here?


> -        if complete:
> -            if r is not None and r > 0:
> -                unresolved += 1
> -            else:
> -                if r is None:
> -                    updated += 1
> -                else:
> -                    merged += 1
> -        else:
> +        if not complete:
>              numupdates += 1
>              tocomplete.append((f, args, msg))
>
> @@ -1101,17 +1093,12 @@ def applyupdates(repo, actions, wctx, mc
>          repo.ui.debug(" %s: %s -> m (merge)\n" % (f, msg))
>          z += 1
>          progress(_updating, z, item=f, total=numupdates, unit=_files)
> -        r = ms.resolve(f, wctx, labels=labels)
> -        if r is not None and r > 0:
> -            unresolved += 1
> -        else:
> -            if r is None:
> -                updated += 1
> -            else:
> -                merged += 1
> +        ms.resolve(f, wctx, labels=labels)
>
>      ms.commit()
>
> +    unresolved = ms.unresolvedcount()
> +
>      if usemergedriver and not unresolved and ms.mdstate() != 's':
>          if not driverconclude(repo, ms, wctx, labels=labels):
>              # XXX setting unresolved to at least 1 is a hack to make sure
> we
> @@ -1120,6 +1107,10 @@ def applyupdates(repo, actions, wctx, mc
>
>          ms.commit()
>
> +    msupdated, msmerged, msremoved = ms.counts()
> +    updated += msupdated
> +    merged += msmerged
> +    removed += msremoved
>      progress(_updating, None, total=numupdates, unit=_files)
>
>      return updated, merged, removed, unresolved
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - Nov. 23, 2015, 5:43 a.m.
On 11/22/15 21:32, Martin von Zweigbergk wrote:
> Can 'preresolve' now stop returning the 'r' value now? I saw that it's 
> also used in commands.py:5677, but maybe there's now a better way to 
> do that just there was here?

I don't think so -- higher level functions still need to know if there 
was a failure during (pre)resolution.
Martin von Zweigbergk - Nov. 23, 2015, 6:17 p.m.
On Sun, Nov 22, 2015 at 9:43 PM Siddharth Agarwal <sid@less-broken.com>
wrote:

> On 11/22/15 21:32, Martin von Zweigbergk wrote:
> > Can 'preresolve' now stop returning the 'r' value now? I saw that it's
> > also used in commands.py:5677, but maybe there's now a better way to
> > do that just there was here?
>
> I don't think so -- higher level functions still need to know if there
> was a failure during (pre)resolution.
>

I think they can do that by checking the mergestate for the file, but if
they want to do that, we might as well continue to return the result for
convenience (i.e., let's leave it as is).

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -944,7 +944,7 @@  def applyupdates(repo, actions, wctx, mc
     describes how many files were affected by the update.
     """
 
-    updated, merged, removed, unresolved = 0, 0, 0, 0
+    updated, merged, removed = 0, 0, 0
     ms = mergestate.clean(repo, wctx.p1().node(), mctx.node())
     moves = []
     for m, l in actions.items():
@@ -1084,15 +1084,7 @@  def applyupdates(repo, actions, wctx, mc
             continue
         audit(f)
         complete, r = ms.preresolve(f, wctx, labels=labels)
-        if complete:
-            if r is not None and r > 0:
-                unresolved += 1
-            else:
-                if r is None:
-                    updated += 1
-                else:
-                    merged += 1
-        else:
+        if not complete:
             numupdates += 1
             tocomplete.append((f, args, msg))
 
@@ -1101,17 +1093,12 @@  def applyupdates(repo, actions, wctx, mc
         repo.ui.debug(" %s: %s -> m (merge)\n" % (f, msg))
         z += 1
         progress(_updating, z, item=f, total=numupdates, unit=_files)
-        r = ms.resolve(f, wctx, labels=labels)
-        if r is not None and r > 0:
-            unresolved += 1
-        else:
-            if r is None:
-                updated += 1
-            else:
-                merged += 1
+        ms.resolve(f, wctx, labels=labels)
 
     ms.commit()
 
+    unresolved = ms.unresolvedcount()
+
     if usemergedriver and not unresolved and ms.mdstate() != 's':
         if not driverconclude(repo, ms, wctx, labels=labels):
             # XXX setting unresolved to at least 1 is a hack to make sure we
@@ -1120,6 +1107,10 @@  def applyupdates(repo, actions, wctx, mc
 
         ms.commit()
 
+    msupdated, msmerged, msremoved = ms.counts()
+    updated += msupdated
+    merged += msmerged
+    removed += msremoved
     progress(_updating, None, total=numupdates, unit=_files)
 
     return updated, merged, removed, unresolved