[PATCH] rebase: persist rebaseskipobsolete state

Martin von Zweigbergk martinvonz at google.com
Fri Mar 25 13:13:31 EDT 2016


I just ran into this bug yesterday. Could you mention in what change it was
introduced so we know in what range it is broken?

On Tue, Mar 15, 2016 at 12:45 PM Laurent Charignon <lcharignon at fb.com>
wrote:

> # HG changeset patch
> # User Laurent Charignon <lcharignon at fb.com>
> # Date 1458071050 25200
> #      Tue Mar 15 12:44:10 2016 -0700
> # Node ID 98edf167ea7b101efb2bd1c59db53e46c0fd182a
> # Parent  70c2f8a982766b512e9d7f41f2d93fdb92f5481f
> rebase: persist rebaseskipobsolete state
>
> Before this patch, when a rebase aborted, we didn't persist the mapping
> between
> obsolete revisions and their successors.
> When running hg rebase --continue, we would crash looking for this
> non-existing
> mapping.
>
> This patch persists the mapping to disk and restores it from disk to avoid
> this
> issue. A test is added for the special case of plain prune revisions.
>
> I didn't do a test for the following case as I don't know how to create the
> marker between B and B' without using evolve:
>
>      O C
>      |
> O B' X B (precursor of B')
> |    |
> O A  O R (conflicting changes with A)
> |    |
> |   /
> |  /
> | /
> |/
> O
>
> $ hg rebase -r "R::" -d B' # aborts
> $ ... # resolve and mark conflicts
> $ hg rebase --continue
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -236,6 +236,7 @@ def rebase(ui, repo, **opts):
>          # keepopen is not meant for use on the command line, but by
>          # other extensions
>          keepopen = opts.get('keepopen', False)
> +        obsoletenotrebased = {}
>
>          if opts.get('interactive'):
>              try:
> @@ -266,7 +267,8 @@ def rebase(ui, repo, **opts):
>
>              try:
>                  (originalwd, target, state, skipped, collapsef, keepf,
> -                 keepbranchesf, external, activebookmark) =
> restorestatus(repo)
> +                 keepbranchesf, external, activebookmark,
> +                 obsoletenotrebased) = restorestatus(repo)
>                  collapsemsg = restorecollapsemsg(repo)
>              except error.RepoLookupError:
>                  if abortf:
> @@ -296,7 +298,6 @@ def rebase(ui, repo, **opts):
>                        " unrebased descendants"),
>                      hint=_('use --keep to keep original changesets'))
>
> -            obsoletenotrebased = {}
>              if ui.configbool('experimental', 'rebaseskipobsolete',
>                               default=True):
>                  rebasesetrevs = set(rebaseset)
> @@ -397,7 +398,8 @@ def rebase(ui, repo, **opts):
>                  p1, p2, base = defineparents(repo, rev, target, state,
>                                               targetancestors)
>                  storestatus(repo, originalwd, target, state, collapsef,
> keepf,
> -                            keepbranchesf, external, activebookmark)
> +                            keepbranchesf, external, activebookmark,
> +                            obsoletenotrebased)
>                  storecollapsemsg(repo, collapsemsg)
>                  if len(repo[None].parents()) == 2:
>                      repo.ui.debug('resuming interrupted rebase\n')
> @@ -888,7 +890,7 @@ def restorecollapsemsg(repo):
>      return collapsemsg
>
>  def storestatus(repo, originalwd, target, state, collapse, keep,
> keepbranches,
> -                external, activebookmark):
> +                external, activebookmark, obsoletenotrebased):
>      'Store the current status to allow recovery'
>      f = repo.vfs("rebasestate", "w")
>      f.write(repo[originalwd].hex() + '\n')
> @@ -898,6 +900,17 @@ def storestatus(repo, originalwd, target
>      f.write('%d\n' % int(keep))
>      f.write('%d\n' % int(keepbranches))
>      f.write('%s\n' % (activebookmark or ''))
> +
> +    # rebaseskipobsolete lines follow the format:
> +    # xOLDHASH:NEWHASH if NEWHASH is nullid, it means no successor
> +    for d, v in obsoletenotrebased.iteritems():
> +        oldrev = repo[d].hex()
> +        if v is not None:
> +            newrev = repo[v].hex()
> +        else:
> +            newrev = hex(nullid)
> +        f.write("x%s:%s\n" %(oldrev, newrev))
> +
>      for d, v in state.iteritems():
>          oldrev = repo[d].hex()
>          if v >= 0:
> @@ -924,6 +937,7 @@ def restorestatus(repo):
>      collapse = False
>      external = nullrev
>      activebookmark = None
> +    obsoletenotrebased = {}
>      state = {}
>
>      try:
> @@ -945,6 +959,14 @@ def restorestatus(repo):
>                  # line 6 is a recent addition, so for backwards
> compatibility
>                  # check that the line doesn't look like the oldrev:newrev
> lines
>                  activebookmark = l
> +            elif i >= 7 and (len(l) == 82 and ':' in l and l[0] == 'x'):
> +                # line 7 is a recent addition, so for backwards
> compatibility
> +                # check that the line doesn't look like the oldrev:newrev
> lines
> +                oldrev, newrev = l[1:].split(':')
> +                if newrev == nullid:
> +                    obsoletenotrebased[repo[oldrev].rev()] = None
> +                else:
> +                    obsoletenotrebased[repo[oldrev].rev()] =
> repo[newrev].rev()
>              else:
>                  oldrev, newrev = l.split(':')
>                  if newrev in (str(nullmerge), str(revignored),
> @@ -977,7 +999,8 @@ def restorestatus(repo):
>      repo.ui.debug('rebase status resumed\n')
>      _setrebasesetvisibility(repo, state.keys())
>      return (originalwd, target, state, skipped,
> -            collapse, keep, keepbranches, external, activebookmark)
> +            collapse, keep, keepbranches, external, activebookmark,
> +            obsoletenotrebased)
>
>  def needupdate(repo, state):
>      '''check whether we should `update --clean` away from a merge, or if
> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
> --- a/tests/test-rebase-obsolete.t
> +++ b/tests/test-rebase-obsolete.t
> @@ -805,3 +805,60 @@ With experimental.allowdivergence=True,
>    phases: 8 draft
>    divergent: 2 changesets
>
> +rebase --continue + skipped rev because their successors are in
> destination
> +we make a change in trunk and work on conflicting changes to make rebase
> abort.
> +
> +  $ hg log -G -r 17::
> +  @  17:61bd55f69bc4 bar foo
> +  |
> +
> +Create the two changes in trunk
> +  $ printf "a" > willconflict
> +  $ hg add willconflict
> +  $ hg commit -m "willconflict first version"
> +
> +  $ printf "dummy" > C
> +  $ hg commit -m "dummy change successor"
> +
> +Create the changes that we will rebase
> +  $ hg update -C 17
> +  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +  $ printf "b" > willconflict
> +  $ hg add willconflict
> +  $ hg commit -m "willconflict second version"
> +  created new head
> +  $ printf "dummy" > K
> +  $ hg add K
> +  $ hg commit -m "dummy change"
> +  $ printf "dummy" > L
> +  $ hg add L
> +  $ hg commit -m "dummy change"
> +  $ hg debugobsolete `hg log -r ".^" -T '{node}\n'` --config
> experimental.evolution=all
> +
> +  $ hg log -G -r 17::
> +  @  22:7bdc8a87673d dummy change
> +  |
> +  x  21:8b31da3c4919 dummy change
> +  |
> +  o  20:b82fb57ea638 willconflict second version
> +  |
> +  | o  19:601db7a18f51 dummy change successor
> +  | |
> +  | o  18:357ddf1602d5 willconflict first version
> +  |/
> +  o  17:61bd55f69bc4 bar foo
> +  |
> +  $ hg rebase -r ".^^ + .^ + ." -d 19
> +  rebasing 20:b82fb57ea638 "willconflict second version"
> +  merging willconflict
> +  warning: conflicts while merging willconflict! (edit, then use 'hg
> resolve --mark')
> +  unresolved conflicts (see hg resolve, then hg rebase --continue)
> +  [1]
> +
> +  $ hg resolve --mark willconflict
> +  (no more unresolved files)
> +  continue: hg rebase --continue
> +  $ hg rebase --continue
> +  rebasing 20:b82fb57ea638 "willconflict second version"
> +  note: not rebasing 21:8b31da3c4919 "dummy change", it has no successor
> +  rebasing 22:7bdc8a87673d "dummy change" (tip)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160325/763574c6/attachment.html>


More information about the Mercurial-devel mailing list