Rev 6041: (jam) Merge 2.3 into 2.4, including the fix for bug #835035 in file:///home/pqm/archives/thelove/bzr/2.4/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/2.4/

------------------------------------------------------------
revno: 6041 [merge]
revision-id: pqm at pqm.ubuntu.com-20110830215254-znnsj21xfei9za3v
parent: pqm at pqm.ubuntu.com-20110830094437-y5wu3f1jio1h70jo
parent: john at arbash-meinel.com-20110830105428-3xu4hkvizhfj5k11
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.4
timestamp: Tue 2011-08-30 21:52:54 +0000
message:
  (jam) Merge 2.3 into 2.4, including the fix for bug #835035
modified:
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/tests/per_repository_reference/__init__.py __init__.py-20080220025549-nnm2s80it1lvcwnc-2
  bzrlib/vf_repository.py        vf_repository.py-20110502151858-yh9nnoxpokg86unk-1
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2011-06-18 13:57:17 +0000
+++ b/bzrlib/remote.py	2011-08-30 10:54:28 +0000
@@ -1505,12 +1505,13 @@
         # We need to accumulate additional repositories here, to pass them in
         # on various RPC's.
         #
+        # Make the check before we lock: this raises an exception.
+        self._check_fallback_repository(repository)
         if self.is_locked():
             # We will call fallback.unlock() when we transition to the unlocked
             # state, so always add a lock here. If a caller passes us a locked
             # repository, they are responsible for unlocking it later.
             repository.lock_read()
-        self._check_fallback_repository(repository)
         self._fallback_repositories.append(repository)
         # If self._real_repository was parameterised already (e.g. because a
         # _real_branch had its get_stacked_on_url method called), then the

=== modified file 'bzrlib/tests/per_repository_reference/__init__.py'
--- a/bzrlib/tests/per_repository_reference/__init__.py	2011-05-04 03:43:03 +0000
+++ b/bzrlib/tests/per_repository_reference/__init__.py	2011-08-30 10:54:28 +0000
@@ -66,18 +66,37 @@
 
 class TestIncompatibleStacking(TestCaseWithRepository):
 
-    def test_add_fallback_repository_rejects_incompatible(self):
-        # Repository.add_fallback_repository raises IncompatibleRepositories if
-        # you take two repositories in different serializations and try to
-        # stack them.
-        if self.make_repository('test')._format.supports_chks:
+    def make_repo_and_incompatible_fallback(self):
+        referring = self.make_repository('referring')
+        if referring._format.supports_chks:
             different_fmt = '1.9'
         else:
             different_fmt = '2a'
-        repo = self.make_repository('repo', format=different_fmt)
-        referring = self.make_repository('referring')
-        self.assertRaises(errors.IncompatibleRepositories,
-                referring.add_fallback_repository, repo)
+        fallback = self.make_repository('fallback', format=different_fmt)
+        return referring, fallback
+
+    def test_add_fallback_repository_rejects_incompatible(self):
+        # Repository.add_fallback_repository raises IncompatibleRepositories
+        # if you take two repositories in different serializations and try to
+        # stack them.
+        referring, fallback = self.make_repo_and_incompatible_fallback()
+        self.assertRaises(errors.IncompatibleRepositories,
+                referring.add_fallback_repository, fallback)
+
+    def test_add_fallback_doesnt_leave_fallback_locked(self):
+        # Bug #835035. If the referring repository is locked, it wants to lock
+        # the fallback repository. But if they are incompatible, the referring
+        # repository won't take ownership of the fallback, and thus should not
+        # leave the repository in a locked state.
+        referring, fallback = self.make_repo_and_incompatible_fallback()
+        self.addCleanup(referring.lock_read().unlock)
+        # Assert precondition.
+        self.assertFalse(fallback.is_locked())
+        # Assert action.
+        self.assertRaises(errors.IncompatibleRepositories,
+                referring.add_fallback_repository, fallback)
+        # Assert postcondition.
+        self.assertFalse(fallback.is_locked())
 
 
 def external_reference_test_scenarios():

=== modified file 'bzrlib/vf_repository.py'
--- a/bzrlib/vf_repository.py	2011-06-28 17:25:26 +0000
+++ b/bzrlib/vf_repository.py	2011-08-30 10:54:28 +0000
@@ -918,11 +918,13 @@
         """
         if not self._format.supports_external_lookups:
             raise errors.UnstackableRepositoryFormat(self._format, self.base)
+        # This can raise an exception, so should be done before we lock the
+        # fallback repository.
+        self._check_fallback_repository(repository)
         if self.is_locked():
             # This repository will call fallback.unlock() when we transition to
             # the unlocked state, so we make sure to increment the lock count
             repository.lock_read()
-        self._check_fallback_repository(repository)
         self._fallback_repositories.append(repository)
         self.texts.add_fallback_versioned_files(repository.texts)
         self.inventories.add_fallback_versioned_files(repository.inventories)