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
1=== modified file 'NEWS'
2--- NEWS 2010-04-19 13:04:30 +0000
3+++ NEWS 2010-04-20 00:37:07 +0000
4@@ -60,6 +60,12 @@
5 which is not installed any more" error.
6 (Martin Pool, James Westby, #528114)
7
8+* Repositories accessed via a smart server now reject being stacked on a
9+ repository in an incompatible format, as is the case when accessing them
10+ via other methods. This was causing fetches from those repositories via
11+ a smart server (e.g. using ``bzr branch``) to receive invalid data.
12+ (Andrew Bennetts, #562380)
13+
14 * Reset ``siginterrupt`` flag to False every time we handle a signal
15 installed with ``set_signal_handler(..., restart_syscall=True)`` (from
16 ``bzrlib.osutils``. Reduces the likelihood of "Interrupted System Call"
17
18=== modified file 'bzrlib/remote.py'
19--- bzrlib/remote.py 2010-04-19 13:04:30 +0000
20+++ bzrlib/remote.py 2010-04-20 00:37:07 +0000
21@@ -27,6 +27,7 @@
22 lock,
23 lockdir,
24 repository,
25+ repository as _mod_repository,
26 revision,
27 revision as _mod_revision,
28 static_tuple,
29@@ -1227,6 +1228,7 @@
30 # state, so always add a lock here. If a caller passes us a locked
31 # repository, they are responsible for unlocking it later.
32 repository.lock_read()
33+ self._check_fallback_repository(repository)
34 self._fallback_repositories.append(repository)
35 # If self._real_repository was parameterised already (e.g. because a
36 # _real_branch had its get_stacked_on_url method called), then the
37@@ -1237,6 +1239,16 @@
38 if repository.bzrdir.root_transport.base not in fallback_locations:
39 self._real_repository.add_fallback_repository(repository)
40
41+ def _check_fallback_repository(self, repository):
42+ """Check that this repository can fallback to repository safely.
43+
44+ Raise an error if not.
45+
46+ :param repository: A repository to fallback to.
47+ """
48+ return _mod_repository.InterRepository._assert_same_model(
49+ self, repository)
50+
51 def add_inventory(self, revid, inv, parents):
52 self._ensure_real()
53 return self._real_repository.add_inventory(revid, inv, parents)
54
55=== modified file 'bzrlib/tests/per_repository_reference/__init__.py'
56--- bzrlib/tests/per_repository_reference/__init__.py 2009-07-15 17:51:40 +0000
57+++ bzrlib/tests/per_repository_reference/__init__.py 2010-04-20 00:37:07 +0000
58@@ -23,6 +23,7 @@
59 """
60
61 from bzrlib import (
62+ errors,
63 repository,
64 remote,
65 )
66@@ -66,6 +67,22 @@
67 self.repository_format.__class__)
68
69
70+class 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+
86 def external_reference_test_scenarios():
87 """Generate test scenarios for repositories supporting external references.
88 """
89
90=== modified file 'bzrlib/tests/per_repository_reference/test_default_stacking.py'
91--- bzrlib/tests/per_repository_reference/test_default_stacking.py 2009-06-10 03:56:49 +0000
92+++ bzrlib/tests/per_repository_reference/test_default_stacking.py 2010-04-20 00:37:07 +0000
93@@ -15,7 +15,6 @@
94 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
95
96
97-from bzrlib.smart import server
98 from bzrlib.tests.per_repository import TestCaseWithRepository
99
100
101
102=== modified file 'bzrlib/tests/test_remote.py'
103--- bzrlib/tests/test_remote.py 2010-02-23 07:43:11 +0000
104+++ bzrlib/tests/test_remote.py 2010-04-20 00:37:07 +0000
105@@ -1223,8 +1223,8 @@
106 len(branch.repository._real_repository._fallback_repositories))
107
108 def test_get_stacked_on_real_branch(self):
109- base_branch = self.make_branch('base', format='1.6')
110- stacked_branch = self.make_branch('stacked', format='1.6')
111+ base_branch = self.make_branch('base')
112+ stacked_branch = self.make_branch('stacked')
113 stacked_branch.set_stacked_on_url('../base')
114 reference_format = self.get_repo_format()
115 network_name = reference_format.network_name()
116@@ -1235,7 +1235,7 @@
117 'success', ('branch', branch_network_name))
118 client.add_expected_call(
119 'BzrDir.find_repositoryV3', ('stacked/',),
120- 'success', ('ok', '', 'no', 'no', 'yes', network_name))
121+ 'success', ('ok', '', 'yes', 'no', 'yes', network_name))
122 # called twice, once from constructor and then again by us
123 client.add_expected_call(
124 'Branch.get_stacked_on_url', ('stacked/',),
125@@ -2191,12 +2191,13 @@
126 """
127 # Make a repo with a fallback repo, both using a FakeClient.
128 format = remote.response_tuple_to_repo_format(
129- ('yes', 'no', 'yes', 'fake-network-name'))
130+ ('yes', 'no', 'yes', self.get_repo_format().network_name()))
131 repo, client = self.setup_fake_client_and_repository('quack')
132 repo._format = format
133 fallback_repo, ignored = self.setup_fake_client_and_repository(
134 'fallback')
135 fallback_repo._client = client
136+ fallback_repo._format = format
137 repo.add_fallback_repository(fallback_repo)
138 # First the client should ask the primary repo
139 client.add_expected_call(