Merge lp:~spiv/bzr/inter1and2helper-636930-2.2 into lp:bzr/2.2

[Andrew Bennetts]

Fix fetching more than 100 revisions from non-rich-root to rich-root repositories.

This fixes a regression in 2.2 caused by very shallow bug. I've also added another per_interrepo scenario so that this case gets some test coverage so it won't break again. The new scenario isn't the most elegant thing ever, but actually the cost in terms of code complexity and in terms of test running time is pretty low, so I think it's worth it.

The really elegant fix is to get rid of InterDifferingSerializer in favour of using streaming fetch all the time ;)

[Martin Pool (mbp)]

nice, thanks.

you might want to check if the other reports gz mentioned in that bug are in fact dupes.

review: Approve

[Andrew Bennetts (spiv)]

sent to pqm by email

[Andrew Bennetts (spiv)]

sent to pqm by email

=== modified file 'NEWS'
--- NEWS	2010-09-13 02:32:44 +0000
+++ NEWS	2010-09-14 07:16:05 +0000
@@ -46,6 +46,12 @@
   later which can mangle bytestrings printed to the console.
   (Martin [gz], #631350)
 
+* Upgrading or fetching from a non-rich-root repository to a rich-root
+  repository (e.g. from pack-0.92 to 2a) no longer fails with
+  ``'Inter1and2Helper' object has no attribute 'source_repo'``.  This was
+  a regression from Bazaar 2.1.  (Andrew Bennetts, #636930)
+  
+
 Improvements
 ************
 

=== modified file 'bzrlib/fetch.py'
--- bzrlib/fetch.py	2010-05-13 10:08:32 +0000
+++ bzrlib/fetch.py	2010-09-14 07:16:05 +0000
@@ -182,6 +182,9 @@
     This is for use by fetchers and converters.
     """
 
+    # This is a class variable so that the test suite can override it.
+    known_graph_threshold = 100
+
     def __init__(self, source):
         """Constructor.
 
@@ -243,10 +246,8 @@
         # yet, and are unlikely to in non-rich-root environments anyway.
         root_id_order.sort(key=operator.itemgetter(0))
         # Create a record stream containing the roots to create.
-        if len(revs) > 100:
-            # XXX: not covered by tests, should have a flag to always run
-            # this. -- mbp 20100129
-            graph = self.source_repo.get_known_graph_ancestry(revs)
+        if len(revs) > self.known_graph_threshold:
+            graph = self.source.get_known_graph_ancestry(revs)
         new_roots_stream = _new_root_data_stream(
             root_id_order, rev_id_to_root_id, parent_map, self.source, graph)
         return [('texts', new_roots_stream)]

=== modified file 'bzrlib/tests/per_interrepository/__init__.py'
--- bzrlib/tests/per_interrepository/__init__.py	2010-06-20 11:18:38 +0000
+++ bzrlib/tests/per_interrepository/__init__.py	2010-09-14 07:16:05 +0000
@@ -49,7 +49,7 @@
         (label, repository_format, repository_format_to).
     """
     result = []
-    for label, repository_format, repository_format_to in formats:
+    for label, repository_format, repository_format_to, extra_setup in formats:
         id = '%s,%s,%s' % (label, repository_format.__class__.__name__,
                            repository_format_to.__class__.__name__)
         scenario = (id,
@@ -57,6 +57,7 @@
              "transport_readonly_server": transport_readonly_server,
              "repository_format": repository_format,
              "repository_format_to": repository_format_to,
+             "extra_setup": extra_setup,
              })
         result.append(scenario)
     return result
@@ -71,8 +72,8 @@
         weaverepo,
         )
     result = []
-    def add_combo(label, from_format, to_format):
-        result.append((label, from_format, to_format))
+    def add_combo(label, from_format, to_format, extra_setup=None):
+        result.append((label, from_format, to_format, extra_setup))
     # test the default InterRepository between format 6 and the current
     # default format.
     # XXX: robertc 20060220 reinstate this when there are two supported
@@ -89,6 +90,9 @@
     # XXX: although we attach InterRepository class names to these scenarios,
     # there's nothing asserting that these labels correspond to what is
     # actually used.
+    def force_known_graph(testcase):
+        from bzrlib.fetch import Inter1and2Helper
+        testcase.overrideAttr(Inter1and2Helper, 'known_graph_threshold', -1)
     add_combo('InterRepository',
               weaverepo.RepositoryFormat5(),
               knitrepo.RepositoryFormatKnit3())
@@ -113,6 +117,11 @@
     add_combo('InterDifferingSerializer',
               pack_repo.RepositoryFormatKnitPack1(),
               pack_repo.RepositoryFormatKnitPack6RichRoot())
+    add_combo('InterDifferingSerializer+get_known_graph_ancestry',
+              pack_repo.RepositoryFormatKnitPack1(),
+              pack_repo.RepositoryFormatKnitPack6RichRoot(),
+              force_known_graph,
+              )
     add_combo('InterDifferingSerializer',
               pack_repo.RepositoryFormatKnitPack6RichRoot(),
               groupcompress_repo.RepositoryFormat2a())
@@ -132,6 +141,8 @@
 
     def setUp(self):
         super(TestCaseWithInterRepository, self).setUp()
+        if self.extra_setup:
+            self.extra_setup(self)
 
     def make_branch(self, relpath, format=None):
         repo = self.make_repository(relpath, format=format)

=== modified file 'bzrlib/tests/test_selftest.py'
--- bzrlib/tests/test_selftest.py	2010-06-20 11:18:38 +0000
+++ bzrlib/tests/test_selftest.py	2010-09-14 07:16:05 +0000
@@ -312,19 +312,21 @@
         from bzrlib.tests.per_interrepository import make_scenarios
         server1 = "a"
         server2 = "b"
-        formats = [("C0", "C1", "C2"), ("D0", "D1", "D2")]
+        formats = [("C0", "C1", "C2", "C3"), ("D0", "D1", "D2", "D3")]
         scenarios = make_scenarios(server1, server2, formats)
         self.assertEqual([
             ('C0,str,str',
              {'repository_format': 'C1',
               'repository_format_to': 'C2',
               'transport_readonly_server': 'b',
-              'transport_server': 'a'}),
+              'transport_server': 'a',
+              'extra_setup': 'C3'}),
             ('D0,str,str',
              {'repository_format': 'D1',
               'repository_format_to': 'D2',
               'transport_readonly_server': 'b',
-              'transport_server': 'a'})],
+              'transport_server': 'a',
+              'extra_setup': 'D3'})],
             scenarios)