Merge lp:~jelmer/bzr/add-revision-doesnt-sign into lp:bzr

[Jelmer Vernooij]

Remove config argument from VersionedFileRepository.add_revision()

Some refactoring of VersionedFileRepository.add_revision:

 * Move revision signing to the CommitBuilder. Revision signing was only done
   if the 'config' argument was specified, and that was only done by the
   commit builder anyway.

 * The commit builder now uses the private _add_revision method which skips
   some checks on the revision object that the commit builder
   already guarantees

These changes are prerequisites for my next branch, which should make reduce
the number of roundtrips for HPSS commits to < 30.

[Martin Packman (gz)]

Looks sensible. If the current tests don't mind where the signing happens that's a good sign. No pun intended.

review: Approve

[Jelmer Vernooij (jelmer)]

sent to pqm by email

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2012-01-03 17:08:29 +0000
+++ bzrlib/remote.py	2012-01-04 16:12:24 +0000
@@ -1844,17 +1844,8 @@
             delta, new_revision_id, parents, basis_inv=basis_inv,
             propagate_caches=propagate_caches)
 
-    def add_revision(self, revision_id, rev, inv=None, config=None):
+    def add_revision(self, revision_id, rev, inv=None):
         _mod_revision.check_not_reserved_id(revision_id)
-        if (config is not None and
-            config.get('create_signatures') == _mod_config.SIGN_ALWAYS):
-            if inv is None:
-                inv = self.get_inventory(revision_id)
-            tree = InventoryRevisionTree(self, inv, revision_id)
-            testament = _mod_testament.Testament(rev, tree)
-            plaintext = testament.as_short_text()
-            self.store_revision_signature(
-                gpg.GPGStrategy(config), plaintext, revision_id)
         key = (revision_id,)
         # check inventory present
         if not self.inventories.get_parent_map([key]):
@@ -1867,10 +1858,13 @@
                                                         rev.parent_ids)
         else:
             rev.inventory_sha1 = self.inventories.get_sha1s([key])[key]
+        self._add_revision(rev)
+
+    def _add_revision(self, rev):
         if self._real_repository is not None:
-            return self._real_repository.add_revision(
-                revision_id, rev, inv, config)
+            return self._real_repository._add_revision(rev)
         text = self._serializer.write_revision_to_string(rev)
+        key = (rev.revision_id,)
         parents = tuple((parent,) for parent in rev.parent_ids)
         self._write_group_tokens, missing_keys = self._get_sink().insert_stream(
             [('revisions', [FulltextContentFactory(key, parents, None, text)])],

=== modified file 'bzrlib/vf_repository.py'
--- bzrlib/vf_repository.py	2011-12-19 13:23:58 +0000
+++ bzrlib/vf_repository.py	2012-01-04 16:12:24 +0000
@@ -200,8 +200,13 @@
                        revision_id=self._new_revision_id,
                        properties=self._revprops)
         rev.parent_ids = self.parents
-        self.repository.add_revision(self._new_revision_id, rev,
-            self.new_inventory, self._config_stack)
+        if self._config_stack.get('create_signatures') == _mod_config.SIGN_ALWAYS:
+            testament = Testament(rev, self.revision_tree())
+            plaintext = testament.as_short_text()
+            self.repository.store_revision_signature(
+                gpg.GPGStrategy(self._config_stack), plaintext,
+                self._new_revision_id)
+        self.repository._add_revision(rev)
         self._ensure_fallback_inventories()
         self.repository.commit_write_group()
         return self._new_revision_id
@@ -1033,29 +1038,17 @@
         self.inventories._access.flush()
         return result
 
-    def add_revision(self, revision_id, rev, inv=None, config=None):
+    def add_revision(self, revision_id, rev, inv=None):
         """Add rev to the revision store as revision_id.
 
         :param revision_id: the revision id to use.
         :param rev: The revision object.
         :param inv: The inventory for the revision. if None, it will be looked
                     up in the inventory storer
-        :param config: If None no digital signature will be created.
-                       If supplied its signature_needed method will be used
-                       to determine if a signature should be made.
         """
         # TODO: jam 20070210 Shouldn't we check rev.revision_id and
         #       rev.parent_ids?
         _mod_revision.check_not_reserved_id(revision_id)
-        if (config is not None and
-            config.get('create_signatures') == _mod_config.SIGN_ALWAYS):
-            if inv is None:
-                inv = self.get_inventory(revision_id)
-            tree = InventoryRevisionTree(self, inv, revision_id)
-            testament = Testament(rev, tree)
-            plaintext = testament.as_short_text()
-            self.store_revision_signature(
-                gpg.GPGStrategy(config), plaintext, revision_id)
         # check inventory present
         if not self.inventories.get_parent_map([(revision_id,)]):
             if inv is None:

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2012-01-03 13:21:09 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2012-01-04 16:12:24 +0000
@@ -125,6 +125,9 @@
   which can be None or a Branch object for the submit branch location.
   (Jelmer Vernooij)
 
+* ``VersionedFileRepository.add_revision`` no longer takes a ``config``
+  argument. (Jelmer Vernooij)
+
 Internals
 *********