Rev 4608: Merge bzr.dev. in http://bazaar.launchpad.net/~lifeless/bzr/bug-398668

Robert Collins robertc at robertcollins.net
Thu Aug 13 01:02:11 BST 2009


At http://bazaar.launchpad.net/~lifeless/bzr/bug-398668

------------------------------------------------------------
revno: 4608 [merge]
revision-id: robertc at robertcollins.net-20090813000208-436r0w9dctbqabcx
parent: robertc at robertcollins.net-20090812235215-anxhe8w6caawgpin
parent: pqm at pqm.ubuntu.com-20090812222828-fdlyshwsomwvfaf2
committer: Robert Collins <robertc at robertcollins.net>
branch nick: bug-398668
timestamp: Thu 2009-08-13 10:02:08 +1000
message:
  Merge bzr.dev.
renamed:
  bzrlib/tests/test_pack_repository.py => bzrlib/tests/per_pack_repository.py test_pack_repository-20080801043947-eaw0e6h2gu75kwmy-1
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/repofmt/groupcompress_repo.py repofmt.py-20080715094215-wp1qfvoo7093c8qr-1
  bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/per_interrepository/__init__.py __init__.py-20060220054744-baf49a1f88f17b1a
  bzrlib/tests/per_interrepository/test_fetch.py test_fetch.py-20080425213627-j60cjh782ufm83ry-1
  bzrlib/tests/per_pack_repository.py test_pack_repository-20080801043947-eaw0e6h2gu75kwmy-1
=== modified file 'NEWS'
--- a/NEWS	2009-08-12 02:59:14 +0000
+++ b/NEWS	2009-08-12 22:28:28 +0000
@@ -27,6 +27,11 @@
 * Further tweaks to handling of ``bzr add`` messages about ignored files.
   (Jason Spashett, #76616)
 
+* Properly handle fetching into a stacked branch while converting the
+  data, especially when there are also ghosts. The code was filling in
+  parent inventories incorrectly, and also not handling when one of the
+  parents was a ghost. (John Arbash Meinel, #402778, #412198)
+
 Improvements
 ************
 

=== modified file 'bzrlib/repofmt/groupcompress_repo.py'
--- a/bzrlib/repofmt/groupcompress_repo.py	2009-08-04 04:36:34 +0000
+++ b/bzrlib/repofmt/groupcompress_repo.py	2009-08-12 18:58:05 +0000
@@ -410,7 +410,18 @@
 
     def _copy_inventory_texts(self):
         source_vf, target_vf = self._build_vfs('inventory', True, True)
-        self._copy_stream(source_vf, target_vf, self.revision_keys,
+        # It is not sufficient to just use self.revision_keys, as stacked
+        # repositories can have more inventories than they have revisions.
+        # One alternative would be to do something with
+        # get_parent_map(self.revision_keys), but that shouldn't be any faster
+        # than this.
+        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,))
+        self._copy_stream(source_vf, target_vf, inventory_keys,
                           'inventories', self._get_filtered_inv_stream, 2)
 
     def _copy_chk_texts(self):
@@ -1110,7 +1121,7 @@
 
 class RepositoryFormat2a(RepositoryFormatCHK2):
     """A CHK repository that uses the bencode revision serializer.
-    
+
     This is the same as RepositoryFormatCHK2 but with a public name.
     """
 

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2009-08-11 02:45:36 +0000
+++ b/bzrlib/repository.py	2009-08-12 22:28:28 +0000
@@ -3812,6 +3812,8 @@
             # for the new revisions that we are about to insert.  We do this
             # before adding the revisions so that no revision is added until
             # all the inventories it may depend on are added.
+            # Note that this is overzealous, as we may have fetched these in an
+            # earlier batch.
             parent_ids = set()
             revision_ids = set()
             for revision in pending_revisions:
@@ -3820,10 +3822,13 @@
             parent_ids.difference_update(revision_ids)
             parent_ids.discard(_mod_revision.NULL_REVISION)
             parent_map = self.source.get_parent_map(parent_ids)
-            for parent_tree in self.source.revision_trees(parent_ids):
-                basis_id, delta = self._get_delta_for_revision(tree, parent_ids, basis_id, cache)
+            # we iterate over parent_map and not parent_ids because we don't
+            # want to try copying any revision which is a ghost
+            for parent_tree in self.source.revision_trees(parent_map):
                 current_revision_id = parent_tree.get_revision_id()
                 parents_parents = parent_map[current_revision_id]
+                basis_id, delta = self._get_delta_for_revision(parent_tree,
+                    parents_parents, basis_id, cache)
                 self.target.add_inventory_by_delta(
                     basis_id, delta, current_revision_id, parents_parents)
         # insert signatures and revisions

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2009-08-04 11:40:59 +0000
+++ b/bzrlib/tests/__init__.py	2009-08-12 18:49:22 +0000
@@ -3386,6 +3386,7 @@
                    'bzrlib.tests.per_lock',
                    'bzrlib.tests.per_transport',
                    'bzrlib.tests.per_tree',
+                   'bzrlib.tests.per_pack_repository',
                    'bzrlib.tests.per_repository',
                    'bzrlib.tests.per_repository_chk',
                    'bzrlib.tests.per_repository_reference',
@@ -3480,7 +3481,6 @@
                    'bzrlib.tests.test_osutils',
                    'bzrlib.tests.test_osutils_encodings',
                    'bzrlib.tests.test_pack',
-                   'bzrlib.tests.test_pack_repository',
                    'bzrlib.tests.test_patch',
                    'bzrlib.tests.test_patches',
                    'bzrlib.tests.test_permissions',

=== modified file 'bzrlib/tests/per_interrepository/__init__.py'
--- a/bzrlib/tests/per_interrepository/__init__.py	2009-07-10 06:46:10 +0000
+++ b/bzrlib/tests/per_interrepository/__init__.py	2009-08-11 21:02:46 +0000
@@ -68,7 +68,12 @@
 
 def default_test_list():
     """Generate the default list of interrepo permutations to test."""
-    from bzrlib.repofmt import knitrepo, pack_repo, weaverepo
+    from bzrlib.repofmt import (
+        groupcompress_repo,
+        knitrepo,
+        pack_repo,
+        weaverepo,
+        )
     result = []
     # test the default InterRepository between format 6 and the current
     # default format.
@@ -111,6 +116,9 @@
     result.append((InterDifferingSerializer,
                    pack_repo.RepositoryFormatKnitPack1(),
                    pack_repo.RepositoryFormatKnitPack6RichRoot()))
+    result.append((InterDifferingSerializer,
+                   pack_repo.RepositoryFormatKnitPack6RichRoot(),
+                   groupcompress_repo.RepositoryFormat2a()))
     return result
 
 

=== modified file 'bzrlib/tests/per_interrepository/test_fetch.py'
--- a/bzrlib/tests/per_interrepository/test_fetch.py	2009-07-10 06:46:10 +0000
+++ b/bzrlib/tests/per_interrepository/test_fetch.py	2009-08-12 02:21:06 +0000
@@ -132,17 +132,23 @@
         altered by all revisions it contains, which means that it needs both
         the inventory for any revision it has, and the inventories of all that
         revision's parents.
+
+        However, we should also skip any revisions which are ghosts in the
+        parents.
         """
-        to_repo = self.make_to_repository('to')
-        if not to_repo._format.supports_external_lookups:
+        if not self.repository_format_to.supports_external_lookups:
             raise TestNotApplicable("Need stacking support in the target.")
         builder = self.make_branch_builder('branch')
         builder.start_series()
         builder.build_snapshot('base', None, [
-            ('add', ('', 'root-id', 'directory', ''))])
-        builder.build_snapshot('left', ['base'], [])
-        builder.build_snapshot('right', ['base'], [])
-        builder.build_snapshot('merge', ['left', 'right'], [])
+            ('add', ('', 'root-id', 'directory', '')),
+            ('add', ('file', 'file-id', 'file', 'content\n'))])
+        builder.build_snapshot('left', ['base'], [
+            ('modify', ('file-id', 'left content\n'))])
+        builder.build_snapshot('right', ['base'], [
+            ('modify', ('file-id', 'right content\n'))])
+        builder.build_snapshot('merge', ['left', 'right'], [
+            ('modify', ('file-id', 'left and right content\n'))])
         builder.finish_series()
         branch = builder.get_branch()
         repo = self.make_to_repository('trunk')
@@ -161,6 +167,57 @@
         self.assertEqual(
             set([('left',), ('right',), ('merge',)]),
             unstacked_repo.inventories.keys())
+        # And the basis inventories have been copied correctly
+        trunk.lock_read()
+        self.addCleanup(trunk.unlock)
+        left_tree, right_tree = trunk.repository.revision_trees(
+            ['left', 'right'])
+        stacked_branch.lock_read()
+        self.addCleanup(stacked_branch.unlock)
+        (stacked_left_tree,
+         stacked_right_tree) = stacked_branch.repository.revision_trees(
+            ['left', 'right'])
+        self.assertEqual(left_tree.inventory, stacked_left_tree.inventory)
+        self.assertEqual(right_tree.inventory, stacked_right_tree.inventory)
+
+    def test_fetch_across_stacking_boundary_ignores_ghost(self):
+        if not self.repository_format_to.supports_external_lookups:
+            raise TestNotApplicable("Need stacking support in the target.")
+        to_repo = self.make_to_repository('to')
+        builder = self.make_branch_builder('branch')
+        builder.start_series()
+        builder.build_snapshot('base', None, [
+            ('add', ('', 'root-id', 'directory', '')),
+            ('add', ('file', 'file-id', 'file', 'content\n'))])
+        builder.build_snapshot('second', ['base'], [
+            ('modify', ('file-id', 'second content\n'))])
+        builder.build_snapshot('third', ['second', 'ghost'], [
+            ('modify', ('file-id', 'third content\n'))])
+        builder.finish_series()
+        branch = builder.get_branch()
+        repo = self.make_to_repository('trunk')
+        trunk = repo.bzrdir.create_branch()
+        trunk.repository.fetch(branch.repository, 'second')
+        repo = self.make_to_repository('stacked')
+        stacked_branch = repo.bzrdir.create_branch()
+        stacked_branch.set_stacked_on_url(trunk.base)
+        stacked_branch.repository.fetch(branch.repository, 'third')
+        unstacked_repo = stacked_branch.bzrdir.open_repository()
+        unstacked_repo.lock_read()
+        self.addCleanup(unstacked_repo.unlock)
+        self.assertFalse(unstacked_repo.has_revision('second'))
+        self.assertFalse(unstacked_repo.has_revision('ghost'))
+        self.assertEqual(
+            set([('second',), ('third',)]),
+            unstacked_repo.inventories.keys())
+        # And the basis inventories have been copied correctly
+        trunk.lock_read()
+        self.addCleanup(trunk.unlock)
+        second_tree = trunk.repository.revision_tree('second')
+        stacked_branch.lock_read()
+        self.addCleanup(stacked_branch.unlock)
+        stacked_second_tree = stacked_branch.repository.revision_tree('second')
+        self.assertEqual(second_tree.inventory, stacked_second_tree.inventory)
 
     def test_fetch_missing_basis_text(self):
         """If fetching a delta, we should die if a basis is not present."""
@@ -276,8 +333,12 @@
         to_repo = self.make_to_repository('to')
         to_repo.fetch(from_tree.branch.repository)
         recorded_inv_sha1 = to_repo.get_inventory_sha1('foo-id')
-        xml = to_repo.get_inventory_xml('foo-id')
-        computed_inv_sha1 = osutils.sha_string(xml)
+        to_repo.lock_read()
+        self.addCleanup(to_repo.unlock)
+        stream = to_repo.inventories.get_record_stream([('foo-id',)],
+                                                       'unordered', True)
+        bytes = stream.next().get_bytes_as('fulltext')
+        computed_inv_sha1 = osutils.sha_string(bytes)
         self.assertEqual(computed_inv_sha1, recorded_inv_sha1)
 
 

=== renamed file 'bzrlib/tests/test_pack_repository.py' => 'bzrlib/tests/per_pack_repository.py'
--- a/bzrlib/tests/test_pack_repository.py	2009-08-11 05:26:57 +0000
+++ b/bzrlib/tests/per_pack_repository.py	2009-08-12 22:28:28 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2008 Canonical Ltd
+# Copyright (C) 2008, 2009 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
@@ -42,7 +42,7 @@
     pack_repo,
     groupcompress_repo,
     )
-from bzrlib.repofmt.groupcompress_repo import RepositoryFormatCHK1
+from bzrlib.repofmt.groupcompress_repo import RepositoryFormat2a
 from bzrlib.smart import (
     client,
     server,
@@ -84,7 +84,7 @@
         """Packs reuse deltas."""
         format = self.get_format()
         repo = self.make_repository('.', format=format)
-        if isinstance(format.repository_format, RepositoryFormatCHK1):
+        if isinstance(format.repository_format, RepositoryFormat2a):
             # TODO: This is currently a workaround. CHK format repositories
             #       ignore the 'deltas' flag, but during conversions, we can't
             #       do unordered delta fetches. Remove this clause once we
@@ -295,6 +295,41 @@
         self.assertEqual(1, len(list(index.iter_all_entries())))
         self.assertEqual(2, len(tree.branch.repository.all_revision_ids()))
 
+    def test_pack_preserves_all_inventories(self):
+        # This is related to bug:
+        #   https://bugs.launchpad.net/bzr/+bug/412198
+        # Stacked repositories need to keep the inventory for parents, even
+        # after a pack operation. However, it is harder to test that, then just
+        # test that all inventory texts are preserved.
+        format = self.get_format()
+        builder = self.make_branch_builder('source', format=format)
+        builder.start_series()
+        builder.build_snapshot('A-id', None, [
+            ('add', ('', 'root-id', 'directory', None))])
+        builder.build_snapshot('B-id', None, [
+            ('add', ('file', 'file-id', 'file', 'B content\n'))])
+        builder.build_snapshot('C-id', None, [
+            ('modify', ('file-id', 'C content\n'))])
+        builder.finish_series()
+        b = builder.get_branch()
+        b.lock_read()
+        self.addCleanup(b.unlock)
+        repo = self.make_repository('repo', shared=True, format=format)
+        repo.lock_write()
+        self.addCleanup(repo.unlock)
+        repo.fetch(b.repository, revision_id='B-id')
+        inv = b.repository.iter_inventories(['C-id']).next()
+        repo.start_write_group()
+        repo.add_inventory('C-id', inv, ['B-id'])
+        repo.commit_write_group()
+        self.assertEqual([('A-id',), ('B-id',), ('C-id',)],
+                         sorted(repo.inventories.keys()))
+        repo.pack()
+        self.assertEqual([('A-id',), ('B-id',), ('C-id',)],
+                         sorted(repo.inventories.keys()))
+        # Content should be preserved as well
+        self.assertEqual(inv, repo.iter_inventories(['C-id']).next())
+
     def test_pack_layout(self):
         # Test that the ordering of revisions in pack repositories is
         # tip->ancestor
@@ -311,7 +346,7 @@
         # revision access tends to be tip->ancestor, so ordering that way on
         # disk is a good idea.
         for _1, key, val, refs in pack.revision_index.iter_all_entries():
-            if type(format.repository_format) is RepositoryFormatCHK1:
+            if type(format.repository_format) is RepositoryFormat2a:
                 # group_start, group_len, internal_start, internal_len
                 pos = map(int, val.split())
             else:
@@ -589,7 +624,7 @@
 
     def make_write_ready_repo(self):
         format = self.get_format()
-        if isinstance(format.repository_format, RepositoryFormatCHK1):
+        if isinstance(format.repository_format, RepositoryFormat2a):
             raise TestNotApplicable("No missing compression parents")
         repo = self.make_repository('.', format=format)
         repo.lock_write()
@@ -808,7 +843,7 @@
                 matching_format_name = 'pack-0.92-subtree'
             else:
                 if repo._format.supports_chks:
-                    matching_format_name = 'development6-rich-root'
+                    matching_format_name = '2a'
                 else:
                     matching_format_name = 'rich-root-pack'
             mismatching_format_name = 'pack-0.92'
@@ -841,7 +876,7 @@
         else:
             if repo.supports_rich_root():
                 if repo._format.supports_chks:
-                    matching_format_name = 'development6-rich-root'
+                    matching_format_name = '2a'
                 else:
                     matching_format_name = 'rich-root-pack'
                 mismatching_format_name = 'pack-0.92-subtree'
@@ -1062,9 +1097,9 @@
                   "(bzr 1.9)\n",
               format_supports_external_lookups=True,
               index_class=BTreeGraphIndex),
-         dict(format_name='development6-rich-root',
-              format_string='Bazaar development format - group compression '
-                  'and chk inventory (needs bzr.dev from 1.14)\n',
+         dict(format_name='2a',
+              format_string="Bazaar repository format 2a "
+                "(needs bzr 1.16 or later)\n",
               format_supports_external_lookups=True,
               index_class=BTreeGraphIndex),
          ]




More information about the bazaar-commits mailing list