changeset 46563:c19c662097e1

copies: detect case when a merge decision overwrite previous data

We now detect and record when a merge case required special logic (eg: thing
that append during the merge, ambiguity leading to picking p1 data, etc) and we
explicitly mark the result as superseding the previous data.

This fixes the family of test we previously added.

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

--- a/mercurial/copies.py	Mon Dec 14 19:26:33 2020 +0100
+++ b/mercurial/copies.py	Sat Dec 12 19:35:08 2020 +0100
@@ -434,7 +434,11 @@
                     # potential filelog related behavior.
                     assert parent == 2
                     current_copies = _merge_copies_dict(
-                        newcopies, current_copies, isancestor, changes
+                        newcopies,
+                        current_copies,
+                        isancestor,
+                        changes,
+                        current_rev,
                     )
             all_copies[current_rev] = current_copies
 
@@ -456,7 +460,7 @@
 PICK_EITHER = 2
 
 
-def _merge_copies_dict(minor, major, isancestor, changes):
+def _merge_copies_dict(minor, major, isancestor, changes, current_merge):
     """merge two copies-mapping together, minor and major
 
     In case of conflict, value from "major" will be picked.
@@ -474,8 +478,15 @@
         if other is None:
             minor[dest] = value
         else:
-            pick = _compare_values(changes, isancestor, dest, other, value)
-            if pick == PICK_MAJOR:
+            pick, overwrite = _compare_values(
+                changes, isancestor, dest, other, value
+            )
+            if overwrite:
+                if pick == PICK_MAJOR:
+                    minor[dest] = (current_merge, value[1])
+                else:
+                    minor[dest] = (current_merge, other[1])
+            elif pick == PICK_MAJOR:
                 minor[dest] = value
     return minor
 
@@ -483,9 +494,10 @@
 def _compare_values(changes, isancestor, dest, minor, major):
     """compare two value within a _merge_copies_dict loop iteration
 
-    return pick
+    return (pick, overwrite).
 
     - pick is one of PICK_MINOR, PICK_MAJOR or PICK_EITHER
+    - overwrite is True if pick is a return of an ambiguity that needs resolution.
     """
     major_tt, major_value = major
     minor_tt, minor_value = minor
@@ -493,7 +505,7 @@
     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
+        return PICK_EITHER, False
     elif (
         changes is not None
         and minor_value is not None
@@ -502,7 +514,7 @@
     ):
         # In this case, a deletion was reverted, the "alive" value overwrite
         # the deleted one.
-        return PICK_MINOR
+        return PICK_MINOR, True
     elif (
         changes is not None
         and major_value is not None
@@ -511,30 +523,30 @@
     ):
         # In this case, a deletion was reverted, the "alive" value overwrite
         # the deleted one.
-        return PICK_MAJOR
+        return PICK_MAJOR, True
     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
+            return PICK_MAJOR, True
         else:
-            return PICK_MAJOR
+            return PICK_MAJOR, False
     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
+            return PICK_MAJOR, True
         else:
-            return PICK_MINOR
+            return PICK_MINOR, False
     elif minor_value is None:
         # in case of conflict, the "alive" side wins.
-        return PICK_MAJOR
+        return PICK_MAJOR, True
     elif major_value is None:
         # in case of conflict, the "alive" side wins.
-        return PICK_MINOR
+        return PICK_MINOR, True
     else:
         # in case of conflict where both side are alive, major wins.
-        return PICK_MAJOR
+        return PICK_MAJOR, True
 
 
 def _revinfo_getter_extra(repo):

--- a/rust/hg-core/src/copy_tracing.rs	Mon Dec 14 19:26:33 2020 +0100
+++ b/rust/hg-core/src/copy_tracing.rs	Sat Dec 12 19:35:08 2020 +0100
@@ -568,20 +568,20 @@
     // This closure exist as temporary help while multiple developper are
     // actively working on this code. Feel free to re-inline it once this
     // code is more settled.
-    let mut cmp_value =
-        |dest: &PathToken,
-         src_minor: &TimeStampedPathCopy,
-         src_major: &TimeStampedPathCopy| {
-            compare_value(
-                path_map,
-                current_merge,
-                changes,
-                oracle,
-                dest,
-                src_minor,
-                src_major,
-            )
-        };
+    let cmp_value = |oracle: &mut AncestorOracle<A>,
+                     dest: &PathToken,
+                     src_minor: &TimeStampedPathCopy,
+                     src_major: &TimeStampedPathCopy| {
+        compare_value(
+            path_map,
+            current_merge,
+            changes,
+            oracle,
+            dest,
+            src_minor,
+            src_major,
+        )
+    };
     if minor.is_empty() {
         major
     } else if major.is_empty() {
@@ -605,11 +605,30 @@
         for (dest, src_minor) in minor {
             let src_major = major.get(&dest);
             match src_major {
-                None => major.insert(dest, src_minor),
+                None => {
+                    major.insert(dest, src_minor);
+                }
                 Some(src_major) => {
-                    match cmp_value(&dest, &src_minor, src_major) {
-                        MergePick::Any | MergePick::Major => None,
-                        MergePick::Minor => major.insert(dest, src_minor),
+                    let (pick, overwrite) =
+                        cmp_value(oracle, &dest, &src_minor, src_major);
+                    if overwrite {
+                        oracle.record_overwrite(src_minor.rev, current_merge);
+                        oracle.record_overwrite(src_major.rev, current_merge);
+                        let path = match pick {
+                            MergePick::Major => src_major.path,
+                            MergePick::Minor => src_minor.path,
+                            MergePick::Any => src_major.path,
+                        };
+                        let src = TimeStampedPathCopy {
+                            rev: current_merge,
+                            path,
+                        };
+                        major.insert(dest, src);
+                    } else {
+                        match pick {
+                            MergePick::Any | MergePick::Major => None,
+                            MergePick::Minor => major.insert(dest, src_minor),
+                        };
                     }
                 }
             };
@@ -621,11 +640,30 @@
         for (dest, src_major) in major {
             let src_minor = minor.get(&dest);
             match src_minor {
-                None => minor.insert(dest, src_major),
+                None => {
+                    minor.insert(dest, src_major);
+                }
                 Some(src_minor) => {
-                    match cmp_value(&dest, src_minor, &src_major) {
-                        MergePick::Any | MergePick::Minor => None,
-                        MergePick::Major => minor.insert(dest, src_major),
+                    let (pick, overwrite) =
+                        cmp_value(oracle, &dest, &src_major, src_minor);
+                    if overwrite {
+                        oracle.record_overwrite(src_minor.rev, current_merge);
+                        oracle.record_overwrite(src_major.rev, current_merge);
+                        let path = match pick {
+                            MergePick::Major => src_minor.path,
+                            MergePick::Minor => src_major.path,
+                            MergePick::Any => src_major.path,
+                        };
+                        let src = TimeStampedPathCopy {
+                            rev: current_merge,
+                            path,
+                        };
+                        minor.insert(dest, src);
+                    } else {
+                        match pick {
+                            MergePick::Any | MergePick::Major => None,
+                            MergePick::Minor => minor.insert(dest, src_major),
+                        };
                     }
                 }
             };
@@ -663,12 +701,32 @@
                 DiffItem::Update { old, new } => {
                     let (dest, src_major) = new;
                     let (_, src_minor) = old;
-                    match cmp_value(dest, src_minor, src_major) {
-                        MergePick::Major => to_minor(dest, src_major),
-                        MergePick::Minor => to_major(dest, src_minor),
-                        // If the two entry are identical, no need to do
-                        // anything (but diff should not have yield them)
-                        MergePick::Any => unreachable!(),
+                    let (pick, overwrite) =
+                        cmp_value(oracle, dest, src_minor, src_major);
+                    if overwrite {
+                        oracle.record_overwrite(src_minor.rev, current_merge);
+                        oracle.record_overwrite(src_major.rev, current_merge);
+                        let path = match pick {
+                            MergePick::Major => src_major.path,
+                            MergePick::Minor => src_minor.path,
+                            // If the two entry are identical, no need to do
+                            // anything (but diff should not have yield them)
+                            MergePick::Any => src_major.path,
+                        };
+                        let src = TimeStampedPathCopy {
+                            rev: current_merge,
+                            path,
+                        };
+                        to_minor(dest, &src);
+                        to_major(dest, &src);
+                    } else {
+                        match pick {
+                            MergePick::Major => to_minor(dest, src_major),
+                            MergePick::Minor => to_major(dest, src_minor),
+                            // If the two entry are identical, no need to do
+                            // anything (but diff should not have yield them)
+                            MergePick::Any => unreachable!(),
+                        }
                     }
                 }
             };
@@ -717,39 +775,37 @@
     dest: &PathToken,
     src_minor: &TimeStampedPathCopy,
     src_major: &TimeStampedPathCopy,
-) -> MergePick {
+) -> (MergePick, bool) {
     if src_major.rev == current_merge {
         if src_minor.rev == current_merge {
             if src_major.path.is_none() {
                 // We cannot get different copy information for both p1 and p2
                 // from the same revision. Unless this was a
                 // deletion
-                MergePick::Any
+                (MergePick::Any, false)
             } else {
                 unreachable!();
             }
         } else {
             // The last value comes the current merge, this value -will- win
             // eventually.
-            oracle.record_overwrite(src_minor.rev, src_major.rev);
-            MergePick::Major
+            (MergePick::Major, true)
         }
     } else if src_minor.rev == current_merge {
         // The last value comes the current merge, this value -will- win
         // eventually.
-        oracle.record_overwrite(src_major.rev, src_minor.rev);
-        MergePick::Minor
+        (MergePick::Minor, true)
     } else if src_major.path == src_minor.path {
         // we have the same value, but from other source;
         if src_major.rev == src_minor.rev {
             // If the two entry are identical, they are both valid
-            MergePick::Any
+            (MergePick::Any, false)
         } else if oracle.is_overwrite(src_major.rev, src_minor.rev) {
-            MergePick::Minor
+            (MergePick::Minor, false)
         } else if oracle.is_overwrite(src_minor.rev, src_major.rev) {
-            MergePick::Major
+            (MergePick::Major, false)
         } else {
-            MergePick::Major
+            (MergePick::Any, true)
         }
     } else if src_major.rev == src_minor.rev {
         // We cannot get copy information for both p1 and p2 in the
@@ -766,7 +822,7 @@
         {
             // If the file is "deleted" in the major side but was
             // salvaged by the merge, we keep the minor side alive
-            MergePick::Minor
+            (MergePick::Minor, true)
         } else if src_major.path.is_some()
             && src_minor.path.is_none()
             && action == MergeCase::Salvaged
@@ -774,7 +830,7 @@
             // If the file is "deleted" in the minor side but was
             // salvaged by the merge, unconditionnaly preserve the
             // major side.
-            MergePick::Major
+            (MergePick::Major, true)
         } else if oracle.is_overwrite(src_minor.rev, src_major.rev) {
             // The information from the minor version are strictly older than
             // the major version
@@ -784,10 +840,10 @@
                 // mean the older copy information are still relevant.
                 //
                 // The major side wins such conflict.
-                MergePick::Major
+                (MergePick::Major, true)
             } else {
                 // No activity on the minor branch, pick the newer one.
-                MergePick::Major
+                (MergePick::Major, false)
             }
         } else if oracle.is_overwrite(src_major.rev, src_minor.rev) {
             if action == MergeCase::Merged {
@@ -796,20 +852,20 @@
                 // mean the older copy information are still relevant.
                 //
                 // The major side wins such conflict.
-                MergePick::Major
+                (MergePick::Major, true)
             } else {
                 // No activity on the minor branch, pick the newer one.
-                MergePick::Minor
+                (MergePick::Minor, false)
             }
         } else if src_minor.path.is_none() {
             // the minor side has no relevant information, pick the alive one
-            MergePick::Major
+            (MergePick::Major, true)
         } else if src_major.path.is_none() {
             // the major side has no relevant information, pick the alive one
-            MergePick::Minor
+            (MergePick::Minor, true)
         } else {
             // by default the major side wins
-            MergePick::Major
+            (MergePick::Major, true)
         }
     }
 }

--- a/tests/test-copies-chain-merge.t	Mon Dec 14 19:26:33 2020 +0100
+++ b/tests/test-copies-chain-merge.t	Sat Dec 12 19:35:08 2020 +0100
@@ -3182,10 +3182,8 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mK,AEm")' f
   A f
     a (filelog !)
-    a (missing-correct-output sidedata !)
-    a (missing-correct-output upgraded !)
-    b (known-bad-output sidedata !)
-    b (known-bad-output upgraded !)
+    a (sidedata !)
+    a (upgraded !)
 
 
 The result from mEAm is the same for the subsequent merge:
@@ -3205,10 +3203,8 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mJ,EAm")' f
   A f
     a (filelog !)
-    b (missing-correct-output sidedata !)
-    b (missing-correct-output upgraded !)
-    a (known-bad-output sidedata !)
-    a (known-bad-output upgraded !)
+    b (sidedata !)
+    b (upgraded !)
 
 Subcase: chaining conflicting rename resolution
 ```````````````````````````````````````````````
@@ -3235,10 +3231,8 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mT,PQm")' v
   A v
     r (filelog !)
-    p (missing-correct-output sidedata !)
-    p (missing-correct-output upgraded !)
-    r (known-bad-output sidedata !)
-    r (known-bad-output upgraded !)
+    p (sidedata !)
+    p (upgraded !)
 
 
 The result from mQPm is the same for the subsequent merge:
@@ -3254,10 +3248,8 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mS,QPm")' v
   A v
     r (filelog !)
-    r (missing-correct-output sidedata !)
-    r (missing-correct-output upgraded !)
-    p (known-bad-output sidedata !)
-    p (known-bad-output upgraded !)
+    r (sidedata !)
+    r (upgraded !)
 
 
 Subcase: chaining salvage information during a merge
@@ -3271,9 +3263,7 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mCB-revert-m-0")'
   M b
   A d
-    a (filelog !)
-    a (sidedata !)
-    a (upgraded !)
+    a (no-changeset no-compatibility !)
   A t
     p
   R a
@@ -3281,22 +3271,17 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBC-revert-m-0")'
   M b
   A d
-    a (filelog !)
-    a (sidedata !)
-    a (upgraded !)
+    a (no-changeset no-compatibility !)
   A t
     p
   R a
   R p
 
 chained output
-
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mBC+revert,Lm")'
   M b
   A d
-    a (filelog !)
-    a (missing-correct-output sidedata !)
-    a (missing-correct-output upgraded !)
+    a (no-changeset no-compatibility !)
   A t
     p
   A unrelated-l
@@ -3305,9 +3290,7 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mCB+revert,Lm")'
   M b
   A d
-    a (filelog !)
-    a (missing-correct-output sidedata !)
-    a (missing-correct-output upgraded !)
+    a (no-changeset no-compatibility !)
   A t
     p
   A unrelated-l
@@ -3316,9 +3299,7 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mL,BC+revertm")'
   M b
   A d
-    a (filelog !)
-    a (missing-correct-output sidedata !)
-    a (missing-correct-output upgraded !)
+    a (no-changeset no-compatibility !)
   A t
     p
   A unrelated-l
@@ -3327,9 +3308,7 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mL,CB+revertm")'
   M b
   A d
-    a (filelog !)
-    a (missing-correct-output sidedata !)
-    a (missing-correct-output upgraded !)
+    a (no-changeset no-compatibility !)
   A t
     p
   A unrelated-l
@@ -3373,18 +3352,10 @@
 
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mGF,Nm")' d
   A d
-    a (filelog !)
-    a (missing-correct-output sidedata !)
-    a (missing-correct-output upgraded !)
-    h (known-bad-output sidedata !)
-    h (known-bad-output upgraded !)
+    a (no-changeset no-compatibility !)
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mN,GFm")' d
   A d
-    a (filelog !)
-    a (missing-correct-output sidedata !)
-    a (missing-correct-output upgraded !)
-    h (known-bad-output sidedata !)
-    h (known-bad-output upgraded !)
+    a (no-changeset no-compatibility !)
 
 
 Subcase: chaining conflicting rename resolution, with extra change during the merge
@@ -3411,11 +3382,7 @@
 
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mK,AE-change-m")' f
   A f
-    a (filelog !)
-    a (missing-correct-output sidedata !)
-    a (missing-correct-output upgraded !)
-    b (known-bad-output sidedata !)
-    b (known-bad-output upgraded !)
+    a (no-changeset no-compatibility !)
 
 
 The result from mEAm is the same for the subsequent merge:
@@ -3435,7 +3402,5 @@
   $ hg status --copies --rev 'desc("i-0")' --rev 'desc("mJ,EA-change-m")' f
   A f
     a (filelog !)
-    b (missing-correct-output sidedata !)
-    b (missing-correct-output upgraded !)
-    a (known-bad-output sidedata !)
-    a (known-bad-output upgraded !)
+    b (sidedata !)
+    b (upgraded !)