[PATCH V3] rebase: calculate ancestors for --base separately (issue5420)

[Mateusz Kwapich]

Just two nits, the rest looks good.

Excerpts from Jun Wu's message of 2016-11-28 05:54:44 +0000:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1480311922 0
> #      Mon Nov 28 05:45:22 2016 +0000
> # Node ID c750859c8069c0ae1d4cbbd0cce583971f8b6f71
> # Parent  64b55bffc1c059eb4c11ca195b561ca8a287f59e
> # Available At https://bitbucket.org/quark-zju/hg-draft 
> #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r c750859c8069
> rebase: calculate ancestors for --base separately (issue5420)
> 
> Previously, the --base option only works with a single "branch" - if there
> is one changeset in the "--base" revset whose branching point(s) is/are
> different from another changeset in the "--base" revset, "rebase" will error
> out with:
> 
>   abort: source is ancestor of destination
> 
> This happens if the user has multiple draft branches, and uses "hg rebase -b
> 'draft()' -d master", for example. The error message looks cryptic to users
> who don't know the implementation detail.
> 
> This patch changes the logic to calculate the common ancestor for every
> "base" changeset separately so we won't (incorrectly) select "source" which
> is an ancestor of the destination.
> 
> This patch should not change the behavior where all changesets specified by
> "--base" have the same branching point(s).
> 
> A new situation is: some of the specified changesets could be rebased, while
> some couldn't (because they are descendants of the destination, or they do
> not share a common ancestor with the destination). The current behavior is
> to show "nothing to rebase" and exits with 1.
> 
> This patch maintains the current behavior (show "nothing to rebase") even if
> part of the "--base" revset could be rebased. A clearer error message may be
> "cannot find branching point for X", or "X is a descendant of destination".
> The error message issue is tracked by issue5422 separately.
> 
> A test is added with all kinds of tricky cases I could think of for now.
> 
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -725,10 +725,17 @@ def _definesets(ui, repo, destf=None, sr
>              destf = str(dest)
>  
> -        commonanc = repo.revs('ancestor(%ld, %d)', base, dest).first()
> -        if commonanc is not None:
> -            rebaseset = repo.revs('(%d::(%ld) - %d)::',
> -                                  commonanc, base, commonanc)
> -        else:
> -            rebaseset = []
> +        roots = [] # selected children of branching points
> +        bpbase = {} # {branchingpoint: [origbase]}
> +        for b in base: # group bases by branching points
> +            bp = repo.revs('ancestor(%d, %d)', b, dest).first()

What if the ancestor doesn't exist? (in case of base and dest totally
unrelated)? Should we check for that?

> +            bpbase[bp] = bpbase.get(bp, []) + [b]

This is more efficient and shorter:
  bpbase.setdefault(bp, []).append(b)

> +        if None in bpbase:
> +            # emulate the old behavior, showing "nothing to rebase" (a better
> +            # behavior may be abort with "cannot find branching point" error)
> +            bpbase.clear()
> +        for bp, bs in bpbase.iteritems(): # calculate roots
> +            roots += list(repo.revs('children(%d) & ancestors(%ld)', bp, bs))
> +
> +        rebaseset = repo.revs('%ld::', roots)
>  
>          if not rebaseset:

--