Rev 5328: (spiv) Fix Branch._unstack to work with repeatedly locked RemoteBranch in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Jul 1 04:41:22 BST 2010


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5328 [merge]
revision-id: pqm at pqm.ubuntu.com-20100701034120-zla513tk7pzf9rzn
parent: pqm at pqm.ubuntu.com-20100630222119-m3e9znqsecsm4tiy
parent: andrew.bennetts at canonical.com-20100701021605-ny3w2a6zmi1yukxl
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2010-07-01 04:41:20 +0100
message:
  (spiv) Fix Branch._unstack to work with repeatedly locked RemoteBranch
   objects. (#551525) (Andrew Bennetts)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
  bzrlib/tests/per_branch/test_stacking.py test_stacking.py-20080214020755-msjlkb7urobwly0f-1
=== modified file 'NEWS'
--- a/NEWS	2010-06-28 21:45:10 +0000
+++ b/NEWS	2010-06-30 08:35:19 +0000
@@ -62,6 +62,9 @@
   or pull location in locations.conf or branch.conf.
   (Gordon Tyler, #534787)
 
+* ``bzr reconfigure --unstacked`` now works with branches accessed via a
+  smart server. (Andrew Bennetts, #551525)
+
 * ``BzrDir.find_branches`` should ignore branches with missing repositories.
   (Marius Kruger, Robert Collins)
 

=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2010-06-20 21:14:49 +0000
+++ b/bzrlib/branch.py	2010-06-30 08:34:11 +0000
@@ -801,28 +801,56 @@
             if len(old_repository._fallback_repositories) != 1:
                 raise AssertionError("can't cope with fallback repositories "
                     "of %r" % (self.repository,))
-            # unlock it, including unlocking the fallback
+            # Open the new repository object.
+            # Repositories don't offer an interface to remove fallback
+            # repositories today; take the conceptually simpler option and just
+            # reopen it.  We reopen it starting from the URL so that we
+            # get a separate connection for RemoteRepositories and can
+            # stream from one of them to the other.  This does mean doing
+            # separate SSH connection setup, but unstacking is not a
+            # common operation so it's tolerable.
+            new_bzrdir = bzrdir.BzrDir.open(self.bzrdir.root_transport.base)
+            new_repository = new_bzrdir.find_repository()
+            if new_repository._fallback_repositories:
+                raise AssertionError("didn't expect %r to have "
+                    "fallback_repositories"
+                    % (self.repository,))
+            # Replace self.repository with the new repository.
+            # Do our best to transfer the lock state (i.e. lock-tokens and
+            # lock count) of self.repository to the new repository.
+            lock_token = old_repository.lock_write().repository_token
+            self.repository = new_repository
+            if isinstance(self, remote.RemoteBranch):
+                # Remote branches can have a second reference to the old
+                # repository that need to be replaced.
+                if self._real_branch is not None:
+                    self._real_branch.repository = new_repository
+            self.repository.lock_write(token=lock_token)
+            if lock_token is not None:
+                old_repository.leave_lock_in_place()
             old_repository.unlock()
+            if lock_token is not None:
+                # XXX: self.repository.leave_lock_in_place() before this
+                # function will not be preserved.  Fortunately that doesn't
+                # affect the current default format (2a), and would be a
+                # corner-case anyway.
+                #  - Andrew Bennetts, 2010/06/30
+                self.repository.dont_leave_lock_in_place()
+            old_lock_count = 0
+            while True:
+                try:
+                    old_repository.unlock()
+                except errors.LockNotHeld:
+                    break
+                old_lock_count += 1
+            if old_lock_count == 0:
+                raise AssertionError(
+                    'old_repository should have been locked at least once.')
+            for i in range(old_lock_count-1):
+                self.repository.lock_write()
+            # Fetch from the old repository into the new.
             old_repository.lock_read()
             try:
-                # Repositories don't offer an interface to remove fallback
-                # repositories today; take the conceptually simpler option and just
-                # reopen it.  We reopen it starting from the URL so that we
-                # get a separate connection for RemoteRepositories and can
-                # stream from one of them to the other.  This does mean doing
-                # separate SSH connection setup, but unstacking is not a
-                # common operation so it's tolerable.
-                new_bzrdir = bzrdir.BzrDir.open(self.bzrdir.root_transport.base)
-                new_repository = new_bzrdir.find_repository()
-                self.repository = new_repository
-                if self.repository._fallback_repositories:
-                    raise AssertionError("didn't expect %r to have "
-                        "fallback_repositories"
-                        % (self.repository,))
-                # this is not paired with an unlock because it's just restoring
-                # the previous state; the lock's released when set_stacked_on_url
-                # returns
-                self.repository.lock_write()
                 # XXX: If you unstack a branch while it has a working tree
                 # with a pending merge, the pending-merged revisions will no
                 # longer be present.  You can (probably) revert and remerge.

=== modified file 'bzrlib/tests/per_branch/test_stacking.py'
--- a/bzrlib/tests/per_branch/test_stacking.py	2010-06-20 11:18:38 +0000
+++ b/bzrlib/tests/per_branch/test_stacking.py	2010-07-01 03:41:20 +0000
@@ -23,8 +23,7 @@
     errors,
     )
 from bzrlib.revision import NULL_REVISION
-from bzrlib.smart import server
-from bzrlib.tests import TestNotApplicable, KnownFailure, transport_util
+from bzrlib.tests import TestNotApplicable, transport_util
 from bzrlib.tests.per_branch import TestCaseWithBranch
 
 
@@ -206,6 +205,40 @@
         self.assertRaises(errors.NotStacked,
             new_branch.get_stacked_on_url)
 
+    def test_unstack_already_locked(self):
+        """Removing the stacked-on branch with an already write-locked branch
+        works.
+
+        This was bug 551525.
+        """
+        try:
+            stacked_bzrdir = self.make_stacked_bzrdir()
+        except unstackable_format_errors, e:
+            raise TestNotApplicable(e)
+        stacked_branch = stacked_bzrdir.open_branch()
+        stacked_branch.lock_write()
+        stacked_branch.set_stacked_on_url(None)
+        stacked_branch.unlock()
+
+    def test_unstack_already_multiple_locked(self):
+        """Unstacking a branch preserves the lock count (even though it
+        replaces the br.repository object).
+
+        This is a more extreme variation of test_unstack_already_locked.
+        """
+        try:
+            stacked_bzrdir = self.make_stacked_bzrdir()
+        except unstackable_format_errors, e:
+            raise TestNotApplicable(e)
+        stacked_branch = stacked_bzrdir.open_branch()
+        stacked_branch.lock_write()
+        stacked_branch.lock_write()
+        stacked_branch.lock_write()
+        stacked_branch.set_stacked_on_url(None)
+        stacked_branch.unlock()
+        stacked_branch.unlock()
+        stacked_branch.unlock()
+
     def make_stacked_bzrdir(self, in_directory=None):
         """Create a stacked branch and return its bzrdir.
 
@@ -221,7 +254,8 @@
         tree = self.make_branch_and_tree(prefix + 'stacked-on')
         tree.commit('Added foo')
         stacked_bzrdir = tree.branch.bzrdir.sprout(
-            prefix + 'stacked', tree.branch.last_revision(), stacked=True)
+            self.get_url(prefix + 'stacked'), tree.branch.last_revision(),
+            stacked=True)
         return stacked_bzrdir
 
     def test_clone_from_stacked_branch_preserve_stacking(self):
@@ -249,7 +283,8 @@
         except unstackable_format_errors, e:
             raise TestNotApplicable(e)
         stacked_bzrdir.open_branch().set_stacked_on_url('../stacked-on')
-        cloned_bzrdir = stacked_bzrdir.clone('cloned', preserve_stacking=True)
+        cloned_bzrdir = stacked_bzrdir.clone(
+            self.get_url('cloned'), preserve_stacking=True)
         self.assertEqual(
             '../dir/stacked-on',
             cloned_bzrdir.open_branch().get_stacked_on_url())




More information about the bazaar-commits mailing list