# HG changeset patch # User Pierre-Yves David # Date 1607798108 -3600 # Node ID c19c662097e160402188091020386a456e5ebaa3 # Parent c692384bb559b4210e80774bcf8974e543face01 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 diff -r c692384bb559 -r c19c662097e1 mercurial/copies.py --- 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): diff -r c692384bb559 -r c19c662097e1 rust/hg-core/src/copy_tracing.rs --- 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, + 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) } } } diff -r c692384bb559 -r c19c662097e1 tests/test-copies-chain-merge.t --- 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 !)