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

login
register
mail settings
Submitter phabricator
Date March 10, 2021, 9:04 a.m.
Message ID <differential-rev-PHID-DREV-xcjphc32tw2ezhjkwzay-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48462/
State Superseded
Headers show

Comments

phabricator - March 10, 2021, 9:04 a.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.
  
  The test change actually demonstrates the point of the patch. During merge we
  were getting the other side of the file but on commit we were marking that as
  merged.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/commit.py
  tests/test-rebase-conflicts.t

CHANGE DETAILS




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

Patch

diff --git a/tests/test-rebase-conflicts.t b/tests/test-rebase-conflicts.t
--- a/tests/test-rebase-conflicts.t
+++ b/tests/test-rebase-conflicts.t
@@ -276,13 +276,13 @@ 
   committing manifest
   committing changelog
   updating the branch cache
-  rebased as 2a7f09cac94c
+  rebased as c1ffa3b5274e
   rebase status stored
   rebase merging completed
   update back to initial working directory parent
   resolving manifests
    branchmerge: False, force: False, partial: False
-   ancestor: 2a7f09cac94c, local: 2a7f09cac94c+, remote: d79e2059b5c0
+   ancestor: c1ffa3b5274e, local: c1ffa3b5274e+, remote: d79e2059b5c0
    f1.txt: other deleted -> r
   removing f1.txt
    f2.txt: remote created -> g
@@ -300,7 +300,7 @@ 
   list of changesets:
   4c9fbe56a16f30c0d5dcc40ec1a97bbe3325209c
   19c888675e133ab5dff84516926a65672eaf04d9
-  2a7f09cac94c7f4b73ebd5cd1a62d3b2e8e336bf
+  c1ffa3b5274e92a9388fe782854e295d2e8d0443
   bundle2-output-bundle: "HG20", 3 parts total
   bundle2-output-part: "changegroup" (params: 1 mandatory 1 advisory) streamed payload
   bundle2-output-part: "cache:rev-branch-cache" (advisory) streamed payload
@@ -311,7 +311,7 @@ 
   adding changesets
   add changeset 4c9fbe56a16f
   add changeset 19c888675e13
-  add changeset 2a7f09cac94c
+  add changeset c1ffa3b5274e
   adding manifests
   adding file changes
   adding f1.txt revisions
diff --git a/mercurial/commit.py b/mercurial/commit.py
--- a/mercurial/commit.py
+++ b/mercurial/commit.py
@@ -359,19 +359,15 @@ 
     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