Rev 2845: Review feedback. in http://people.ubuntu.com/~robertc/baz2.0/repository
Robert Collins
robertc at robertcollins.net
Fri Oct 19 01:57:21 BST 2007
At http://people.ubuntu.com/~robertc/baz2.0/repository
------------------------------------------------------------
revno: 2845
revision-id: robertc at robertcollins.net-20071019005716-owvnvr01kgswmeaq
parent: robertc at robertcollins.net-20071018013219-m98nsombwidgpd9d
committer: Robert Collins <robertc at robertcollins.net>
branch nick: repository
timestamp: Fri 2007-10-19 10:57:16 +1000
message:
Review feedback.
modified:
bzrlib/repofmt/pack_repo.py pack_repo.py-20070813041115-gjv5ma7ktfqwsjgn-1
bzrlib/tests/test_repository.py test_repository.py-20060131075918-65c555b881612f4d
=== modified file 'bzrlib/repofmt/pack_repo.py'
--- a/bzrlib/repofmt/pack_repo.py 2007-10-18 00:30:57 +0000
+++ b/bzrlib/repofmt/pack_repo.py 2007-10-19 00:57:16 +0000
@@ -324,6 +324,12 @@
self.upload_transport.rename(self.random_name,
'../packs/' + self.name + '.pack')
self._state = 'finished'
+ if 'fetch' in debug.debug_flags:
+ # XXX: size might be interesting?
+ mutter('%s: create_pack: pack renamed into place: %s%s->%s%s t+%6.3fs',
+ time.ctime(), self.upload_transport.base, self.random_name,
+ self.pack_transport, self.name,
+ time.time() - self.start_time)
def make_index(self, index_type):
"""Construct a GraphIndex object for this packs index 'index_type'."""
@@ -700,17 +706,6 @@
return None
new_pack.finish()
self.allocate(new_pack)
- if 'fetch' in debug.debug_flags:
- # XXX: size might be interesting?
- mutter('%s: create_pack: pack renamed into place: %s%s->%s%s t+%6.3fs',
- time.ctime(), self._upload_transport.base, new_pack.random_name,
- new_pack.pack_transport, new_pack.name,
- time.time() - new_pack.start_time)
- if 'fetch' in debug.debug_flags:
- # XXX: size might be interesting?
- mutter('%s: create_pack: finished: %s%s t+%6.3fs',
- time.ctime(), self._upload_transport.base, new_pack.random_name,
- time.time() - new_pack.start_time)
return new_pack
def _execute_pack_operations(self, pack_operations):
@@ -1231,12 +1226,6 @@
class GraphKnitRevisionStore(KnitRevisionStore):
"""An object to adapt access from RevisionStore's to use GraphKnits.
- This should not live through to production: by production time we should
- have fully integrated the new indexing and have new data for the
- repository classes; also we may choose not to do a Knit1 compatible
- new repository, just a Knit3 one. If neither of these happen, this
- should definately be cleaned up before merging.
-
This class works by replacing the original RevisionStore.
We need to do this because the GraphKnitRevisionStore is less
isolated in its layering - it uses services from the repo.
@@ -1455,11 +1444,11 @@
return self
def _refresh_data(self):
- if self._write_lock_count == 1 or self.control_files._lock_count==1:
+ if self._write_lock_count==1 or self.control_files._lock_count==1:
# forget what names there are
self._packs.reset()
- # XXX: Better to do an in-memory merge, factor out code from
- # _save_pack_names.
+ # XXX: Better to do an in-memory merge when acquiring a new lock -
+ # factor out code from _save_pack_names.
def _start_write_group(self):
self._packs._start_write_group()
=== modified file 'bzrlib/tests/test_repository.py'
--- a/bzrlib/tests/test_repository.py 2007-10-17 09:39:41 +0000
+++ b/bzrlib/tests/test_repository.py 2007-10-19 00:57:16 +0000
@@ -748,12 +748,6 @@
# in case of side effects of locking.
repo.lock_write()
repo.unlock()
- # we want:
- # format 'Bazaar Experimental'
- # lock: is a directory
- # inventory.weave == empty_weave
- # empty revision-store directory
- # empty weaves directory
t = repo.bzrdir.get_repository_transport(None)
self.check_format(t)
# XXX: no locks left when unlocked at the moment
@@ -793,21 +787,18 @@
list(GraphIndex(t, 'pack-names', None).iter_all_entries()))
self.assertTrue(S_ISDIR(t.stat('packs').st_mode))
self.assertTrue(S_ISDIR(t.stat('upload').st_mode))
+ self.assertTrue(S_ISDIR(t.stat('indices').st_mode))
+ self.assertTrue(S_ISDIR(t.stat('obsolete_packs').st_mode))
def test_shared_disk_layout(self):
format = self.get_format()
repo = self.make_repository('.', shared=True, format=format)
# we want:
- # format 'Bazaar-NG Knit Repository Format 1'
- # lock: is a directory
- # inventory.weave == empty_weave
- # empty revision-store directory
- # empty weaves directory
- # a 'shared-storage' marker file.
t = repo.bzrdir.get_repository_transport(None)
self.check_format(t)
# XXX: no locks left when unlocked at the moment
# self.assertEqualDiff('', t.get('lock').read())
+ # We should have a 'shared-storage' marker file.
self.assertEqualDiff('', t.get('shared-storage').read())
self.check_databases(t)
@@ -816,18 +807,15 @@
repo = self.make_repository('.', shared=True, format=format)
repo.set_make_working_trees(False)
# we want:
- # format 'Bazaar-NG Knit Repository Format 1'
- # lock ''
- # inventory.weave == empty_weave
- # empty revision-store directory
- # empty weaves directory
- # a 'shared-storage' marker file.
t = repo.bzrdir.get_repository_transport(None)
self.check_format(t)
# XXX: no locks left when unlocked at the moment
# self.assertEqualDiff('', t.get('lock').read())
+ # We should have a 'shared-storage' marker file.
self.assertEqualDiff('', t.get('shared-storage').read())
+ # We should have a marker for the no-working-trees flag.
self.assertEqualDiff('', t.get('no-working-trees').read())
+ # The marker should go when we toggle the setting.
repo.set_make_working_trees(True)
self.assertFalse(t.has('no-working-trees'))
self.check_databases(t)
@@ -840,8 +828,9 @@
list(GraphIndex(trans, 'pack-names', None).iter_all_entries()))
tree.commit('foobarbaz')
index = GraphIndex(trans, 'pack-names', None)
- self.assertEqual(1, len(list(index.iter_all_entries())))
- node = list(index.iter_all_entries())[0]
+ index_nodes = list(index.iter_all_entries())
+ self.assertEqual(1, len(index_nodes))
+ node = index_nodes[0]
name = node[1][0]
# the pack sizes should be listed in the index
pack_value = node[2]
@@ -865,7 +854,7 @@
trans = tree.branch.repository.bzrdir.get_repository_transport(None)
# This test could be a little cheaper by replacing the packs
# attribute on the repository to allow a different pack distribution
- # and max packs policy - so we are hecking the policy is honoured
+ # and max packs policy - so we are checking the policy is honoured
# in the test. But for now 11 commits is not a big deal in a single
# test.
for x in range(9):
@@ -898,7 +887,7 @@
tree.commit('start')
tree.commit('more work')
tree.branch.repository.pack()
- # there should be 1 packs:
+ # there should be 1 pack:
index = GraphIndex(trans, 'pack-names', None)
self.assertEqual(1, len(list(index.iter_all_entries())))
self.assertEqual(2, len(tree.branch.repository.all_revision_ids()))
@@ -949,12 +938,13 @@
try:
r1.commit_write_group()
except:
+ r1.abort_write_group()
r2.abort_write_group()
raise
r2.commit_write_group()
# tell r1 to reload from disk
r1._packs.reset()
- # Now both repositories should now about both names
+ # Now both repositories should know about both names
r1._packs.ensure_loaded()
r2._packs.ensure_loaded()
self.assertEqual(r1._packs.names(), r2._packs.names())
@@ -1041,6 +1031,8 @@
self.assertFalse(repo.get_physical_lock_status())
def prepare_for_break_lock(self):
+ # Setup the global ui factory state so that a break-lock method call
+ # will find usable input in the input stream.
old_factory = bzrlib.ui.ui_factory
def restoreFactory():
bzrlib.ui.ui_factory = old_factory
@@ -1083,7 +1075,7 @@
return bzrdir.format_registry.make_bzrdir('experimental')
def test__max_pack_count(self):
- """The maximum pack count is geared from the number of revisions."""
+ """The maximum pack count is a function of the number of revisions."""
format = self.get_format()
repo = self.make_repository('.', format=format)
packs = repo._packs
@@ -1242,8 +1234,7 @@
sig_index = GraphIndex(packs._index_transport, name + '.six',
packs._names[name][3])
self.assertEqual(pack_repo.ExistingPack(packs._pack_transport,
- packs.names()[0], rev_index, inv_index, txt_index, sig_index),
- pack_1)
+ name, rev_index, inv_index, txt_index, sig_index), pack_1)
# and the same instance should be returned on successive calls.
self.assertTrue(pack_1 is packs.get_pack_by_name(name))
More information about the bazaar-commits
mailing list