use inv deltas by default for all formats now: --classic to get old algorithm for packs

4 files changed, 48 insertions, 31 deletions

diff --git a/__init__.py b/__init__.py
index ef50c28..14716e2 100644
--- a/__init__.py
+++ b/__init__.py
@@ -48,7 +48,7 @@ page on Launchpad, https://launchpad.net/bzr-fastimport.
 version_info = (0, 8, 0, 'dev', 0)
 
 from bzrlib.commands import Command, register_command
-from bzrlib.option import Option, ListOption
+from bzrlib.option import Option, ListOption, RegistryOption
 
 
 def test_suite():
@@ -176,8 +176,13 @@ class cmd_fast_import(Command):
                     Option('inv-cache', type=int,
                         help="Number of inventories to cache.",
                         ),
-                    Option('experimental',
-                        help="Enable experimental features.",
+                    RegistryOption.from_kwargs('mode',
+                        'The import algorithm to use.',
+                        title='Import Algorithm',
+                        default='Use the preferred algorithm (inventory deltas).',
+                        classic="Use the original algorithm (mutable inventories).",
+                        experimental="Enable experimental features.",
+                        value_switches=True, enum_switch=False,
                         ),
                     Option('import-marks', type=str,
                         help="Import marks from file."
@@ -189,10 +194,12 @@ class cmd_fast_import(Command):
     aliases = []
     def run(self, source, verbose=False, info=None, trees=False,
         count=-1, checkpoint=10000, autopack=4, inv_cache=-1,
-        experimental=False, import_marks=None, export_marks=None):
+        mode=None, import_marks=None, export_marks=None):
         from bzrlib import bzrdir
         from bzrlib.errors import BzrCommandError, NotBranchError
         from bzrlib.plugins.fastimport.processors import generic_processor
+        if mode is None:
+            mode = 'default'
         try:
             control, relpath = bzrdir.BzrDir.open_containing('.')
         except NotBranchError:
@@ -206,7 +213,7 @@ class cmd_fast_import(Command):
             'checkpoint': checkpoint,
             'autopack': autopack,
             'inv-cache': inv_cache,
-            'experimental': experimental,
+            'mode': mode,
             'import-marks': import_marks,
             'export-marks': export_marks,
             }
diff --git a/bzr_commit_handler.py b/bzr_commit_handler.py
index eade296..f26891f 100644
--- a/bzr_commit_handler.py
+++ b/bzr_commit_handler.py
@@ -52,8 +52,8 @@ class GenericCommitHandler(processor.CommitHandler):
         self.revision_id = self.gen_revision_id()
         # cache of texts for this commit, indexed by file-id
         self.lines_for_commit = {}
-        if self.rev_store.expects_rich_root():
-            self.lines_for_commit[inventory.ROOT_ID] = []
+        #if self.rev_store.expects_rich_root():
+        self.lines_for_commit[inventory.ROOT_ID] = []
 
         # Track the heads and get the real parent list
         parents = self.cache_mgr.track_heads(self.command)
@@ -503,11 +503,11 @@ class InventoryCommitHandler(GenericCommitHandler):
         self._delete_all_items(self.inventory)
 
 
-class CHKInventoryCommitHandler(GenericCommitHandler):
-    """A CommitHandler that builds and saves CHKInventory objects."""
+class InventoryDeltaCommitHandler(GenericCommitHandler):
+    """A CommitHandler that builds Inventories by applying a delta."""
 
     def pre_process_files(self):
-        super(CHKInventoryCommitHandler, self).pre_process_files()
+        super(InventoryDeltaCommitHandler, self).pre_process_files()
         # A given file-id can only appear once so we accumulate
         # the entries in a dict then build the actual delta at the end
         self._delta_entries_by_fileid = {}
@@ -519,10 +519,6 @@ class CHKInventoryCommitHandler(GenericCommitHandler):
             # Need to explicitly add the root entry for the first revision
             # and for non rich-root inventories
             root_id = inventory.ROOT_ID
-            # XXX: We *could* make this a CHKInventoryDirectory but it
-            # seems that deltas ought to use normal InventoryDirectory's
-            # because they simply don't know the chk_inventory that they
-            # are about to become a part of.
             root_ie = inventory.InventoryDirectory(root_id, u'', None)
             root_ie.revision = self.revision_id
             self._add_entry((old_path, '', root_id, root_ie))
@@ -531,8 +527,8 @@ class CHKInventoryCommitHandler(GenericCommitHandler):
         """Save the revision."""
         delta = list(self._delta_entries_by_fileid.values())
         #print "delta:\n%s\n\n" % "\n".join([str(de) for de in delta])
-        inv = self.rev_store.chk_load(self.revision, self.basis_inventory,
-            delta, None,
+        inv = self.rev_store.load_using_delta(self.revision,
+            self.basis_inventory, delta, None,
             lambda file_id: self._get_lines(file_id),
             lambda file_id: self._get_per_file_parents(file_id),
             lambda revision_ids: self._get_inventories(revision_ids))
diff --git a/processors/generic_processor.py b/processors/generic_processor.py
index c91e067..e8bb1a0 100644
--- a/processors/generic_processor.py
+++ b/processors/generic_processor.py
@@ -97,8 +97,7 @@ class GenericProcessor(processor.ImportProcessor):
     * inv-cache - number of inventories to cache.
       If not set, the default is 100 for CHK formats and 10 otherwise.
 
-    * experimental - enable experimental mode, i.e. use features
-      not yet fully tested.
+    * mode - import algorithm to use: default, experimental or classic.
 
     * import-marks - name of file to read to load mark information from
 
@@ -112,7 +111,7 @@ class GenericProcessor(processor.ImportProcessor):
         'checkpoint',
         'autopack',
         'inv-cache',
-        'experimental',
+        'mode',
         'import-marks',
         'export-marks',
         ]
@@ -169,7 +168,8 @@ class GenericProcessor(processor.ImportProcessor):
         self.repo.start_write_group()
 
     def _load_info_and_params(self):
-        self._experimental = bool(self.params.get('experimental', False))
+        self._mode = bool(self.params.get('mode', 'default'))
+        self._experimental = self._mode == 'experimental'
 
         # This is currently hard-coded but might be configurable via
         # parameters one day if that's needed
@@ -185,12 +185,16 @@ class GenericProcessor(processor.ImportProcessor):
 
         # Decide which CommitHandler to use
         self.supports_chk = getattr(self.repo._format, 'supports_chks', False)
-        if self.supports_chk:
+        if self.supports_chk and self._mode == 'classic':
+            note("Cannot use classic algorithm on CHK repositories"
+                 " - using default one instead")
+            self._mode = 'default'
+        if self._mode == 'classic':
             self.commit_handler_factory = \
-                bzr_commit_handler.CHKInventoryCommitHandler
+                bzr_commit_handler.InventoryCommitHandler
         else:
             self.commit_handler_factory = \
-                bzr_commit_handler.InventoryCommitHandler
+                bzr_commit_handler.InventoryDeltaCommitHandler
 
         # Decide how often to automatically report progress
         # (not a parameter yet)
diff --git a/revision_store.py b/revision_store.py
index 02ed289..d2ab2d3 100644
--- a/revision_store.py
+++ b/revision_store.py
@@ -205,12 +205,12 @@ class AbstractRevisionStore(object):
             self.repo.add_signature_text(rev.revision_id, signature)
         self._add_revision(rev, inv)
 
-    def chk_load(self, rev, basis_inv, inv_delta, signature,
+    def load_using_delta(self, rev, basis_inv, inv_delta, signature,
         text_provider, parents_provider, inventories_provider=None):
-        """Load a revision for a CHKInventory.
+        """Load a revision by applying a delta to a (CHK)Inventory.
 
         :param rev: the Revision
-        :param basis_inv: the basis CHKInventory
+        :param basis_inv: the basis Inventory or CHKInventory
         :param inv_delta: the inventory delta
         :param signature: signing information
         :param text_provider: a callable expecting a file_id parameter
@@ -292,7 +292,7 @@ class AbstractRevisionStore(object):
         parents, parent_invs):
         """Add the inventory to the repository as revision_id.
         
-        :param basis_inv: the basis CHKInventory
+        :param basis_inv: the basis Inventory or CHKInventory
         :param inv_delta: the inventory delta
         :param parents: The revision ids of the parents that revision_id
                         is known to have and are in the repository already.
@@ -304,11 +304,21 @@ class AbstractRevisionStore(object):
           inv is the generated inventory
         """
         if len(parents):
-            validator, new_inv = self.repo.add_inventory_by_delta(parents[0],
-                inv_delta, revision_id, parents, basis_inv=basis_inv,
-                propagate_caches=False)
+            if self._supports_chks:
+                validator, new_inv = self.repo.add_inventory_by_delta(parents[0],
+                    inv_delta, revision_id, parents, basis_inv=basis_inv,
+                    propagate_caches=False)
+            else:
+                validator, new_inv = self.repo.add_inventory_by_delta(parents[0],
+                    inv_delta, revision_id, parents)
         else:
-            new_inv = basis_inv.create_by_apply_delta(inv_delta, revision_id)
+            if hasattr(basis_inv, 'create_by_apply_delta'):
+                new_inv = basis_inv.create_by_apply_delta(inv_delta, revision_id)
+            else:
+                new_inv = inventory.Inventory(revision_id=revision_id)
+                # This is set in the delta so remove it to prevent a duplicate
+                del new_inv[inventory.ROOT_ID]
+                new_inv.apply_delta(inv_delta)
             validator = self.repo.add_inventory(revision_id, new_inv, parents)
         return validator, new_inv