Bazaar

Merge lp:~jameinel/bzr/quick-fix into lp:~bzr/bzr/trunk-old

  1. quick-fix
  2. Merge into trunk-old
Proposed by John A Meinel
Status: Merged
Merged at revision: not available
Proposed branch: lp:~jameinel/bzr/quick-fix
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 252 lines
To merge this branch: bzr merge lp:~jameinel/bzr/quick-fix
Reviewer Review Type Date Requested Status
Andrew Bennetts Approve
Review via email: mp+6948@code.launchpad.net
To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

Fixes some issues with fetch + ghosts + smart server
Turns out that my update for gc stacking slightly regressed our ability to push 'bzr.dev' via the smart server, because of how missing inventories and ghosts interact.

This patch adds tests that assert we can fetch via smart server for *all* formats with a revision that has a ghost parent (a test which was missing w/ Andrew's fix). It also adds similar tests for fetching revisions w/ ghosts into a stacked branch. (Which Andrew's original fix did *not* address.)

Both StreamSource and GroupCHKStreamSource were affected by this, though there are still more failures with --dev6 + stacking + smart server code to be fixed. (Namely 'bzr push bzr+ssh://.../new-branch' fails because the smart remote client tries to 'upgrade' the default branch to a hard-coded 1.6.1/1.6-rich-root format repository)

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

I think this looks good. One change you should make: if it does fix more than my previous ghosts+stacking fix as you say, then it deserves a NEWS entry. :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-05-29 11:26:30 +0000
+++ NEWS 2009-06-02 02:35:12 +0000
@@ -18,6 +18,11 @@
18Improvements18Improvements
19************19************
2020
21
22* ``--development6-rich-root`` can now stack. (Modulo some smart-server
23 bugs with stacking and non default formats.)
24 (John Arbash Meinel, #373455)
25
21* Numerous operations are now faster for huge projects, i.e. those26* Numerous operations are now faster for huge projects, i.e. those
22 with a large number of files and/or a large number of revisions,27 with a large number of files and/or a large number of revisions,
23 particularly when the latest development format is used. These28 particularly when the latest development format is used. These
@@ -30,10 +35,6 @@
3035
31 (Ian Clatworthy)36 (Ian Clatworthy)
3237
33* ``--development6-rich-root`` can now stack. (Modulo some smart-server
34 bugs with stacking and non default formats.)
35 (John Arbash Meinel, #373455)
36
3738
38Bug Fixes39Bug Fixes
39*********40*********
@@ -45,6 +46,11 @@
45 ``RemoteRepository`` was handling fallbacks along with the46 ``RemoteRepository`` was handling fallbacks along with the
46 ``_real_repository``. (Andrew Bennetts, John Arbash Meinel, #375496)47 ``_real_repository``. (Andrew Bennetts, John Arbash Meinel, #375496)
4748
49* Fix a small bug with fetching revisions w/ ghosts into a new stacked
50 branch. Not often triggered, because it required ghosts to be part of
51 the fetched revisions, not in the stacked-on ancestry.
52 (John Arbash Meinel)
53
4854
49Documentation55Documentation
50*************56*************
5157
=== modified file 'bzrlib/repofmt/groupcompress_repo.py'
--- bzrlib/repofmt/groupcompress_repo.py 2009-05-29 10:25:37 +0000
+++ bzrlib/repofmt/groupcompress_repo.py 2009-06-02 02:35:12 +0000
@@ -857,7 +857,7 @@
857 self._chk_id_roots = None857 self._chk_id_roots = None
858 self._chk_p_id_roots = None858 self._chk_p_id_roots = None
859859
860 def _get_inventory_stream(self, inventory_keys):860 def _get_inventory_stream(self, inventory_keys, allow_absent=False):
861 """Get a stream of inventory texts.861 """Get a stream of inventory texts.
862862
863 When this function returns, self._chk_id_roots and self._chk_p_id_roots863 When this function returns, self._chk_id_roots and self._chk_p_id_roots
@@ -872,6 +872,11 @@
872 stream = source_vf.get_record_stream(inventory_keys,872 stream = source_vf.get_record_stream(inventory_keys,
873 'groupcompress', True)873 'groupcompress', True)
874 for record in stream:874 for record in stream:
875 if record.storage_kind == 'absent':
876 if allow_absent:
877 continue
878 else:
879 raise errors.NoSuchRevision(self, record.key)
875 bytes = record.get_bytes_as('fulltext')880 bytes = record.get_bytes_as('fulltext')
876 chk_inv = inventory.CHKInventory.deserialise(None, bytes,881 chk_inv = inventory.CHKInventory.deserialise(None, bytes,
877 record.key)882 record.key)
@@ -981,7 +986,11 @@
981 raise AssertionError('Cannot call get_stream_for_missing_keys'986 raise AssertionError('Cannot call get_stream_for_missing_keys'
982 ' untill all of get_stream() has been consumed.')987 ' untill all of get_stream() has been consumed.')
983 # Yield the inventory stream, so we can find the chk stream988 # Yield the inventory stream, so we can find the chk stream
984 yield self._get_inventory_stream(missing_inventory_keys)989 # Some of the missing_keys will be missing because they are ghosts.
990 # As such, we can ignore them. The Sink is required to verify there are
991 # no unavailable texts when the ghost inventories are not filled in.
992 yield self._get_inventory_stream(missing_inventory_keys,
993 allow_absent=True)
985 # We use the empty set for excluded_revision_ids, to make it clear that994 # We use the empty set for excluded_revision_ids, to make it clear that
986 # we want to transmit all referenced chk pages.995 # we want to transmit all referenced chk pages.
987 for stream_info in self._get_filtered_chk_streams(set()):996 for stream_info in self._get_filtered_chk_streams(set()):
988997
=== modified file 'bzrlib/repository.py'
--- bzrlib/repository.py 2009-05-27 15:39:12 +0000
+++ bzrlib/repository.py 2009-06-02 02:35:12 +0000
@@ -4265,8 +4265,12 @@
4265 continue4265 continue
4266 # Ask for full texts always so that we don't need more round trips4266 # Ask for full texts always so that we don't need more round trips
4267 # after this stream.4267 # after this stream.
4268 stream = vf.get_record_stream(keys,4268 # Some of the missing keys are genuinely ghosts, so filter absent
4269 self.to_format._fetch_order, True)4269 # records. The Sink is responsible for doing another check to
4270 # ensure that ghosts don't introduce missing data for future
4271 # fetches.
4272 stream = versionedfile.filter_absent(vf.get_record_stream(keys,
4273 self.to_format._fetch_order, True))
4270 yield substream_kind, stream4274 yield substream_kind, stream
42714275
4272 def inventory_fetch_order(self):4276 def inventory_fetch_order(self):
42734277
=== modified file 'bzrlib/smart/repository.py'
--- bzrlib/smart/repository.py 2009-05-07 05:08:46 +0000
+++ bzrlib/smart/repository.py 2009-06-02 02:35:12 +0000
@@ -436,6 +436,8 @@
436 for record in substream:436 for record in substream:
437 if record.storage_kind in ('chunked', 'fulltext'):437 if record.storage_kind in ('chunked', 'fulltext'):
438 serialised = record_to_fulltext_bytes(record)438 serialised = record_to_fulltext_bytes(record)
439 elif record.storage_kind == 'absent':
440 raise ValueError("Absent factory for %s" % (record.key,))
439 else:441 else:
440 serialised = record.get_bytes_as(record.storage_kind)442 serialised = record.get_bytes_as(record.storage_kind)
441 if serialised:443 if serialised:
442444
=== modified file 'bzrlib/tests/per_repository/test_fetch.py'
--- bzrlib/tests/per_repository/test_fetch.py 2009-05-23 04:55:52 +0000
+++ bzrlib/tests/per_repository/test_fetch.py 2009-06-02 02:35:12 +0000
@@ -23,6 +23,7 @@
23 graph,23 graph,
24 remote,24 remote,
25 repository,25 repository,
26 tests,
26 )27 )
27from bzrlib.inventory import ROOT_ID28from bzrlib.inventory import ROOT_ID
28from bzrlib.tests import TestSkipped29from bzrlib.tests import TestSkipped
@@ -145,7 +146,7 @@
145 ])146 ])
146147
147 def test_fetch_to_rich_root_set_parent_1_parent(self):148 def test_fetch_to_rich_root_set_parent_1_parent(self):
148 # 1 parent rev -> 1 parent 149 # 1 parent rev -> 1 parent
149 self.do_test_fetch_to_rich_root_sets_parents_correctly(150 self.do_test_fetch_to_rich_root_sets_parents_correctly(
150 ((ROOT_ID, 'base'),),151 ((ROOT_ID, 'base'),),
151 [('base', None, [('add', ('', ROOT_ID, 'directory', ''))]),152 [('base', None, [('add', ('', ROOT_ID, 'directory', ''))]),
@@ -300,6 +301,55 @@
300 repo.fetch(tree.branch.repository)301 repo.fetch(tree.branch.repository)
301 repo.fetch(tree.branch.repository)302 repo.fetch(tree.branch.repository)
302303
304 def make_simple_branch_with_ghost(self):
305 builder = self.make_branch_builder('source')
306 builder.start_series()
307 builder.build_snapshot('A-id', None, [
308 ('add', ('', 'root-id', 'directory', None)),
309 ('add', ('file', 'file-id', 'file', 'content\n'))])
310 builder.build_snapshot('B-id', ['A-id', 'ghost-id'], [])
311 builder.finish_series()
312 source_b = builder.get_branch()
313 source_b.lock_read()
314 self.addCleanup(source_b.unlock)
315 return source_b
316
317 def test_fetch_with_ghost(self):
318 source_b = self.make_simple_branch_with_ghost()
319 target = self.make_repository('target')
320 target.lock_write()
321 self.addCleanup(target.unlock)
322 target.fetch(source_b.repository, revision_id='B-id')
323
324 def test_fetch_into_smart_with_ghost(self):
325 trans = self.make_smart_server('target')
326 source_b = self.make_simple_branch_with_ghost()
327 target = self.make_repository('target')
328 # Re-open the repository over the smart protocol
329 target = repository.Repository.open(trans.base)
330 target.lock_write()
331 self.addCleanup(target.unlock)
332 try:
333 target.fetch(source_b.repository, revision_id='B-id')
334 except errors.TokenLockingNotSupported:
335 # The code inside fetch() that tries to lock and then fails, also
336 # causes weird problems with 'lock_not_held' later on...
337 target.lock_read()
338 raise tests.KnownFailure('some repositories fail to fetch'
339 ' via the smart server because of locking issues.')
340
341 def test_fetch_from_smart_with_ghost(self):
342 trans = self.make_smart_server('source')
343 source_b = self.make_simple_branch_with_ghost()
344 target = self.make_repository('target')
345 target.lock_write()
346 self.addCleanup(target.unlock)
347 # Re-open the repository over the smart protocol
348 source = repository.Repository.open(trans.base)
349 source.lock_read()
350 self.addCleanup(source.unlock)
351 target.fetch(source, revision_id='B-id')
352
303353
304class TestSource(TestCaseWithRepository):354class TestSource(TestCaseWithRepository):
305 """Tests for/about the results of Repository._get_source."""355 """Tests for/about the results of Repository._get_source."""
306356
=== modified file 'bzrlib/tests/per_repository_reference/test_fetch.py'
--- bzrlib/tests/per_repository_reference/test_fetch.py 2009-05-29 09:56:12 +0000
+++ bzrlib/tests/per_repository_reference/test_fetch.py 2009-06-02 02:35:12 +0000
@@ -15,7 +15,13 @@
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 server18from bzrlib import (
19 branch,
20 errors,
21 )
22from bzrlib.smart import (
23 server,
24 )
19from bzrlib.tests.per_repository import TestCaseWithRepository25from bzrlib.tests.per_repository import TestCaseWithRepository
2026
2127
@@ -99,3 +105,47 @@
99 final2_b = target_b.bzrdir.sprout('final2',105 final2_b = target_b.bzrdir.sprout('final2',
100 revision_id='C-id').open_branch()106 revision_id='C-id').open_branch()
101 self.assertEqual('C-id', final_b.last_revision())107 self.assertEqual('C-id', final_b.last_revision())
108
109 def make_source_with_ghost_and_stacked_target(self):
110 builder = self.make_branch_builder('source')
111 builder.start_series()
112 builder.build_snapshot('A-id', None, [
113 ('add', ('', 'root-id', 'directory', None)),
114 ('add', ('file', 'file-id', 'file', 'content\n'))])
115 builder.build_snapshot('B-id', ['A-id', 'ghost-id'], [])
116 builder.finish_series()
117 source_b = builder.get_branch()
118 source_b.lock_read()
119 self.addCleanup(source_b.unlock)
120 base = self.make_branch('base')
121 base.pull(source_b, stop_revision='A-id')
122 stacked = self.make_branch('stacked')
123 stacked.set_stacked_on_url('../base')
124 return source_b, base, stacked
125
126 def test_fetch_with_ghost_stacked(self):
127 (source_b, base,
128 stacked) = self.make_source_with_ghost_and_stacked_target()
129 stacked.pull(source_b, stop_revision='B-id')
130
131 def test_fetch_into_smart_stacked_with_ghost(self):
132 (source_b, base,
133 stacked) = self.make_source_with_ghost_and_stacked_target()
134 # Now, create a smart server on 'stacked' and re-open to force the
135 # target to be a smart target
136 trans = self.make_smart_server('stacked')
137 stacked = branch.Branch.open(trans.base)
138 stacked.lock_write()
139 self.addCleanup(stacked.unlock)
140 stacked.pull(source_b, stop_revision='B-id')
141
142 def test_fetch_to_stacked_from_smart_with_ghost(self):
143 (source_b, base,
144 stacked) = self.make_source_with_ghost_and_stacked_target()
145 # Now, create a smart server on 'source' and re-open to force the
146 # target to be a smart target
147 trans = self.make_smart_server('source')
148 source_b = branch.Branch.open(trans.base)
149 source_b.lock_read()
150 self.addCleanup(source_b.unlock)
151 stacked.pull(source_b, stop_revision='B-id')