Patchwork D10116: commit: reorder if-else conditional to give mergestate info priority

login
register
mail settings
Submitter phabricator
Date March 5, 2021, 5:30 p.m.
Message ID <differential-rev-PHID-DREV-xuz6m5rq3zrlwaarbat2-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48431/
State Superseded
Headers show

Comments

phabricator - March 5, 2021, 5:30 p.m.
pulkit created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Looking at the code now, I was unable to find a good reason as why we only rely
  on mergestate extras info after checking whether a filelog parent is ancestor of
  other or not.
  
  I mean if we have stored in mergestate that `other` was chosed, we should
  blindly pick that one.
  
  This cleanup will also help introduce more cases when both `fparent1` and
  `fparent2` are non-null but using info from mergestate, we can fastpath.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10116

AFFECTED FILES
  mercurial/commit.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/mercurial/commit.py b/mercurial/commit.py
--- a/mercurial/commit.py
+++ b/mercurial/commit.py
@@ -359,19 +359,18 @@ 
     elif fparent1 == nullid:
         fparent1, fparent2 = fparent2, nullid
     elif fparent2 != nullid:
-        # is one parent an ancestor of the other?
-        fparentancestors = flog.commonancestorsheads(fparent1, fparent2)
-        if fparent1 in fparentancestors:
+        if (
+            ms.active()
+            and ms.extras(fname).get(b'filenode-source') == b'other'
+        ):
             fparent1, fparent2 = fparent2, nullid
-        elif fparent2 in fparentancestors:
-            fparent2 = nullid
-        elif not fparentancestors:
-            # TODO: this whole if-else might be simplified much more
-            if (
-                ms.active()
-                and ms.extras(fname).get(b'filenode-source') == b'other'
-            ):
+        # is one parent an ancestor of the other?
+        else:
+            fparentancestors = flog.commonancestorsheads(fparent1, fparent2)
+            if fparent1 in fparentancestors:
                 fparent1, fparent2 = fparent2, nullid
+            elif fparent2 in fparentancestors:
+                fparent2 = nullid
 
     force_new_node = False
     # The file might have been deleted by merge code and user explicitly choose