# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1504284370 -7200
# Node ID 95bb27b8918c44ad8052523fc002bbfd11b861ad
# Parent  d617128279f63f15e47c25114d9aaa5662bb1db4
convertbookmark: perform all actions at the end

This will make the whole operation more deterministic and robust.

diff -r d617128279f6 -r 95bb27b8918c hgext3rd/topic/__init__.py
--- a/hgext3rd/topic/__init__.py	Fri Sep 01 18:33:08 2017 +0200
+++ b/hgext3rd/topic/__init__.py	Fri Sep 01 18:46:10 2017 +0200
@@ -458,7 +458,6 @@
         raise error.Abort(_("you must specify either '--all' or '-b'"))
 
     bmstore = repo._bookmarks
-    lock = wlock = tr = None
 
     nodetobook = {}
     for book, revnode in bmstore.iteritems():
@@ -471,38 +470,29 @@
     # warning repeatedly
     skipped = []
 
-    if bookmark:
-        try:
-            node = bmstore[bookmark]
-        except KeyError:
-            raise error.Abort(_("no such bookmark exists: '%s'") % bookmark)
+    actions = {}
 
-        if len(nodetobook[node]) > 1:
-            revnum = repo[node].rev()
-            ui.status(_("skipping revision '%d' as it has multiple bookmarks "
-                      "on it\n") % revnum)
-            return
+    lock = wlock = tr = None
+    try:
+        wlock = repo.wlock()
+        lock = repo.lock()
+        if bookmark:
+            try:
+                node = bmstore[bookmark]
+            except KeyError:
+                raise error.Abort(_("no such bookmark exists: '%s'") % bookmark)
 
-        revnum = repo[node].rev()
-        try:
-            wlock = repo.wlock()
-            lock = repo.lock()
-            tr = repo.transaction('debugconvertbookmark')
+            revnum = repo[node].rev()
+            if len(nodetobook[node]) > 1:
+                ui.status(_("skipping revision '%d' as it has multiple bookmarks "
+                          "on it\n") % revnum)
+                return
             targetrevs = _findconvertbmarktopic(repo, bookmark)
-            _applyconvertbmarktopic(ui, repo, targetrevs, revnum, bookmark, tr)
-            tr.close()
-        finally:
-            lockmod.release(tr, lock, wlock)
+            if targetrevs:
+                actions[(bookmark, revnum)] = targetrevs
 
-    elif convertall:
-        # deletion of bookmark will result in change in size of bmstore during
-        # iteration, so let's make a copy to iterate
-        storecopy = bmstore.copy()
-        try:
-            wlock = repo.wlock()
-            lock = repo.lock()
-            tr = repo.transaction('debugconvertbookmark')
-            for bmark, revnode in sorted(storecopy.iteritems()):
+        elif convertall:
+            for bmark, revnode in sorted(bmstore.iteritems()):
                 revnum = repo[revnode].rev()
                 if revnum in skipped:
                     continue
@@ -514,10 +504,19 @@
                 if bmark == '@':
                     continue
                 targetrevs = _findconvertbmarktopic(repo, bmark)
-                _applyconvertbmarktopic(ui, repo, targetrevs, revnum, bmark, tr)
-            tr.close()
-        finally:
-            lockmod.release(tr, lock, wlock)
+                if targetrevs:
+                    actions[(bmark, revnum)] = targetrevs
+
+        if actions:
+            try:
+                tr = repo.transaction('debugconvertbookmark')
+                for ((bmark, revnum), targetrevs) in sorted(actions.iteritems()):
+                    _applyconvertbmarktopic(ui, repo, targetrevs, revnum, bmark, tr)
+                tr.close()
+            finally:
+                tr.release()
+    finally:
+        lockmod.release(lock, wlock)
 
 # inspired from mercurial.repair.stripbmrevset
 CONVERTBOOKREVSET = """