Rev 3712: Fix bug #242510, when determining the autopack sequence, in http://bzr.arbash-meinel.com/branches/bzr/1.8-dev/autopack_bug_242510

John Arbash Meinel john at arbash-meinel.com
Fri Sep 19 16:03:57 BST 2008


At http://bzr.arbash-meinel.com/branches/bzr/1.8-dev/autopack_bug_242510

------------------------------------------------------------
revno: 3712
revision-id: john at arbash-meinel.com-20080919150355-14s3zy2jndtlmc2q
parent: pqm at pqm.ubuntu.com-20080917230446-p0wippqwikt511sp
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: autopack_bug_242510
timestamp: Fri 2008-09-19 10:03:55 -0500
message:
  Fix bug #242510, when determining the autopack sequence,
  if the last entry is in a pack by itself, don't repack it.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2008-09-17 22:22:25 +0000
+++ b/NEWS	2008-09-19 15:03:55 +0000
@@ -32,6 +32,10 @@
     * Make the first line of the manpage preamble a comment again.
       (David Futcher, #242106)
 
+    * The autopacking logic will now avoid creating a new pack with
+      identical contents to an existing pack.
+      (John Arbash Meinel, #242510)
+
   DOCUMENTATION:
 
   API CHANGES:

=== modified file 'bzrlib/repofmt/pack_repo.py'
--- a/bzrlib/repofmt/pack_repo.py	2008-09-02 03:17:08 +0000
+++ b/bzrlib/repofmt/pack_repo.py	2008-09-19 15:03:55 +0000
@@ -1276,8 +1276,8 @@
         # plan out what packs to keep, and what to reorganise
         while len(existing_packs):
             # take the largest pack, and if its less than the head of the
-            # distribution chart we will include its contents in the new pack for
-            # that position. If its larger, we remove its size from the
+            # distribution chart we will include its contents in the new pack
+            # for that position. If its larger, we remove its size from the
             # distribution chart
             next_pack_rev_count, next_pack = existing_packs.pop(0)
             if next_pack_rev_count >= pack_distribution[0]:
@@ -1300,7 +1300,12 @@
                     # this pack is used up, shift left.
                     del pack_distribution[0]
                     pack_operations.append([0, []])
-        
+
+        if len(pack_operations) > 1:
+            # Check the last pack, it is possible that it has only a single
+            # entry, which would cause us to repack the node into itself
+            if len(pack_operations[-1][1]) == 1:
+                pack_operations.pop()
         return pack_operations
 
     def ensure_loaded(self):

=== modified file 'bzrlib/tests/test_repository.py'
--- a/bzrlib/tests/test_repository.py	2008-09-04 21:10:07 +0000
+++ b/bzrlib/tests/test_repository.py	2008-09-19 15:03:55 +0000
@@ -744,13 +744,16 @@
     def get_format(self):
         return bzrdir.format_registry.make_bzrdir('pack-0.92')
 
+    def get_packs(self):
+        format = self.get_format()
+        repo = self.make_repository('.', format=format)
+        return repo._pack_collection
+
     def test__max_pack_count(self):
         """The maximum pack count is a function of the number of revisions."""
-        format = self.get_format()
-        repo = self.make_repository('.', format=format)
-        packs = repo._pack_collection
         # no revisions - one pack, so that we can have a revision free repo
         # without it blowing up
+        packs = self.get_packs()
         self.assertEqual(1, packs._max_pack_count(0))
         # after that the sum of the digits, - check the first 1-9
         self.assertEqual(1, packs._max_pack_count(1))
@@ -772,21 +775,16 @@
         self.assertEqual(25, packs._max_pack_count(112894))
 
     def test_pack_distribution_zero(self):
-        format = self.get_format()
-        repo = self.make_repository('.', format=format)
-        packs = repo._pack_collection
+        packs = self.get_packs()
         self.assertEqual([0], packs.pack_distribution(0))
 
     def test_ensure_loaded_unlocked(self):
-        format = self.get_format()
-        repo = self.make_repository('.', format=format)
+        packs = self.get_packs()
         self.assertRaises(errors.ObjectNotLocked,
-                          repo._pack_collection.ensure_loaded)
+                          packs.ensure_loaded)
 
     def test_pack_distribution_one_to_nine(self):
-        format = self.get_format()
-        repo = self.make_repository('.', format=format)
-        packs = repo._pack_collection
+        packs = self.get_packs()
         self.assertEqual([1],
             packs.pack_distribution(1))
         self.assertEqual([1, 1],
@@ -808,9 +806,7 @@
 
     def test_pack_distribution_stable_at_boundaries(self):
         """When there are multi-rev packs the counts are stable."""
-        format = self.get_format()
-        repo = self.make_repository('.', format=format)
-        packs = repo._pack_collection
+        packs = self.get_packs()
         # in 10s:
         self.assertEqual([10], packs.pack_distribution(10))
         self.assertEqual([10, 1], packs.pack_distribution(11))
@@ -825,9 +821,7 @@
         self.assertEqual([100, 100, 10, 1], packs.pack_distribution(211))
 
     def test_plan_pack_operations_2009_revisions_skip_all_packs(self):
-        format = self.get_format()
-        repo = self.make_repository('.', format=format)
-        packs = repo._pack_collection
+        packs = self.get_packs()
         existing_packs = [(2000, "big"), (9, "medium")]
         # rev count - 2009 -> 2x1000 + 9x1
         pack_operations = packs.plan_autopack_combinations(
@@ -835,9 +829,7 @@
         self.assertEqual([], pack_operations)
 
     def test_plan_pack_operations_2010_revisions_skip_all_packs(self):
-        format = self.get_format()
-        repo = self.make_repository('.', format=format)
-        packs = repo._pack_collection
+        packs = self.get_packs()
         existing_packs = [(2000, "big"), (9, "medium"), (1, "single")]
         # rev count - 2010 -> 2x1000 + 1x10
         pack_operations = packs.plan_autopack_combinations(
@@ -845,9 +837,7 @@
         self.assertEqual([], pack_operations)
 
     def test_plan_pack_operations_2010_combines_smallest_two(self):
-        format = self.get_format()
-        repo = self.make_repository('.', format=format)
-        packs = repo._pack_collection
+        packs = self.get_packs()
         existing_packs = [(1999, "big"), (9, "medium"), (1, "single2"),
             (1, "single1")]
         # rev count - 2010 -> 2x1000 + 1x10 (3)
@@ -855,6 +845,18 @@
             existing_packs, [1000, 1000, 10])
         self.assertEqual([[2, ["single2", "single1"]], [0, []]], pack_operations)
 
+    def test_plan_pack_operations_110_doesnt_repack_single_entry(self):
+        packs = self.get_packs()
+        existing_packs = [(55, "big"), (25, "medium"), (21, "medium2"),
+                          (9, "small")]
+        pack_operations = packs.plan_autopack_combinations(existing_packs,
+                                                           [100, 10])
+        # This initially wants to create a 100 size pack, and a 10 size pack.
+        # However, there is only a single pack with 1 entry to put into the
+        # final pack.
+        self.assertEqual([[101, ['big', 'medium', 'medium2']]],
+                         pack_operations)
+
     def test_all_packs_none(self):
         format = self.get_format()
         tree = self.make_branch_and_tree('.', format=format)



More information about the bazaar-commits mailing list