evolve: refactor content-divergence resolution logic
> What is the case we are looking at?
This is about refactoring the part of content-div resolution logic where it
decides which cset should be relocated and where.
> What is a "topologicial common ancestors" vs a "greatest common ancestors"?
`tca` is an ancestor which we can decide/find by looking at the at graph
visually for e.g
```
c3(*) c4(*)
| |
c2(x) c1(x)
c5 | /
\ | /
c0
```
(c5 is the successor of c2 and c1)
now here,
`tca` of c3 and c4 is: c0
`gca` of c3 and c4 is: c5
> What is the new top-level logic/behavior that makes it better?
The old code had some unnecessary edge cases just because we were using `gca`,
since it can point to a revision that is not a topological ancestor.
For e.g see b779b40f996e
Eventually, the code around this was getting messy unnecessarily. So I looked
into it and found a simple and more robust approach.
And in new code, it is simple and straightforward (and easy to understand),
where we handle the following 4 cases when solving content-div:
1) when both are on the same parent
=> (no need to do anything special, and simply proceed)
2) both are on the different parent but a) `tca` is the parent of one of them
or b) there is no non-obsolete revision between `tca` and one of the
divergent cset.
=> (relocate one to the other side and proceed)
3) both are on different parents and `tca` is not the parent of any of them and
there is at least one non-obsolete cset between tca and both the divergent
cset i.e (tca::div1) and (tca::div2) both the ranges have at least one non-obs
revision.
=> (this is the case which we don't handle yet, but the solution would be to
prompt the user to choose an evolve destination.)
4) both are in the parent-child relation
=> (both are merged and new cset will be based on the successor of `tca`)
Changes in test-evolve-issue5958.t demonstrate that new code also covered case4
because in a resolution of "two divergent csets with parent-child relation"
there should be one cset as a result and no orphan revs (as you can see there
was an orphan before this patch).
--- a/hgext3rd/evolve/evolvecmd.py Mon Dec 30 00:11:00 2019 +0530
+++ b/hgext3rd/evolve/evolvecmd.py Sun Dec 29 23:59:41 2019 +0530
@@ -349,32 +349,9 @@
ui.write_err(hint)
return (False, b".")
- otherp1 = succsotherp1 = other.p1().rev()
- divp1 = succsdivp1 = divergent.p1().rev()
-
- # finding single successors of otherp1 and divp1
- try:
- succsotherp1 = utility._singlesuccessor(repo, other.p1())
- except utility.MultipleSuccessorsError:
- pass
+ otherp1 = other.p1().rev()
+ divp1 = divergent.p1().rev()
- try:
- succsdivp1 = utility._singlesuccessor(repo, divergent.p1())
- except utility.MultipleSuccessorsError:
- pass
-
- # the changeset on which resolution changeset will be based on
- resolutionparent = repo[succsdivp1].node()
-
- # the nullrev has to be handled specially because -1 is overloaded to both
- # mean nullrev (this meaning is used for the result of changectx.rev(), as
- # called above) and the tipmost revision (this meaning is used for the %d
- # format specifier, as used below)
- if nodemod.nullrev in (succsotherp1, succsdivp1):
- # nullrev is the only possible ancestor if succsotherp1 or succsdivp1 is nullrev
- gca = [nodemod.nullrev]
- else:
- gca = repo.revs(b"ancestor(%d, %d)" % (succsotherp1, succsdivp1))
# divonly: non-obsolete csets which are topological ancestor of "divergent"
# but not "other"
divonly = repo.revs(b"only(%d, %d) - obsolete()" % (divergent.rev(),
@@ -393,9 +370,9 @@
# possible cases here:
#
# 1) both have the same parents
- # 2) both have different parents but greatest common anscestor of them is
+ # 2) both have different parents but topological common anscestor is
# parent of one of them
- # 3) both have different parents and gca is not parent of any of them
+ # 3) both have different parents and tca is not parent of any of them
# 4) one of them is parent of other
#
# we are handling 1) very good now.
@@ -405,57 +382,44 @@
if otherp1 == divp1:
# both are on the same parents
pass
- elif succsotherp1 in gca and succsdivp1 in gca:
- # both are not on the same parent but have same parents's succs.
- if otheronly and divonly:
+ else:
+ if otheronly and divonly and not haspubdiv:
# case: we have visible csets on both side diverging from
# tca of "divergent" and "other". We still need to decide what
# to do in this case
- pass
+ msg = _(b"skipping %s: have a different parent than %s "
+ b"(not handled yet)\n") % (divergent, other)
+ hint = _(b"| %(d)s, %(o)s are not based on the same changeset.\n"
+ b"| With the current state of its implementation, \n"
+ b"| evolve does not work in that case.\n"
+ b"| rebase one of them next to the other and run \n"
+ b"| this command again.\n"
+ b"| - either: hg rebase --dest 'p1(%(d)s)' -r %(o)s\n"
+ b"| - or: hg rebase --dest 'p1(%(o)s)' -r %(d)s\n"
+ ) % {b'd': divergent, b'o': other}
+ ui.write_err(msg)
+ ui.write_err(hint)
+ return (False, b".")
if otheronly:
relocatereq = True
+ # When public branch is behind to the mutable branch, for now we
+ # relocate mutable cset to public one's side in every case.
+ #
+ # This behaviour might be sub optimal when ancestors of mutable
+ # cset has changes its relocated descendant rely on.
+ #
+ # Otherwise, we are going to rebase the "behind" branch up to the
+ # new brancmap level.
if not haspubdiv:
# can't swap when public divergence, as public can't move
divergent, other = swapnodes(divergent, other)
- resolutionparent = repo[succsotherp1].node()
elif divonly:
relocatereq = True
else:
# no extra cset on either side; so not considering relocation
pass
- elif succsotherp1 in gca and succsdivp1 not in gca:
- relocatereq = True
- pass
- elif succsdivp1 in gca and succsotherp1 not in gca:
- relocatereq = True
- # When public branch is behind to the mutable branch, for now we
- # relocate mutable cset to public one's side in every case.
- #
- # This behaviour might be sub optimal when ancestors of mutable
- # cset has changes its relocated descendant rely on.
- #
- # Otherwise, we are going to rebase the "behind" branch up to the new
- # brancmap level.
- if not haspubdiv:
- divergent, other = swapnodes(divergent, other)
- resolutionparent = divergent.p1().node()
- else:
- msg = _(b"skipping %s: have a different parent than %s "
- b"(not handled yet)\n") % (divergent, other)
- hint = _(b"| %(d)s, %(o)s are not based on the same changeset.\n"
- b"| With the current state of its implementation, \n"
- b"| evolve does not work in that case.\n"
- b"| rebase one of them next to the other and run \n"
- b"| this command again.\n"
- b"| - either: hg rebase --dest 'p1(%(d)s)' -r %(o)s\n"
- b"| - or: hg rebase --dest 'p1(%(o)s)' -r %(d)s\n"
- ) % {b'd': divergent, b'o': other}
- ui.write_err(msg)
- ui.write_err(hint)
- return (False, b".")
-
- return (True, divergent, other, resolutionparent, relocatereq)
+ return (True, divergent, other, relocatereq)
def _solvedivergent(ui, repo, divergent, evolvestate, displayer, dryrun=False,
confirm=False, progresscb=None):
@@ -495,10 +459,17 @@
evolvestate)
if not datatoproceed[0]:
return (False, b".")
- divergent, other, resolutionparent, relocatereq = datatoproceed[1:]
+
+ divergent, other, relocatereq = datatoproceed[1:]
if relocatereq:
evolvestate[b'relocation-req'] = True
+ try:
+ succsdivp1 = utility._singlesuccessor(repo, divergent.p1())
+ except utility.MultipleSuccessorsError:
+ msg = _(b"ambiguous orphan resolution parent for %s")
+ raise error.Abort(msg % divergent.hex()[:12])
+ resolutionparent = repo[succsdivp1].node()
evolvestate[b'resolutionparent'] = resolutionparent
if not ui.quiet or confirm:
@@ -520,13 +491,10 @@
ui.write((b'hg commit -m "`hg log -r %s --template={desc}`";\n'
% divergent))
return (False, b".")
- try:
- succsdivp1 = utility._singlesuccessor(repo, divergent.p1())
- except utility.MultipleSuccessorsError:
- msg = _(b"ambiguous orphan resolution parent for %s")
- raise error.Abort(msg % divergent.hex()[:12])
- # relocate divergent cset to its obsolete parent's successsor
- if succsdivp1 != divergent.p1().rev():
+
+ # relocate divergent cset to resolution parent (which is its obsolete
+ # parent's successsor)
+ if divergent.p1().rev() != repo[resolutionparent].rev():
evolvestate[b'relocating-div'] = True
ui.status(_(b'rebasing "divergent" content-divergent changeset %s on'
b' %s\n' % (divergent, repo[succsdivp1])))
--- a/tests/test-evolve-content-divergent-stack.t Mon Dec 30 00:11:00 2019 +0530
+++ b/tests/test-evolve-content-divergent-stack.t Sun Dec 29 23:59:41 2019 +0530
@@ -997,11 +997,5 @@
1 changesets pruned
$ hg evolve --content-divergent
- skipping 57a3f8edf065: have a different parent than 9a1f460df8b5 (not handled yet)
- | 57a3f8edf065, 9a1f460df8b5 are not based on the same changeset.
- | With the current state of its implementation,
- | evolve does not work in that case.
- | rebase one of them next to the other and run
- | this command again.
- | - either: hg rebase --dest 'p1(57a3f8edf065)' -r 9a1f460df8b5
- | - or: hg rebase --dest 'p1(9a1f460df8b5)' -r 57a3f8edf065
+ abort: ambiguous orphan resolution parent for 9a1f460df8b5
+ [255]
--- a/tests/test-evolve-issue5958.t Mon Dec 30 00:11:00 2019 +0530
+++ b/tests/test-evolve-issue5958.t Sun Dec 29 23:59:41 2019 +0530
@@ -80,43 +80,33 @@
rewritten(date) as c17bf400a278 using metaedit by test (Thu Jan 01 00:00:00 1970 +0000)
$ hg evolve --content-divergent
- merge:[6] add foo.txt
- with: [4] add foo.txt
+ merge:[4] add foo.txt
+ with: [6] add foo.txt
base: [1] add foo.txt
- 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
- 1 new orphan changesets
- working directory is now at 459c64f7eaad
+ 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+ working directory is now at 95a06d27be64
$ hg log -Gp
- @ changeset: 7:459c64f7eaad
+ @ changeset: 7:95a06d27be64
| tag: tip
- | parent: 4:c17bf400a278
+ | parent: 0:a24ed8ad918c
| user: test
| date: Wed Dec 31 23:59:58 1969 -0000
- | instability: orphan
| summary: add foo.txt
|
- | diff -r c17bf400a278 -r 459c64f7eaad bar.txt
+ | diff -r a24ed8ad918c -r 95a06d27be64 bar.txt
| --- /dev/null Thu Jan 01 00:00:00 1970 +0000
| +++ b/bar.txt Wed Dec 31 23:59:58 1969 -0000
| @@ -0,0 +1,1 @@
| +hi
- | diff -r c17bf400a278 -r 459c64f7eaad baz.txt
+ | diff -r a24ed8ad918c -r 95a06d27be64 baz.txt
| --- /dev/null Thu Jan 01 00:00:00 1970 +0000
| +++ b/baz.txt Wed Dec 31 23:59:58 1969 -0000
| @@ -0,0 +1,1 @@
| +hi
- |
- x changeset: 4:c17bf400a278
- | parent: 0:a24ed8ad918c
- | user: test
- | date: Wed Dec 31 23:59:59 1969 -0000
- | obsolete: rewritten using evolve as 7:459c64f7eaad
- | summary: add foo.txt
- |
- | diff -r a24ed8ad918c -r c17bf400a278 foo.txt
+ | diff -r a24ed8ad918c -r 95a06d27be64 foo.txt
| --- /dev/null Thu Jan 01 00:00:00 1970 +0000
- | +++ b/foo.txt Wed Dec 31 23:59:59 1969 -0000
+ | +++ b/foo.txt Wed Dec 31 23:59:58 1969 -0000
| @@ -0,0 +1,1 @@
| +hi
|