Rev 3577: Clean up the build_snapshot api a bit. in http://bzr.arbash-meinel.com/branches/bzr/1.7-dev/branch_builder

John Arbash Meinel john at arbash-meinel.com
Tue Jul 22 19:59:01 BST 2008


At http://bzr.arbash-meinel.com/branches/bzr/1.7-dev/branch_builder

------------------------------------------------------------
revno: 3577
revision-id: john at arbash-meinel.com-20080722185757-98rezzd05dgcfr3e
parent: john at arbash-meinel.com-20080722184833-b06pe6ihw4imz7rc
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: branch_builder
timestamp: Tue 2008-07-22 13:57:57 -0500
message:
  Clean up the build_snapshot api a bit.
  
  Move the parent_ids to the second parameter, because it makes more sense there.
  Document the function parameters and return value.
  Factor out moving the branch tip, to help clean up the function.
-------------- next part --------------
=== modified file 'bzrlib/branchbuilder.py'
--- a/bzrlib/branchbuilder.py	2008-07-22 18:48:33 +0000
+++ b/bzrlib/branchbuilder.py	2008-07-22 18:57:57 +0000
@@ -60,19 +60,39 @@
         finally:
             tree.unlock()
 
-    def build_snapshot(self, parent_ids, revision_id, actions):
+    def _move_branch_pointer(self, new_revision_id):
+        """Point self._branch to a different revision id."""
+        self._branch.lock_write()
+        try:
+            # We don't seem to have a simple set_last_revision(), so we
+            # implement it here.
+            cur_revno, cur_revision_id = self._branch.last_revision_info()
+            g = self._branch.repository.get_graph()
+            new_revno = g.find_distance_to_null(new_revision_id,
+                                                [(cur_revision_id, cur_revno)])
+            self._branch.set_last_revision_info(new_revno, new_revision_id)
+        finally:
+            self._branch.unlock()
+
+    def build_snapshot(self, revision_id, parent_ids, actions):
+        """Build a commit, shaped in a specific way.
+
+        :param revision_id: The handle for the new commit, could be none, as it
+            will be returned, though it is put in the commit message.
+        :param parent_ids: A list of parent_ids to use for the commit.
+            It can be None, which indicates to use the last commit.
+        :param actions: A list of actions to perform. Supported actions are:
+            ('add', ('path', 'file-id', 'kind', 'content' or None))
+            ('modify', ('file-id', 'new-content'))
+            ('unversion', 'file-id')
+            # not supported yet: ('rename', ('orig-path', 'new-path'))
+        ;return: The revision_id of the new commit
+        """
         if parent_ids is not None:
-            self._branch.lock_write()
-            try:
-                base_id = parent_ids[0]
-                # Unfortunately, this is the only real way to get the revno
-                cur_revno, cur_revision_id = self._branch.last_revision_info()
-                g = self._branch.repository.get_graph()
-                new_revno = g.find_distance_to_null(base_id,
-                                [(cur_revision_id, cur_revno)])
-                self._branch.set_last_revision_info(new_revno, base_id)
-            finally:
-                self._branch.unlock()
+            base_id = parent_ids[0]
+            if base_id != self._branch.last_revision():
+                self._move_branch_pointer(base_id)
+
         tree = memorytree.MemoryTree.create_on_branch(self._branch)
         tree.lock_write()
         try:

=== modified file 'bzrlib/tests/test_branchbuilder.py'
--- a/bzrlib/tests/test_branchbuilder.py	2008-07-22 18:47:24 +0000
+++ b/bzrlib/tests/test_branchbuilder.py	2008-07-22 18:57:57 +0000
@@ -88,7 +88,7 @@
 
     def build_a_rev(self):
         builder = BranchBuilder(self.get_transport().clone('foo'))
-        rev_id1 = builder.build_snapshot(None, 'A-id',
+        rev_id1 = builder.build_snapshot('A-id', None,
             [('add', ('', 'a-root-id', 'directory', None)),
              ('add', ('a', 'a-id', 'file', 'contents'))])
         self.assertEqual('A-id', rev_id1)
@@ -107,7 +107,7 @@
 
     def test_add_second_file(self):
         builder = self.build_a_rev()
-        rev_id2 = builder.build_snapshot(None, 'B-id',
+        rev_id2 = builder.build_snapshot('B-id', None,
             [('add', ('b', 'b-id', 'file', 'content_b'))])
         self.assertEqual('B-id', rev_id2)
         branch = builder.get_branch()
@@ -122,7 +122,7 @@
 
     def test_add_empty_dir(self):
         builder = self.build_a_rev()
-        rev_id2 = builder.build_snapshot(None, 'B-id',
+        rev_id2 = builder.build_snapshot('B-id', None,
             [('add', ('b', 'b-id', 'directory', None))])
         rev_tree = builder.get_branch().repository.revision_tree('B-id')
         self.assertTreeShape([(u'', 'a-root-id', 'directory'),
@@ -132,7 +132,7 @@
 
     def test_modify_file(self):
         builder = self.build_a_rev()
-        rev_id2 = builder.build_snapshot(None, 'B-id',
+        rev_id2 = builder.build_snapshot('B-id', None,
             [('modify', ('a-id', 'new\ncontent\n'))])
         self.assertEqual('B-id', rev_id2)
         branch = builder.get_branch()
@@ -143,7 +143,7 @@
 
     def test_delete_file(self):
         builder = self.build_a_rev()
-        rev_id2 = builder.build_snapshot(None, 'B-id',
+        rev_id2 = builder.build_snapshot('B-id', None,
             [('unversion', 'a-id')])
         self.assertEqual('B-id', rev_id2)
         branch = builder.get_branch()
@@ -154,7 +154,7 @@
 
     def test_delete_directory(self):
         builder = self.build_a_rev()
-        rev_id2 = builder.build_snapshot(None, 'B-id',
+        rev_id2 = builder.build_snapshot('B-id', None,
             [('add', ('b', 'b-id', 'directory', None)),
              ('add', ('b/c', 'c-id', 'file', 'foo\n')),
              ('add', ('b/d', 'd-id', 'directory', None)),
@@ -168,7 +168,7 @@
                               (u'b/d', 'd-id', 'directory'),
                               (u'b/d/e', 'e-id', 'file')], rev_tree)
         # Removing a directory removes all child dirs
-        builder.build_snapshot(None, 'C-id', [('unversion', 'b-id')])
+        builder.build_snapshot('C-id', None, [('unversion', 'b-id')])
         rev_tree = builder.get_branch().repository.revision_tree('C-id')
         self.assertTreeShape([(u'', 'a-root-id', 'directory'),
                               (u'a', 'a-id', 'file'),
@@ -177,16 +177,16 @@
     def test_unknown_action(self):
         builder = self.build_a_rev()
         self.assertRaises(errors.UnknownBuildAction,
-            builder.build_snapshot, None, 'B-id', [('weirdo', ('foo',))])
+            builder.build_snapshot, 'B-id', None, [('weirdo', ('foo',))])
 
     # TODO: rename a file/directory, but rename isn't supported by the
     #       MemoryTree api yet, so for now we wait until it is used
 
     def test_set_parent(self):
         builder = self.build_a_rev()
-        builder.build_snapshot(['A-id'], 'B-id',
+        builder.build_snapshot('B-id', ['A-id'],
             [('modify', ('a-id', 'new\ncontent\n'))])
-        builder.build_snapshot(['A-id'], 'C-id',
+        builder.build_snapshot('C-id', ['A-id'], 
             [('add', ('c', 'c-id', 'file', 'alt\ncontent\n'))])
         # We should now have a graph:
         #   A
@@ -215,11 +215,11 @@
 
     def test_set_merge_parent(self):
         builder = self.build_a_rev()
-        builder.build_snapshot(['A-id'], 'B-id',
+        builder.build_snapshot('B-id', ['A-id'],
             [('add', ('b', 'b-id', 'file', 'b\ncontent\n'))])
-        builder.build_snapshot(['A-id'], 'C-id',
+        builder.build_snapshot('C-id', ['A-id'],
             [('add', ('c', 'c-id', 'file', 'alt\ncontent\n'))])
-        builder.build_snapshot(['B-id', 'C-id'], 'D-id', [])
+        builder.build_snapshot('D-id', ['B-id', 'C-id'], [])
         repo = builder.get_branch().repository
         repo.lock_read()
         self.addCleanup(repo.unlock)
@@ -236,11 +236,11 @@
 
     def test_set_merge_parent_and_contents(self):
         builder = self.build_a_rev()
-        builder.build_snapshot(['A-id'], 'B-id',
+        builder.build_snapshot('B-id', ['A-id'],
             [('add', ('b', 'b-id', 'file', 'b\ncontent\n'))])
-        builder.build_snapshot(['A-id'], 'C-id',
+        builder.build_snapshot('C-id', ['A-id'],
             [('add', ('c', 'c-id', 'file', 'alt\ncontent\n'))])
-        builder.build_snapshot(['B-id', 'C-id'], 'D-id',
+        builder.build_snapshot('D-id', ['B-id', 'C-id'],
             [('add', ('c', 'c-id', 'file', 'alt\ncontent\n'))])
         repo = builder.get_branch().repository
         repo.lock_read()



More information about the bazaar-commits mailing list