# HG changeset patch # User Sushil khanchi # Date 1577644181 -19800 # Node ID 13152b2fe8f78641a6a1c711cd70c161f1675ff8 # Parent 4a9f9cd7a0abeebdaa9f978cad6a10119f290270 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). diff -r 4a9f9cd7a0ab -r 13152b2fe8f7 hgext3rd/evolve/evolvecmd.py --- 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]))) diff -r 4a9f9cd7a0ab -r 13152b2fe8f7 tests/test-evolve-content-divergent-stack.t --- 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] diff -r 4a9f9cd7a0ab -r 13152b2fe8f7 tests/test-evolve-issue5958.t --- 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 |