[PATCH 2 of 2 V2] rebase: support multiple roots for rebaseset

[Pierre-Yves David]

On 16 janv. 2013, at 23:39, Benoit Boissinot wrote:

> Actually it still looks fishy.
> 
>  def buildstate(repo, dest, rebaseset, collapse):
> @@ -599,24 +599,21 @@ def buildstate(repo, dest, rebaseset, co
>          raise util.Abort(_('cannot rebase onto an applied mq patch'))
> 
>      roots = list(repo.set('roots(%ld)', rebaseset))
>      if not roots:
>          raise util.Abort(_('no matching revisions'))
> -    if len(roots) > 1:
> -        raise util.Abort(_("can't rebase multiple roots"))
> -    root = roots[0]
> +    for root in roots:
> +        commonbase = root.ancestor(dest)
> +        if commonbase == root:
> +            raise util.Abort(_('source is ancestor of destination'))
> +        if commonbase == dest:
> +            samebranch = root.branch() == dest.branch()
> +            if not collapse and samebranch and root in dest.children():
> +                repo.ui.debug('source is a child of destination\n')
> +                return None
> 
> commonbase is defined in the loop. 
> 
> -    commonbase = root.ancestor(dest)
> -    if commonbase == root:
> -        raise util.Abort(_('source is ancestor of destination'))
> -    if commonbase == dest:
> -        samebranch = root.branch() == dest.branch()
> -        if not collapse and samebranch and root in dest.children():
> -            repo.ui.debug('source is a child of destination\n')
> -            return None
> -
> -    repo.ui.debug('rebase onto %d starting from %d\n' % (dest, root))
> +    repo.ui.debug('rebase onto %d starting from %s\n' % (dest, roots))
>      state = dict.fromkeys(rebaseset, nullrev)
>      # Rebase tries to turn <dest> into a parent of <root> while
>      # preserving the number of parents of rebased changesets:
>      #
>      # - A changeset with a single parent will always be rebased as a
> @@ -651,17 +648,21 @@ def buildstate(repo, dest, rebaseset, co
>      # +--------------------+----------------------+-------------------------+
>      # | unrelated source   | new parent is <dest> | ambiguous, abort        |
>      # +--------------------+----------------------+-------------------------+
>      #
>      # The actual abort is handled by `defineparents`
> -    if len(root.parents()) <= 1:
> -        # ancestors of <root> not ancestors of <dest>
> -        detachset = repo.changelog.findmissingrevs([commonbase.rev()],
> -                                                   [root.rev()])
> -        state.update(dict.fromkeys(detachset, nullmerge))
> -        # detachset can have root, and we definitely want to rebase that
> -        state[root.rev()] = nullrev
> +    roots.sort()
> +    for root in roots:
> +        if len(root.parents()) <= 1:
> +            # ancestors of <root> not ancestors of <dest>
> +            detachset = repo.changelog.findmissingrevs([commonbase.rev()],
> +                                                       [root.rev()])
> 
> And used here. I think dest could be used instead of commonbase.
>  
> +            for r in detachset:
> +                if r not in state:
> +                    state[r] = nullmerge
> +            # detachset can have root, and we definitely want to rebase that
> +            state[root.rev()] = nullrev
>      return repo['.'].rev(), dest.rev(), state


Good catch, we need single loop. For clarity detachset may be accumulated and processed after the loop.

-- 
Pierre-Yves