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

Jun Wu quark at fb.com
Mon Nov 28 10:22:18 EST 2016


Excerpts from Mateusz Kwapich's message of 2016-11-28 13:46:05 +0000:
> 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?

They are covered by the "Disconnected graph" and "Multiple roots" test cases.

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

Will change.

> > +        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:
> 


More information about the Mercurial-devel mailing list