evolve: saner locking an transaction in `hg evolve`
authorPierre-Yves David <pierre-yves.david@fb.com>
Wed, 06 Aug 2014 18:06:17 -0700
changeset 1021 200f2d9b9f39
parent 1020 155949287628
child 1022 6f4fd3e49d1c
evolve: saner locking an transaction in `hg evolve` Each trouble solved used to handle locking and transaction on its own. We now have a top level locking and transaction. This will helps use making sure phase are moved within a transaction.
hgext/evolve.py
--- a/hgext/evolve.py	Wed Aug 06 17:08:51 2014 -0700
+++ b/hgext/evolve.py	Wed Aug 06 18:06:17 2014 -0700
@@ -836,62 +836,52 @@
 
 def relocate(repo, orig, dest):
     """rewrite <rev> on dest"""
+    if orig.rev() == dest.rev():
+        raise util.Abort(_('tried to relocate a node on top of itself'),
+                         hint=_("This shouldn't happen. If you still "
+                                "need to move changesets, please do so "
+                                "manually with nothing to rebase - working "
+                                "directory parent is also destination"))
+
+    rebase = extensions.find('rebase')
+    # dummy state to trick rebase node
+    if not orig.p2().rev() == node.nullrev:
+        raise util.Abort(
+            'no support for evolving merge changesets yet',
+            hint="Redo the merge a use `hg prune` to obsolete the old one")
+    destbookmarks = repo.nodebookmarks(dest.node())
+    nodesrc = orig.node()
+    destphase = repo[nodesrc].phase()
     try:
-        if orig.rev() == dest.rev():
-            raise util.Abort(_('tried to relocate a node on top of itself'),
-                             hint=_("This shouldn't happen. If you still "
-                                    "need to move changesets, please do so "
-                                    "manually with nothing to rebase - working "
-                                    "directory parent is also destination"))
-
-        rebase = extensions.find('rebase')
-        # dummy state to trick rebase node
-        if not orig.p2().rev() == node.nullrev:
+        r = rebase.rebasenode(repo, orig.node(), dest.node(),
+                              {node.nullrev: node.nullrev}, False)
+        if r[-1]: #some conflict
             raise util.Abort(
-                'no support for evolving merge changesets yet',
-                hint="Redo the merge a use `hg prune` to obsolete the old one")
-        destbookmarks = repo.nodebookmarks(dest.node())
-        nodesrc = orig.node()
-        destphase = repo[nodesrc].phase()
-        wlock = lock = None
-        try:
-            wlock = repo.wlock()
-            lock = repo.lock()
-            r = rebase.rebasenode(repo, orig.node(), dest.node(),
-                                  {node.nullrev: node.nullrev}, False)
-            if r[-1]: #some conflict
-                raise util.Abort(
-                        'unresolved merge conflicts (see hg help resolve)')
-            cmdutil.duplicatecopies(repo, orig.node(), dest.node())
-            nodenew = rebase.concludenode(repo, orig.node(), dest.node(),
-                                          node.nullid)
-        except util.Abort, exc:
-            class LocalMergeFailure(MergeFailure, exc.__class__):
-                pass
-            exc.__class__ = LocalMergeFailure
-            raise
-        finally:
-            lockmod.release(lock, wlock)
-        oldbookmarks = repo.nodebookmarks(nodesrc)
-        if nodenew is not None:
-            retractboundary(repo, destphase, [nodenew])
-            createmarkers(repo, [(repo[nodesrc], (repo[nodenew],))])
-            for book in oldbookmarks:
-                repo._bookmarks[book] = nodenew
-        else:
-            createmarkers(repo, [(repo[nodesrc], ())])
-            # Behave like rebase, move bookmarks to dest
-            for book in oldbookmarks:
-                repo._bookmarks[book] = dest.node()
-        for book in destbookmarks: # restore bookmark that rebase move
+                    'unresolved merge conflicts (see hg help resolve)')
+        cmdutil.duplicatecopies(repo, orig.node(), dest.node())
+        nodenew = rebase.concludenode(repo, orig.node(), dest.node(),
+                                      node.nullid)
+    except util.Abort, exc:
+        class LocalMergeFailure(MergeFailure, exc.__class__):
+            pass
+        exc.__class__ = LocalMergeFailure
+        raise
+    oldbookmarks = repo.nodebookmarks(nodesrc)
+    if nodenew is not None:
+        retractboundary(repo, destphase, [nodenew])
+        createmarkers(repo, [(repo[nodesrc], (repo[nodenew],))])
+        for book in oldbookmarks:
+            repo._bookmarks[book] = nodenew
+    else:
+        createmarkers(repo, [(repo[nodesrc], ())])
+        # Behave like rebase, move bookmarks to dest
+        for book in oldbookmarks:
             repo._bookmarks[book] = dest.node()
-        if oldbookmarks or destbookmarks:
-            repo._bookmarks.write()
-        return nodenew
-    except util.Abort:
-        # Invalidate the previous setparents
-        repo.dirstate.invalidate()
-        raise
+    for book in destbookmarks: # restore bookmark that rebase move
+        repo._bookmarks[book] = dest.node()
+    if oldbookmarks or destbookmarks:
+        repo._bookmarks.write()
+    return nodenew
 
 def _bookmarksupdater(repo, oldid):
     """Return a callable update(newid) updating the current bookmark
@@ -1250,7 +1240,16 @@
 
     while tro is not None:
         progresscb()
-        result = _evolveany(ui, repo, tro, dryrunopt, progresscb=progresscb)
+        wlock = lock = tr = None
+        try:
+            wlock = repo.wlock()
+            lock = repo.lock()
+            tr = repo.transaction("evolve")
+            result = _evolveany(ui, repo, tro, dryrunopt,
+                                progresscb=progresscb)
+            tr.close()
+        finally:
+            lockmod.release(tr, lock, wlock)
         progresscb()
         seen += 1
         if not allopt:
@@ -1364,7 +1363,6 @@
     else:
         repo.ui.note(todo)
         if progresscb: progresscb()
-        lock = repo.lock()
         try:
             relocate(repo, orig, target)
         except MergeFailure:
@@ -1373,8 +1371,6 @@
             repo.ui.write_err(
                 _('fix conflict and run "hg evolve --continue"\n'))
             raise
-        finally:
-            lock.release()
 
 def _solvebumped(ui, repo, bumped, dryrun=False, progresscb=None):
     """Stabilize a bumped changeset"""
@@ -1403,83 +1399,75 @@
         repo.ui.write('hg commit --msg "bumped update to %s"')
         return 0
     if progresscb: progresscb()
-    wlock = repo.wlock()
+    newid = tmpctx = None
+    tmpctx = bumped
+    bmupdate = _bookmarksupdater(repo, bumped.node())
+    # Basic check for common parent. Far too complicated and fragile
+    tr = repo.transaction('bumped-stabilize')
     try:
-        newid = tmpctx = None
-        tmpctx = bumped
-        lock = repo.lock()
-        try:
-            bmupdate = _bookmarksupdater(repo, bumped.node())
-            # Basic check for common parent. Far too complicated and fragile
-            tr = repo.transaction('bumped-stabilize')
+        if not list(repo.set('parents(%d) and parents(%d)', bumped, prec)):
+            # Need to rebase the changeset at the right place
+            repo.ui.status(
+                _('rebasing to destination parent: %s\n') % prec.p1())
             try:
-                if not list(repo.set('parents(%d) and parents(%d)', bumped, prec)):
-                    # Need to rebase the changeset at the right place
-                    repo.ui.status(
-                        _('rebasing to destination parent: %s\n') % prec.p1())
-                    try:
-                        tmpid = relocate(repo, bumped, prec.p1())
-                        if tmpid is not None:
-                            tmpctx = repo[tmpid]
-                            createmarkers(repo, [(bumped, (tmpctx,))])
-                    except MergeFailure:
-                        repo.opener.write('graftstate', bumped.hex() + '\n')
-                        repo.ui.write_err(_('evolution failed!\n'))
-                        repo.ui.write_err(
-                            _('fix conflict and run "hg evolve --continue"\n'))
-                        raise
-                # Create the new commit context
-                repo.ui.status(_('computing new diff\n'))
-                files = set()
-                copied = copies.pathcopies(prec, bumped)
-                precmanifest = prec.manifest()
-                for key, val in bumped.manifest().iteritems():
-                    if precmanifest.pop(key, None) != val:
-                        files.add(key)
-                files.update(precmanifest)  # add missing files
-                # commit it
-                if files: # something to commit!
-                    def filectxfn(repo, ctx, path):
-                        if path in bumped:
-                            fctx = bumped[path]
-                            flags = fctx.flags()
-                            mctx = memfilectx(repo, fctx.path(), fctx.data(),
-                                              islink='l' in flags,
-                                              isexec='x' in flags,
-                                              copied=copied.get(path))
-                            return mctx
-                        raise IOError()
-                    text = 'bumped update to %s:\n\n' % prec
-                    text += bumped.description()
-
-                    new = context.memctx(repo,
-                                         parents=[prec.node(), node.nullid],
-                                         text=text,
-                                         files=files,
-                                         filectxfn=filectxfn,
-                                         user=bumped.user(),
-                                         date=bumped.date(),
-                                         extra=bumped.extra())
-
-                    newid = repo.commitctx(new)
-                if newid is None:
-                    createmarkers(repo, [(tmpctx, ())])
-                    newid = prec.node()
-                else:
-                    retractboundary(repo, bumped.phase(), [newid])
-                    createmarkers(repo, [(tmpctx, (repo[newid],))],
-                                           flag=obsolete.bumpedfix)
-                bmupdate(newid)
-                tr.close()
-                repo.ui.status(_('committed as %s\n') % node.short(newid))
-            finally:
-                tr.release()
-        finally:
-            lock.release()
-        # reroute the working copy parent to the new changeset
-        repo.dirstate.setparents(newid, node.nullid)
+                tmpid = relocate(repo, bumped, prec.p1())
+                if tmpid is not None:
+                    tmpctx = repo[tmpid]
+                    createmarkers(repo, [(bumped, (tmpctx,))])
+            except MergeFailure:
+                repo.opener.write('graftstate', bumped.hex() + '\n')
+                repo.ui.write_err(_('evolution failed!\n'))
+                repo.ui.write_err(
+                    _('fix conflict and run "hg evolve --continue"\n'))
+                raise
+        # Create the new commit context
+        repo.ui.status(_('computing new diff\n'))
+        files = set()
+        copied = copies.pathcopies(prec, bumped)
+        precmanifest = prec.manifest()
+        for key, val in bumped.manifest().iteritems():
+            if precmanifest.pop(key, None) != val:
+                files.add(key)
+        files.update(precmanifest)  # add missing files
+        # commit it
+        if files: # something to commit!
+            def filectxfn(repo, ctx, path):
+                if path in bumped:
+                    fctx = bumped[path]
+                    flags = fctx.flags()
+                    mctx = memfilectx(repo, fctx.path(), fctx.data(),
+                                      islink='l' in flags,
+                                      isexec='x' in flags,
+                                      copied=copied.get(path))
+                    return mctx
+                raise IOError()
+            text = 'bumped update to %s:\n\n' % prec
+            text += bumped.description()
+
+            new = context.memctx(repo,
+                                 parents=[prec.node(), node.nullid],
+                                 text=text,
+                                 files=files,
+                                 filectxfn=filectxfn,
+                                 user=bumped.user(),
+                                 date=bumped.date(),
+                                 extra=bumped.extra())
+
+            newid = repo.commitctx(new)
+        if newid is None:
+            createmarkers(repo, [(tmpctx, ())])
+            newid = prec.node()
+        else:
+            retractboundary(repo, bumped.phase(), [newid])
+            createmarkers(repo, [(tmpctx, (repo[newid],))],
+                                   flag=obsolete.bumpedfix)
+        bmupdate(newid)
+        tr.close()
+        repo.ui.status(_('committed as %s\n') % node.short(newid))
     finally:
-        wlock.release()
+        tr.release()
+    # reroute the working copy parent to the new changeset
+    repo.dirstate.setparents(newid, node.nullid)
 
 def _solvedivergent(ui, repo, divergent, dryrun=False, progresscb=None):
     base, others = divergentdata(divergent)
@@ -1537,55 +1525,48 @@
         ui.write('hg commit -m "`hg log -r %s --template={desc}`";\n'
                  % divergent)
         return
-    wlock = lock = None
-    try:
-        wlock = repo.wlock()
-        lock = repo.lock()
-        if divergent not in repo[None].parents():
-            repo.ui.status(_('updating to "local" conflict\n'))
-            hg.update(repo, divergent.rev())
-        repo.ui.note(_('merging divergent changeset\n'))
-        if progresscb: progresscb()
-        stats = merge.update(repo,
-                             other.node(),
-                             branchmerge=True,
-                             force=False,
-                             partial=None,
-                             ancestor=base.node(),
-                             mergeancestor=True)
-        hg._showstats(repo, stats)
-        if stats[3]:
-            repo.ui.status(_("use 'hg resolve' to retry unresolved file merges "
-                             "or 'hg update -C .' to abandon\n"))
-        if stats[3] > 0:
-            raise util.Abort('merge conflict between several amendments '
-                '(this is not automated yet)',
-                hint="""/!\ You can try:
+    if divergent not in repo[None].parents():
+        repo.ui.status(_('updating to "local" conflict\n'))
+        hg.update(repo, divergent.rev())
+    repo.ui.note(_('merging divergent changeset\n'))
+    if progresscb: progresscb()
+    stats = merge.update(repo,
+                         other.node(),
+                         branchmerge=True,
+                         force=False,
+                         partial=None,
+                         ancestor=base.node(),
+                         mergeancestor=True)
+    hg._showstats(repo, stats)
+    if stats[3]:
+        repo.ui.status(_("use 'hg resolve' to retry unresolved file merges "
+                         "or 'hg update -C .' to abandon\n"))
+    if stats[3] > 0:
+        raise util.Abort('merge conflict between several amendments '
+            '(this is not automated yet)',
+            hint="""/!\ You can try:
 /!\ * manual merge + resolve => new cset X
 /!\ * hg up to the parent of the amended changeset (which are named W and Z)
 /!\ * hg revert --all -r X
 /!\ * hg ci -m "same message as the amended changeset" => new cset Y
 /!\ * hg kill -n Y W Z
 """)
-        if progresscb: progresscb()
-        tr = repo.transaction('stabilize-divergent')
-        try:
-            repo.dirstate.setparents(divergent.node(), node.nullid)
-            oldlen = len(repo)
-            amend(ui, repo, message='', logfile='')
-            if oldlen == len(repo):
-                new = divergent
-                # no changes
-            else:
-                new = repo['.']
-            createmarkers(repo, [(other, (new,))])
-            retractboundary(repo, other.phase(), [new.node()])
-            tr.close()
-        finally:
-            tr.release()
+    if progresscb: progresscb()
+    tr = repo.transaction('stabilize-divergent')
+    try:
+        repo.dirstate.setparents(divergent.node(), node.nullid)
+        oldlen = len(repo)
+        amend(ui, repo, message='', logfile='')
+        if oldlen == len(repo):
+            new = divergent
+            # no changes
+        else:
+            new = repo['.']
+        createmarkers(repo, [(other, (new,))])
+        retractboundary(repo, other.phase(), [new.node()])
+        tr.close()
     finally:
-        lockmod.release(lock, wlock)
-
+        tr.release()
 
 def divergentdata(ctx):
     """return base, other part of a conflict