Bazaar

Merge lp:~spiv/bzr/cross-format-stacking-fetch-562380 into lp:bzr

  1. cross-format-stacking-fetch-562380
  2. Merge into bzr.dev
Proposed by Andrew Bennetts
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~spiv/bzr/cross-format-stacking-fetch-562380
Merge into: lp:bzr
Diff against target: 139 lines (+40/-5)
5 files modified
NEWS (+6/-0)
bzrlib/remote.py (+12/-0)
bzrlib/tests/per_repository_reference/__init__.py (+17/-0)
bzrlib/tests/per_repository_reference/test_default_stacking.py (+0/-1)
bzrlib/tests/test_remote.py (+5/-4)
To merge this branch: bzr merge lp:~spiv/bzr/cross-format-stacking-fetch-562380
Reviewer Review Type Date Requested Status
bzr-core Pending
Review via email: mp+23526@code.launchpad.net

Commit message

Don't allow RemoteRepository to stack on an incompatible-format repository. (#562380)

Description of the change

I think this fixes bug 562380. It certainly fixes a serious bug!

Opening a branch stacked on an incompatible repository via a smart server was not failing as it was supposed to, due to a missing check. Well, if _ensure_real was invoked on the RemoteRepository then it would fail, because the real repository's check would fail it.

The duplication of code between Repository and RemoteRepository is probably partly to blame.

This should probably be backported to the stable releases too.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

Queued on 2010-04-16 07:20:00.794099+00:00 now submitted to PQM.

Revision history for this message
Andrew Bennetts (spiv) wrote :

It was queued on Friday, but still isn't merged. What happened?

Revision history for this message
Robert Collins (lifeless) wrote :

On Mon, Apr 19, 2010 at 12:25 PM, Andrew Bennetts <
<email address hidden>> wrote:

> It was queued on Friday, but still isn't merged. What happened?
>

It should be merging at the moment; you can't see the queue just yet, that
should be up properly shortly.

Revision history for this message
bzr PQM (bzr-pqm) wrote :

Exception processing merge: 'NoneType' object is not iterable

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-04-19 13:04:30 +0000
+++ NEWS 2010-04-20 00:37:07 +0000
@@ -60,6 +60,12 @@
60 which is not installed any more" error.60 which is not installed any more" error.
61 (Martin Pool, James Westby, #528114)61 (Martin Pool, James Westby, #528114)
6262
63* Repositories accessed via a smart server now reject being stacked on a
64 repository in an incompatible format, as is the case when accessing them
65 via other methods. This was causing fetches from those repositories via
66 a smart server (e.g. using ``bzr branch``) to receive invalid data.
67 (Andrew Bennetts, #562380)
68
63* Reset ``siginterrupt`` flag to False every time we handle a signal69* Reset ``siginterrupt`` flag to False every time we handle a signal
64 installed with ``set_signal_handler(..., restart_syscall=True)`` (from70 installed with ``set_signal_handler(..., restart_syscall=True)`` (from
65 ``bzrlib.osutils``. Reduces the likelihood of "Interrupted System Call"71 ``bzrlib.osutils``. Reduces the likelihood of "Interrupted System Call"
6672
=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py 2010-04-19 13:04:30 +0000
+++ bzrlib/remote.py 2010-04-20 00:37:07 +0000
@@ -27,6 +27,7 @@
27 lock,27 lock,
28 lockdir,28 lockdir,
29 repository,29 repository,
30 repository as _mod_repository,
30 revision,31 revision,
31 revision as _mod_revision,32 revision as _mod_revision,
32 static_tuple,33 static_tuple,
@@ -1227,6 +1228,7 @@
1227 # state, so always add a lock here. If a caller passes us a locked1228 # state, so always add a lock here. If a caller passes us a locked
1228 # repository, they are responsible for unlocking it later.1229 # repository, they are responsible for unlocking it later.
1229 repository.lock_read()1230 repository.lock_read()
1231 self._check_fallback_repository(repository)
1230 self._fallback_repositories.append(repository)1232 self._fallback_repositories.append(repository)
1231 # If self._real_repository was parameterised already (e.g. because a1233 # If self._real_repository was parameterised already (e.g. because a
1232 # _real_branch had its get_stacked_on_url method called), then the1234 # _real_branch had its get_stacked_on_url method called), then the
@@ -1237,6 +1239,16 @@
1237 if repository.bzrdir.root_transport.base not in fallback_locations:1239 if repository.bzrdir.root_transport.base not in fallback_locations:
1238 self._real_repository.add_fallback_repository(repository)1240 self._real_repository.add_fallback_repository(repository)
12391241
1242 def _check_fallback_repository(self, repository):
1243 """Check that this repository can fallback to repository safely.
1244
1245 Raise an error if not.
1246
1247 :param repository: A repository to fallback to.
1248 """
1249 return _mod_repository.InterRepository._assert_same_model(
1250 self, repository)
1251
1240 def add_inventory(self, revid, inv, parents):1252 def add_inventory(self, revid, inv, parents):
1241 self._ensure_real()1253 self._ensure_real()
1242 return self._real_repository.add_inventory(revid, inv, parents)1254 return self._real_repository.add_inventory(revid, inv, parents)
12431255
=== modified file 'bzrlib/tests/per_repository_reference/__init__.py'
--- bzrlib/tests/per_repository_reference/__init__.py 2009-07-15 17:51:40 +0000
+++ bzrlib/tests/per_repository_reference/__init__.py 2010-04-20 00:37:07 +0000
@@ -23,6 +23,7 @@
23"""23"""
2424
25from bzrlib import (25from bzrlib import (
26 errors,
26 repository,27 repository,
27 remote,28 remote,
28 )29 )
@@ -66,6 +67,22 @@
66 self.repository_format.__class__)67 self.repository_format.__class__)
6768
6869
70class TestIncompatibleStacking(TestCaseWithRepository):
71
72 def test_add_fallback_repository_rejects_incompatible(self):
73 # Repository.add_fallback_repository raises IncompatibleRepositories if
74 # you take two repositories in different serializations and try to
75 # stack them.
76 if self.make_repository('test')._format.supports_chks:
77 different_fmt = '1.9'
78 else:
79 different_fmt = '2a'
80 repo = self.make_repository('repo', format=different_fmt)
81 referring = self.make_repository('referring')
82 self.assertRaises(errors.IncompatibleRepositories,
83 referring.add_fallback_repository, repo)
84
85
69def external_reference_test_scenarios():86def external_reference_test_scenarios():
70 """Generate test scenarios for repositories supporting external references.87 """Generate test scenarios for repositories supporting external references.
71 """88 """
7289
=== modified file 'bzrlib/tests/per_repository_reference/test_default_stacking.py'
--- bzrlib/tests/per_repository_reference/test_default_stacking.py 2009-06-10 03:56:49 +0000
+++ bzrlib/tests/per_repository_reference/test_default_stacking.py 2010-04-20 00:37:07 +0000
@@ -15,7 +15,6 @@
15# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA15# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
1616
1717
18from bzrlib.smart import server
19from bzrlib.tests.per_repository import TestCaseWithRepository18from bzrlib.tests.per_repository import TestCaseWithRepository
2019
2120
2221
=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py 2010-02-23 07:43:11 +0000
+++ bzrlib/tests/test_remote.py 2010-04-20 00:37:07 +0000
@@ -1223,8 +1223,8 @@
1223 len(branch.repository._real_repository._fallback_repositories))1223 len(branch.repository._real_repository._fallback_repositories))
12241224
1225 def test_get_stacked_on_real_branch(self):1225 def test_get_stacked_on_real_branch(self):
1226 base_branch = self.make_branch('base', format='1.6')1226 base_branch = self.make_branch('base')
1227 stacked_branch = self.make_branch('stacked', format='1.6')1227 stacked_branch = self.make_branch('stacked')
1228 stacked_branch.set_stacked_on_url('../base')1228 stacked_branch.set_stacked_on_url('../base')
1229 reference_format = self.get_repo_format()1229 reference_format = self.get_repo_format()
1230 network_name = reference_format.network_name()1230 network_name = reference_format.network_name()
@@ -1235,7 +1235,7 @@
1235 'success', ('branch', branch_network_name))1235 'success', ('branch', branch_network_name))
1236 client.add_expected_call(1236 client.add_expected_call(
1237 'BzrDir.find_repositoryV3', ('stacked/',),1237 'BzrDir.find_repositoryV3', ('stacked/',),
1238 'success', ('ok', '', 'no', 'no', 'yes', network_name))1238 'success', ('ok', '', 'yes', 'no', 'yes', network_name))
1239 # called twice, once from constructor and then again by us1239 # called twice, once from constructor and then again by us
1240 client.add_expected_call(1240 client.add_expected_call(
1241 'Branch.get_stacked_on_url', ('stacked/',),1241 'Branch.get_stacked_on_url', ('stacked/',),
@@ -2191,12 +2191,13 @@
2191 """2191 """
2192 # Make a repo with a fallback repo, both using a FakeClient.2192 # Make a repo with a fallback repo, both using a FakeClient.
2193 format = remote.response_tuple_to_repo_format(2193 format = remote.response_tuple_to_repo_format(
2194 ('yes', 'no', 'yes', 'fake-network-name'))2194 ('yes', 'no', 'yes', self.get_repo_format().network_name()))
2195 repo, client = self.setup_fake_client_and_repository('quack')2195 repo, client = self.setup_fake_client_and_repository('quack')
2196 repo._format = format2196 repo._format = format
2197 fallback_repo, ignored = self.setup_fake_client_and_repository(2197 fallback_repo, ignored = self.setup_fake_client_and_repository(
2198 'fallback')2198 'fallback')
2199 fallback_repo._client = client2199 fallback_repo._client = client
2200 fallback_repo._format = format
2200 repo.add_fallback_repository(fallback_repo)2201 repo.add_fallback_repository(fallback_repo)
2201 # First the client should ask the primary repo2202 # First the client should ask the primary repo
2202 client.add_expected_call(2203 client.add_expected_call(