changeset 46606:c692384bb559

copies: rearrange all value comparison conditional

To properly handle the newly tested case (chaining of merges) we will need to
detect more accurately when an actualy merging of the copy information (and
superseed the two existing data). Before starting to do so, we need to
reorganise the values comparison to introduce different conditional branches
when such actual merging is needed/detected.

To avoid mixing too many change in this complicated code, we do the
reorganisation before adding the "overwrite detection" logic in the next
changesets.

Differential Revision: https://phab.mercurial-scm.org/D9612

--- a/mercurial/copies.py	Mon Feb 22 12:21:00 2021 +0100
+++ b/mercurial/copies.py	Mon Dec 14 19:26:33 2020 +0100
@@ -481,32 +481,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):

--- a/rust/hg-core/src/copy_tracing.rs	Mon Feb 22 12:21:00 2021 +0100
+++ b/rust/hg-core/src/copy_tracing.rs	Mon Dec 14 19:26:33 2020 +0100
@@ -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
         }
     }
 }