Rev 4769: (jameinel) Fix bug #437003. Don't abort an auto-pack if an inventory is in in file:///home/pqm/archives/thelove/bzr/2.0/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Tue Mar 8 13:41:55 UTC 2011
At file:///home/pqm/archives/thelove/bzr/2.0/
------------------------------------------------------------
revno: 4769 [merge]
revision-id: pqm at pqm.ubuntu.com-20110308134153-t9v2zggqfynqsz18
parent: pqm at pqm.ubuntu.com-20101216104429-qk47pgj62ekxvdhw
parent: john at arbash-meinel.com-20110308115515-qyci0hdb9s0g7bvb
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.0
timestamp: Tue 2011-03-08 13:41:53 +0000
message:
(jameinel) Fix bug #437003. Don't abort an auto-pack if an inventory is in
the repository,
just not in the subset of packs we are handling. (John A Meinel)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/repofmt/groupcompress_repo.py repofmt.py-20080715094215-wp1qfvoo7093c8qr-1
bzrlib/tests/test_repository.py test_repository.py-20060131075918-65c555b881612f4d
=== modified file 'NEWS'
--- a/NEWS 2010-12-16 09:01:41 +0000
+++ b/NEWS 2011-03-08 11:48:17 +0000
@@ -28,6 +28,10 @@
* Avoid UnicodeDecodeError in ``bzr add`` with multiple files under a non-ascii
path on windows from symlink support addition. (Martin [gz], #686611)
+* Avoid spurious ValueErrors when autopacking a subset of the repository,
+ and seeing a revision without its related inventory
+ (John Arbash Meinel, #437003)
+
Improvements
************
=== modified file 'bzrlib/repofmt/groupcompress_repo.py'
--- a/bzrlib/repofmt/groupcompress_repo.py 2010-01-21 16:59:21 +0000
+++ b/bzrlib/repofmt/groupcompress_repo.py 2011-03-08 11:48:17 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2008, 2009, 2010 Canonical Ltd
+# Copyright (C) 2008-2011 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -421,9 +421,18 @@
inventory_keys = source_vf.keys()
missing_inventories = set(self.revision_keys).difference(inventory_keys)
if missing_inventories:
- missing_inventories = sorted(missing_inventories)
- raise ValueError('We are missing inventories for revisions: %s'
- % (missing_inventories,))
+ # Go back to the original repo, to see if these are really missing
+ # https://bugs.launchpad.net/bzr/+bug/437003
+ # If we are packing a subset of the repo, it is fine to just have
+ # the data in another Pack file, which is not included in this pack
+ # operation.
+ inv_index = self._pack_collection.repo.inventories._index
+ pmap = inv_index.get_parent_map(missing_inventories)
+ really_missing = missing_inventories.difference(pmap)
+ if really_missing:
+ missing_inventories = sorted(really_missing)
+ raise ValueError('We are missing inventories for revisions: %s'
+ % (missing_inventories,))
self._copy_stream(source_vf, target_vf, inventory_keys,
'inventories', self._get_filtered_inv_stream, 2)
=== modified file 'bzrlib/tests/test_repository.py'
--- a/bzrlib/tests/test_repository.py 2010-01-21 19:24:26 +0000
+++ b/bzrlib/tests/test_repository.py 2011-03-08 11:55:15 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006-2010 Canonical Ltd
+# Copyright (C) 2006-2011 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -1564,6 +1564,106 @@
self.assertTrue(new_pack.signature_index._optimize_for_size)
+class TestGCCHKPacker(TestCaseWithTransport):
+
+ def make_abc_branch(self):
+ builder = self.make_branch_builder('source')
+ builder.start_series()
+ builder.build_snapshot('A', None, [
+ ('add', ('', 'root-id', 'directory', None)),
+ ('add', ('file', 'file-id', 'file', 'content\n')),
+ ])
+ builder.build_snapshot('B', ['A'], [
+ ('add', ('dir', 'dir-id', 'directory', None))])
+ builder.build_snapshot('C', ['B'], [
+ ('modify', ('file-id', 'new content\n'))])
+ builder.finish_series()
+ return builder.get_branch()
+
+ def make_branch_with_disjoint_inventory_and_revision(self):
+ """a repo with separate packs for a revisions Revision and Inventory.
+
+ There will be one pack file that holds the Revision content, and one
+ for the Inventory content.
+
+ :return: (repository,
+ pack_name_with_rev_A_Revision,
+ pack_name_with_rev_A_Inventory,
+ pack_name_with_rev_C_content)
+ """
+ b_source = self.make_abc_branch()
+ b_base = b_source.bzrdir.sprout('base', revision_id='A').open_branch()
+ b_stacked = b_base.bzrdir.sprout('stacked', stacked=True).open_branch()
+ b_stacked.lock_write()
+ self.addCleanup(b_stacked.unlock)
+ b_stacked.fetch(b_source, 'B')
+ # Now re-open the stacked repo directly (no fallbacks) so that we can
+ # fill in the A rev.
+ repo_not_stacked = b_stacked.bzrdir.open_repository()
+ repo_not_stacked.lock_write()
+ self.addCleanup(repo_not_stacked.unlock)
+ # Now we should have a pack file with A's inventory, but not its
+ # Revision
+ self.assertEqual([('A',), ('B',)],
+ sorted(repo_not_stacked.inventories.keys()))
+ self.assertEqual([('B',)],
+ sorted(repo_not_stacked.revisions.keys()))
+ stacked_pack_names = repo_not_stacked._pack_collection.names()
+ # We have a couple names here, figure out which has A's inventory
+ for name in stacked_pack_names:
+ pack = repo_not_stacked._pack_collection.get_pack_by_name(name)
+ keys = [n[1] for n in pack.inventory_index.iter_all_entries()]
+ if ('A',) in keys:
+ inv_a_pack_name = name
+ break
+ else:
+ self.fail('Could not find pack containing A\'s inventory')
+ repo_not_stacked.fetch(b_source.repository, 'A')
+ self.assertEqual([('A',), ('B',)],
+ sorted(repo_not_stacked.revisions.keys()))
+ new_pack_names = set(repo_not_stacked._pack_collection.names())
+ rev_a_pack_names = new_pack_names.difference(stacked_pack_names)
+ self.assertEqual(1, len(rev_a_pack_names))
+ rev_a_pack_name = list(rev_a_pack_names)[0]
+ # Now fetch 'C', so we have a couple pack files to join
+ repo_not_stacked.fetch(b_source.repository, 'C')
+ rev_c_pack_names = set(repo_not_stacked._pack_collection.names())
+ rev_c_pack_names = rev_c_pack_names.difference(new_pack_names)
+ self.assertEqual(1, len(rev_c_pack_names))
+ rev_c_pack_name = list(rev_c_pack_names)[0]
+ return (repo_not_stacked, rev_a_pack_name, inv_a_pack_name,
+ rev_c_pack_name)
+
+ def test_pack_with_distant_inventories(self):
+ # See https://bugs.launchpad.net/bzr/+bug/437003
+ # When repacking, it is possible to have an inventory in a different
+ # pack file than the associated revision. An autopack can then come
+ # along, and miss that inventory, and complain.
+ (repo, rev_a_pack_name, inv_a_pack_name, rev_c_pack_name
+ ) = self.make_branch_with_disjoint_inventory_and_revision()
+ a_pack = repo._pack_collection.get_pack_by_name(rev_a_pack_name)
+ c_pack = repo._pack_collection.get_pack_by_name(rev_c_pack_name)
+ packer = groupcompress_repo.GCCHKPacker(repo._pack_collection,
+ [a_pack, c_pack], '.test-pack')
+ # This would raise ValueError in bug #437003, but should not raise an
+ # error once fixed.
+ packer.pack()
+
+ def test_pack_with_missing_inventory(self):
+ # Similar to test_pack_with_missing_inventory, but this time, we force
+ # the A inventory to actually be gone from the repository.
+ (repo, rev_a_pack_name, inv_a_pack_name, rev_c_pack_name
+ ) = self.make_branch_with_disjoint_inventory_and_revision()
+ inv_a_pack = repo._pack_collection.get_pack_by_name(inv_a_pack_name)
+ repo._pack_collection._remove_pack_from_memory(inv_a_pack)
+ packer = groupcompress_repo.GCCHKPacker(repo._pack_collection,
+ repo._pack_collection.all_packs(), '.test-pack')
+ e = self.assertRaises(ValueError, packer.pack)
+ packer.new_pack.abort()
+ self.assertContainsRe(str(e),
+ r"We are missing inventories for revisions: .*'A'")
+
+
class TestCrossFormatPacks(TestCaseWithTransport):
def log_pack(self, hint=None):
More information about the bazaar-commits
mailing list