[Updated] D9613: copies: detect case when a merge decision overwrite previous data
marmoute (Pierre-Yves David)
phabricator at mercurial-scm.org
Wed Feb 24 16:02:11 UTC 2021
Closed by commit rHG740234aca875: copies: detect case when a merge decision overwrite previous data (authored by marmoute).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D9613?vs=25778&id=25843
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D9613/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D9613
AFFECTED FILES
mercurial/copies.py
rust/hg-core/src/copy_tracing.rs
tests/test-copies-chain-merge.t
CHANGE DETAILS
diff --git a/tests/test-copies-chain-merge.t b/tests/test-copies-chain-merge.t
--- a/tests/test-copies-chain-merge.t
+++ b/tests/test-copies-chain-merge.t
@@ -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 !)
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
@@ -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)
}
}
}
diff --git a/mercurial/copies.py b/mercurial/copies.py
--- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -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):
To: marmoute, #hg-reviewers, Alphare
Cc: Alphare, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210224/05c4ae12/attachment-0002.html>
More information about the Mercurial-patches
mailing list