[Commented On] D9612: copies: rearrange all value comparison conditional

baymax (Baymax, Your Personal Patch-care Companion) phabricator at mercurial-scm.org
Tue Jan 5 14:02:17 UTC 2021


baymax added a comment.
baymax updated this revision to Diff 24594.


  ✅ refresh by Heptapod after a successful CI run (🐙 💚)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D9612?vs=24571&id=24594

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D9612/new/

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

AFFECTED FILES
  mercurial/copies.py
  rust/hg-core/src/copy_tracing.rs

CHANGE DETAILS

diff --git a/rust/hg-core/src/copy_tracing.rs b/rust/hg-core/src/copy_tracing.rs
--- a/rust/hg-core/src/copy_tracing.rs
+++ b/rust/hg-core/src/copy_tracing.rs
@@ -746,6 +746,8 @@
             MergePick::Any
         } else if oracle.is_overwrite(src_major.rev, src_minor.rev) {
             MergePick::Minor
+        } else if oracle.is_overwrite(src_minor.rev, src_major.rev) {
+            MergePick::Major
         } else {
             MergePick::Major
         }
@@ -753,45 +755,61 @@
         // We cannot get copy information for both p1 and p2 in the
         // same rev. So this is the same value.
         unreachable!(
-            "conflict information from p1 and p2 in the same revision"
+            "conflicting information from p1 and p2 in the same revision"
         );
     } else {
         let dest_path = path_map.untokenize(*dest);
         let action = changes.get_merge_case(dest_path);
-        if src_major.path.is_none() && action == MergeCase::Salvaged {
+        if src_minor.path.is_some()
+            && src_major.path.is_none()
+            && action == MergeCase::Salvaged
+        {
             // If the file is "deleted" in the major side but was
             // salvaged by the merge, we keep the minor side alive
             MergePick::Minor
-        } else if src_minor.path.is_none() && action == MergeCase::Salvaged {
+        } else if src_major.path.is_some()
+            && src_minor.path.is_none()
+            && action == MergeCase::Salvaged
+        {
             // If the file is "deleted" in the minor side but was
             // salvaged by the merge, unconditionnaly preserve the
             // major side.
             MergePick::Major
-        } else if action == MergeCase::Merged {
-            // If the file was actively merged, copy information
-            // from each side might conflict.  The major side will
-            // win such conflict.
-            MergePick::Major
+        } else if oracle.is_overwrite(src_minor.rev, src_major.rev) {
+            // The information from the minor version are strictly older than
+            // the major version
+            if action == MergeCase::Merged {
+                // If the file was actively merged, its means some non-copy
+                // activity happened on the other branch. It
+                // mean the older copy information are still relevant.
+                //
+                // The major side wins such conflict.
+                MergePick::Major
+            } else {
+                // No activity on the minor branch, pick the newer one.
+                MergePick::Major
+            }
         } else if oracle.is_overwrite(src_major.rev, src_minor.rev) {
-            // If the minor side is strictly newer than the major
-            // side, it should be kept.
-            MergePick::Minor
-        } else if src_major.path.is_some() {
-            // without any special case, the "major" value win
-            // other the "minor" one.
+            if action == MergeCase::Merged {
+                // If the file was actively merged, its means some non-copy
+                // activity happened on the other branch. It
+                // mean the older copy information are still relevant.
+                //
+                // The major side wins such conflict.
+                MergePick::Major
+            } else {
+                // No activity on the minor branch, pick the newer one.
+                MergePick::Minor
+            }
+        } else if src_minor.path.is_none() {
+            // the minor side has no relevant information, pick the alive one
             MergePick::Major
-        } else if oracle.is_overwrite(src_minor.rev, src_major.rev) {
-            // the "major" rev is a direct ancestors of "minor",
-            // any different value should
-            // overwrite
-            MergePick::Major
+        } else if src_major.path.is_none() {
+            // the major side has no relevant information, pick the alive one
+            MergePick::Minor
         } else {
-            // major version is None (so the file was deleted on
-            // that branch) and that branch is independant (neither
-            // minor nor major is an ancestors of the other one.)
-            // We preserve the new
-            // information about the new file.
-            MergePick::Minor
+            // by default the major side wins
+            MergePick::Major
         }
     }
 }
diff --git a/mercurial/copies.py b/mercurial/copies.py
--- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -474,32 +474,60 @@
 
 
 def _compare_values(changes, isancestor, dest, minor, major):
-    """compare two value within a _merge_copies_dict loop iteration"""
+    """compare two value within a _merge_copies_dict loop iteration
+
+    return pick
+
+    - pick is one of PICK_MINOR, PICK_MAJOR or PICK_EITHER
+    """
     major_tt, major_value = major
     minor_tt, minor_value = minor
 
-    # evacuate some simple case first:
     if major_tt == minor_tt:
         # if it comes from the same revision it must be the same value
         assert major_value == minor_value
         return PICK_EITHER
-    elif major[1] == minor[1]:
-        return PICK_EITHER
-
-    # actual merging needed: content from "major" wins, unless it is older than
-    # the branch point or there is a merge
-    elif changes is not None and major[1] is None and dest in changes.salvaged:
+    elif (
+        changes is not None
+        and minor_value is not None
+        and major_value is None
+        and dest in changes.salvaged
+    ):
+        # In this case, a deletion was reverted, the "alive" value overwrite
+        # the deleted one.
         return PICK_MINOR
-    elif changes is not None and minor[1] is None and dest in changes.salvaged:
+    elif (
+        changes is not None
+        and major_value is not None
+        and minor_value is None
+        and dest in changes.salvaged
+    ):
+        # In this case, a deletion was reverted, the "alive" value overwrite
+        # the deleted one.
         return PICK_MAJOR
-    elif changes is not None and dest in changes.merged:
+    elif isancestor(minor_tt, major_tt):
+        if changes is not None and dest in changes.merged:
+            # change to dest happened on the branch without copy-source change,
+            # so both source are valid and "major" wins.
+            return PICK_MAJOR
+        else:
+            return PICK_MAJOR
+    elif isancestor(major_tt, minor_tt):
+        if changes is not None and dest in changes.merged:
+            # change to dest happened on the branch without copy-source change,
+            # so both source are valid and "major" wins.
+            return PICK_MAJOR
+        else:
+            return PICK_MINOR
+    elif minor_value is None:
+        # in case of conflict, the "alive" side wins.
         return PICK_MAJOR
-    elif not isancestor(major_tt, minor_tt):
-        if major[1] is not None:
-            return PICK_MAJOR
-        elif isancestor(minor_tt, major_tt):
-            return PICK_MAJOR
-    return PICK_MINOR
+    elif major_value is None:
+        # in case of conflict, the "alive" side wins.
+        return PICK_MINOR
+    else:
+        # in case of conflict where both side are alive, major wins.
+        return PICK_MAJOR
 
 
 def _revinfo_getter_extra(repo):



To: marmoute, #hg-reviewers
Cc: mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210105/1f8a7255/attachment-0002.html>


More information about the Mercurial-patches mailing list