Merge lp:~spiv/bzr/reconcile-cleanup-503878 into lp:bzr

[Andrew Bennetts]

[Andrew Bennetts (spiv)]

This replaces some try/finally blocks in reconcile.py with more robust add_cleanup calls using OperationWithCleanups. This fixes bug 503878 and probably a few unreported ones like it.

(This borrows the definition of OperationWithCleanups.run_simple from <https://code.edge.launchpad.net/~spiv/bzr/command-cleanup/+merge/16943>)

[John A Meinel (jameinel)]

review: Approve

=== modified file 'NEWS'
--- NEWS	2010-01-13 23:26:10 +0000
+++ NEWS	2010-01-13 23:33:13 +0000
@@ -71,6 +71,9 @@
   returns ``EINTR`` by calling ``PyErr_CheckSignals``.  This affected the
   optional ``_readdir_pyx`` extension.  (Andrew Bennetts, #495023)
 
+* Fix "Too many concurrent requests" in reconcile when network connection
+  fails.  (Andrew Bennetts, #503878)
+
 * Fixed a side effect mutation of ``RemoteBzrDirFormat._network_name``
   that caused some tests to fail when run in a non-default order.
   Probably no user impact.  (Martin Pool, #504102)
 
=== modified file 'bzrlib/reconcile.py'
--- bzrlib/reconcile.py	2009-09-24 04:54:19 +0000
+++ bzrlib/reconcile.py	2010-01-13 23:33:13 +0000
@@ -27,12 +27,12 @@
 
 
 from bzrlib import (
+    cleanup,
     errors,
     ui,
     repository,
-    repofmt,
     )
-from bzrlib.trace import mutter, note
+from bzrlib.trace import mutter
 from bzrlib.tsort import topo_sort
 from bzrlib.versionedfile import AdapterFactory, FulltextContentFactory
 
@@ -120,15 +120,16 @@
         self.branch = a_branch
 
     def reconcile(self):
+        operation = cleanup.OperationWithCleanups(self._reconcile)
+        self.add_cleanup = operation.add_cleanup
+        operation.run_simple()
+
+    def _reconcile(self):
         self.branch.lock_write()
-        try:
-            self.pb = ui.ui_factory.nested_progress_bar()
-            try:
-                self._reconcile_steps()
-            finally:
-                self.pb.finished()
-        finally:
-            self.branch.unlock()
+        self.add_cleanup(self.branch.unlock)
+        self.pb = ui.ui_factory.nested_progress_bar()
+        self.add_cleanup(self.pb.finished)
+        self._reconcile_steps()
 
     def _reconcile_steps(self):
         self._reconcile_revision_history()
@@ -192,15 +193,16 @@
         garbage_inventories: The number of inventory objects without revisions
                              that were garbage collected.
         """
+        operation = cleanup.OperationWithCleanups(self._reconcile)
+        self.add_cleanup = operation.add_cleanup
+        operation.run_simple()
+
+    def _reconcile(self):
         self.repo.lock_write()
-        try:
-            self.pb = ui.ui_factory.nested_progress_bar()
-            try:
-                self._reconcile_steps()
-            finally:
-                self.pb.finished()
-        finally:
-            self.repo.unlock()
+        self.add_cleanup(self.repo.unlock)
+        self.pb = ui.ui_factory.nested_progress_bar()
+        self.add_cleanup(self.pb.finished)
+        self._reconcile_steps()
 
     def _reconcile_steps(self):
         """Perform the steps to reconcile this repository."""
@@ -502,23 +504,21 @@
         collection = self.repo._pack_collection
         collection.ensure_loaded()
         collection.lock_names()
-        try:
-            packs = collection.all_packs()
-            all_revisions = self.repo.all_revision_ids()
-            total_inventories = len(list(
-                collection.inventory_index.combined_index.iter_all_entries()))
-            if len(all_revisions):
-                new_pack =  self.repo._reconcile_pack(collection, packs,
-                    ".reconcile", all_revisions, self.pb)
-                if new_pack is not None:
-                    self._discard_and_save(packs)
-            else:
-                # only make a new pack when there is data to copy.
+        self.add_cleanup(collection._unlock_names)
+        packs = collection.all_packs()
+        all_revisions = self.repo.all_revision_ids()
+        total_inventories = len(list(
+            collection.inventory_index.combined_index.iter_all_entries()))
+        if len(all_revisions):
+            new_pack =  self.repo._reconcile_pack(collection, packs,
+                ".reconcile", all_revisions, self.pb)
+            if new_pack is not None:
                 self._discard_and_save(packs)
-            self.garbage_inventories = total_inventories - len(list(
-                collection.inventory_index.combined_index.iter_all_entries()))
-        finally:
-            collection._unlock_names()
+        else:
+            # only make a new pack when there is data to copy.
+            self._discard_and_save(packs)
+        self.garbage_inventories = total_inventories - len(list(
+            collection.inventory_index.combined_index.iter_all_entries()))
 
     def _discard_and_save(self, packs):
         """Discard some packs from the repository.