[Commented On] D9613: copies: detect case when a merge decision overwrite previous data
baymax (Baymax, Your Personal Patch-care Companion)
phabricator at mercurial-scm.org
Tue Jan 5 14:02:27 UTC 2021
baymax added a comment.
baymax updated this revision to Diff 24595.
✅ refresh by Heptapod after a successful CI run (🐙 💚)
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D9613?vs=24572&id=24595
BRANCH
default
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
@@ -2144,10 +2144,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:
@@ -2167,10 +2165,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 salvage information during a merge
@@ -2184,50 +2180,37 @@
$ 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 !)
R a
$ 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 !)
R a
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 unrelated-l
R a
$ 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 unrelated-l
R a
$ 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 unrelated-l
R a
$ 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 unrelated-l
R a
@@ -2268,15 +2251,7 @@
$ 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 !)
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
@@ -427,7 +427,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
@@ -449,7 +453,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.
@@ -467,8 +471,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
@@ -476,9 +487,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 and ambiguity that needs resolution.
"""
major_tt, major_value = major
minor_tt, minor_value = minor
@@ -486,7 +498,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
@@ -495,7 +507,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
@@ -504,30 +516,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
Cc: mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210105/3341b80b/attachment-0002.html>
More information about the Mercurial-patches
mailing list