Rev 2507: Optimize TreeTransform to avoid renames where possible in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Tue Jun 5 17:48:11 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2507
revision-id: pqm at pqm.ubuntu.com-20070605164810-ay1hxyvqofffy0me
parent: pqm at pqm.ubuntu.com-20070604194535-ihhpf84qp0icoj2t
parent: abentley at panoramicfeedback.com-20070605161922-0j7x00tazfnh5zf1
committer: Canonical.com Patch Queue Manager<pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2007-06-05 17:48:10 +0100
message:
  Optimize TreeTransform to avoid renames where possible
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/test_transform.py test_transaction.py-20060105172520-b3ffb3946550e6c4
  bzrlib/transform.py            transform.py-20060105172343-dd99e54394d91687
    ------------------------------------------------------------
    revno: 2502.1.11
    merged: abentley at panoramicfeedback.com-20070605161922-0j7x00tazfnh5zf1
    parent: abentley at panoramicfeedback.com-20070605161828-5spb276vsp8fxjx8
    parent: pqm at pqm.ubuntu.com-20070604194535-ihhpf84qp0icoj2t
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: fast-checkout
    timestamp: Tue 2007-06-05 12:19:22 -0400
    message:
      Merge bzr.dev
    ------------------------------------------------------------
    revno: 2502.1.10
    merged: abentley at panoramicfeedback.com-20070605161828-5spb276vsp8fxjx8
    parent: abentley at panoramicfeedback.com-20070605154520-kw4foazx2ayumzpr
    parent: aaron.bentley at utoronto.ca-20070603220035-p9mqj3zzus8ew1bz
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: fast-checkout
    timestamp: Tue 2007-06-05 12:18:28 -0400
    message:
      Merge NEWS
        ------------------------------------------------------------
        revno: 2502.1.5.1.2
        merged: aaron.bentley at utoronto.ca-20070603220035-p9mqj3zzus8ew1bz
        parent: aaron.bentley at utoronto.ca-20070603215101-oljwf5m6fen1s6q1
        committer: Aaron Bentley <aaron.bentley at utoronto.ca>
        branch nick: fast-checkout
        timestamp: Sun 2007-06-03 18:00:35 -0400
        message:
          Fix NEWS
        ------------------------------------------------------------
        revno: 2502.1.5.1.1
        merged: aaron.bentley at utoronto.ca-20070603215101-oljwf5m6fen1s6q1
        parent: aaron.bentley at utoronto.ca-20070603163308-r6el1wy9cvj9h6h2
        committer: Aaron Bentley <aaron.bentley at utoronto.ca>
        branch nick: fast-checkout
        timestamp: Sun 2007-06-03 17:51:01 -0400
        message:
          Add NEWS entry
    ------------------------------------------------------------
    revno: 2502.1.9
    merged: abentley at panoramicfeedback.com-20070605154520-kw4foazx2ayumzpr
    parent: abentley at panoramicfeedback.com-20070605141443-wb5xay9e02lnzk76
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: fast-checkout
    timestamp: Tue 2007-06-05 11:45:20 -0400
    message:
      Add additional test for no-name contents
    ------------------------------------------------------------
    revno: 2502.1.8
    merged: abentley at panoramicfeedback.com-20070605141443-wb5xay9e02lnzk76
    parent: abentley at panoramicfeedback.com-20070604170453-eo9440menna0peju
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: fast-checkout
    timestamp: Tue 2007-06-05 10:14:43 -0400
    message:
      Updates from review comments
    ------------------------------------------------------------
    revno: 2502.1.7
    merged: abentley at panoramicfeedback.com-20070604170453-eo9440menna0peju
    parent: abentley at panoramicfeedback.com-20070604165336-ehy5ckbw9dtbpb78
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: fast-checkout
    timestamp: Mon 2007-06-04 13:04:53 -0400
    message:
      Fix finalize deletion ordering
    ------------------------------------------------------------
    revno: 2502.1.6
    merged: abentley at panoramicfeedback.com-20070604165336-ehy5ckbw9dtbpb78
    parent: aaron.bentley at utoronto.ca-20070603163308-r6el1wy9cvj9h6h2
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: fast-checkout
    timestamp: Mon 2007-06-04 12:53:36 -0400
    message:
      Update from review comments
    ------------------------------------------------------------
    revno: 2502.1.5
    merged: aaron.bentley at utoronto.ca-20070603163308-r6el1wy9cvj9h6h2
    parent: aaron.bentley at utoronto.ca-20070603155659-rimfmfeyw7zgbgzm
    committer: Aaron Bentley <aaron.bentley at utoronto.ca>
    branch nick: fast-checkout
    timestamp: Sun 2007-06-03 12:33:08 -0400
    message:
      Cleanup
    ------------------------------------------------------------
    revno: 2502.1.4
    merged: aaron.bentley at utoronto.ca-20070603155659-rimfmfeyw7zgbgzm
    parent: aaron.bentley at utoronto.ca-20070603041408-j30fz6nncfm9vgtr
    committer: Aaron Bentley <aaron.bentley at utoronto.ca>
    branch nick: fast-checkout
    timestamp: Sun 2007-06-03 11:56:59 -0400
    message:
      Ensure we only reuse limbo names appropriately
    ------------------------------------------------------------
    revno: 2502.1.3
    merged: aaron.bentley at utoronto.ca-20070603041408-j30fz6nncfm9vgtr
    parent: aaron.bentley at utoronto.ca-20070602154854-dxx6c0hbjm2h1jel
    committer: Aaron Bentley <aaron.bentley at utoronto.ca>
    branch nick: fast-checkout
    timestamp: Sun 2007-06-03 00:14:08 -0400
    message:
      Don't cause errors when creating contents for trans_ids with no parent/name
    ------------------------------------------------------------
    revno: 2502.1.2
    merged: aaron.bentley at utoronto.ca-20070602154854-dxx6c0hbjm2h1jel
    parent: aaron.bentley at utoronto.ca-20070602000645-51h5g95v86g8305g
    committer: Aaron Bentley <aaron.bentley at utoronto.ca>
    branch nick: fast-checkout
    timestamp: Sat 2007-06-02 11:48:54 -0400
    message:
      Make the limited-renames functionality safer in the general case
    ------------------------------------------------------------
    revno: 2502.1.1
    merged: aaron.bentley at utoronto.ca-20070602000645-51h5g95v86g8305g
    parent: pqm at pqm.ubuntu.com-20070601221655-eeiryluirj5h73hp
    committer: Aaron Bentley <aaron.bentley at utoronto.ca>
    branch nick: fast-checkout
    timestamp: Fri 2007-06-01 20:06:45 -0400
    message:
      Ensure renames only root children are renamed when building trees
=== modified file 'NEWS'
--- a/NEWS	2007-06-04 18:59:21 +0000
+++ b/NEWS	2007-06-05 16:19:22 +0000
@@ -29,6 +29,10 @@
       understanding what changes have occurred
       (John Arbash Meinel, #83887)
 
+    * TreeTransform avoids many renames when contructing large trees,
+      improving speed.  3.25x speedups have been observed for construction of
+      kernel-sized-trees, and checkouts are 1.28x faster.  (Aaron Bentley)
+
   BUGFIXES:
 
     * ``bzr push`` should only connect to the remote location one time.

=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py	2007-03-11 20:34:48 +0000
+++ b/bzrlib/tests/test_transform.py	2007-06-05 15:45:20 +0000
@@ -394,7 +394,6 @@
         self.assertEqual(os.readlink(self.wt.abspath('oz/wizard')),
                          'wizard-target')
 
-
     def get_conflicted(self):
         create,root = self.get_transform()
         create.new_file('dorothy', root, 'dorothy', 'dorothy-id')
@@ -545,9 +544,11 @@
         wt = transform._tree
         transform.new_file('set_on_creation', root, 'Set on creation', 'soc',
                            True)
-        sac = transform.new_file('set_after_creation', root, 'Set after creation', 'sac')
+        sac = transform.new_file('set_after_creation', root,
+                                 'Set after creation', 'sac')
         transform.set_executability(True, sac)
-        uws = transform.new_file('unset_without_set', root, 'Unset badly', 'uws')
+        uws = transform.new_file('unset_without_set', root, 'Unset badly',
+                                 'uws')
         self.assertRaises(KeyError, transform.set_executability, None, uws)
         transform.apply()
         self.assertTrue(wt.is_executable('soc'))
@@ -774,6 +775,166 @@
         finally:
             transform.finalize()
 
+    def test_rename_count(self):
+        transform, root = self.get_transform()
+        transform.new_file('name1', root, 'contents')
+        self.assertEqual(transform.rename_count, 0)
+        transform.apply()
+        self.assertEqual(transform.rename_count, 1)
+        transform2, root = self.get_transform()
+        transform2.adjust_path('name2', root,
+                               transform2.trans_id_tree_path('name1'))
+        self.assertEqual(transform2.rename_count, 0)
+        transform2.apply()
+        self.assertEqual(transform2.rename_count, 2)
+
+    def test_change_parent(self):
+        """Ensure that after we change a parent, the results are still right.
+
+        Renames and parent changes on pending transforms can happen as part
+        of conflict resolution, and are explicitly permitted by the
+        TreeTransform API.
+
+        This test ensures they work correctly with the rename-avoidance
+        optimization.
+        """
+        transform, root = self.get_transform()
+        parent1 = transform.new_directory('parent1', root)
+        child1 = transform.new_file('child1', parent1, 'contents')
+        parent2 = transform.new_directory('parent2', root)
+        transform.adjust_path('child1', parent2, child1)
+        transform.apply()
+        self.failIfExists(self.wt.abspath('parent1/child1'))
+        self.failUnlessExists(self.wt.abspath('parent2/child1'))
+        # rename limbo/new-1 => parent1, rename limbo/new-3 => parent2
+        # no rename for child1 (counting only renames during apply)
+        self.failUnlessEqual(2, transform.rename_count)
+
+    def test_cancel_parent(self):
+        """Cancelling a parent doesn't cause deletion of a non-empty directory
+
+        This is like the test_change_parent, except that we cancel the parent
+        before adjusting the path.  The transform must detect that the
+        directory is non-empty, and move children to safe locations.
+        """
+        transform, root = self.get_transform()
+        parent1 = transform.new_directory('parent1', root)
+        child1 = transform.new_file('child1', parent1, 'contents')
+        child2 = transform.new_file('child2', parent1, 'contents')
+        try:
+            transform.cancel_creation(parent1)
+        except OSError:
+            self.fail('Failed to move child1 before deleting parent1')
+        transform.cancel_creation(child2)
+        transform.create_directory(parent1)
+        try:
+            transform.cancel_creation(parent1)
+        # If the transform incorrectly believes that child2 is still in
+        # parent1's limbo directory, it will try to rename it and fail
+        # because was already moved by the first cancel_creation.
+        except OSError:
+            self.fail('Transform still thinks child2 is a child of parent1')
+        parent2 = transform.new_directory('parent2', root)
+        transform.adjust_path('child1', parent2, child1)
+        transform.apply()
+        self.failIfExists(self.wt.abspath('parent1'))
+        self.failUnlessExists(self.wt.abspath('parent2/child1'))
+        # rename limbo/new-3 => parent2, rename limbo/new-2 => child1
+        self.failUnlessEqual(2, transform.rename_count)
+
+    def test_adjust_and_cancel(self):
+        """Make sure adjust_path keeps track of limbo children properly"""
+        transform, root = self.get_transform()
+        parent1 = transform.new_directory('parent1', root)
+        child1 = transform.new_file('child1', parent1, 'contents')
+        parent2 = transform.new_directory('parent2', root)
+        transform.adjust_path('child1', parent2, child1)
+        transform.cancel_creation(child1)
+        try:
+            transform.cancel_creation(parent1)
+        # if the transform thinks child1 is still in parent1's limbo
+        # directory, it will attempt to move it and fail.
+        except OSError:
+            self.fail('Transform still thinks child1 is a child of parent1')
+        transform.finalize()
+
+    def test_noname_contents(self):
+        """TreeTransform should permit deferring naming files."""
+        transform, root = self.get_transform()
+        parent = transform.trans_id_file_id('parent-id')
+        try:
+            transform.create_directory(parent)
+        except KeyError:
+            self.fail("Can't handle contents with no name")
+        transform.finalize()
+
+    def test_noname_contents_nested(self):
+        """TreeTransform should permit deferring naming files."""
+        transform, root = self.get_transform()
+        parent = transform.trans_id_file_id('parent-id')
+        try:
+            transform.create_directory(parent)
+        except KeyError:
+            self.fail("Can't handle contents with no name")
+        child = transform.new_directory('child', parent)
+        transform.adjust_path('parent', root, parent)
+        transform.apply()
+        self.failUnlessExists(self.wt.abspath('parent/child'))
+        self.assertEqual(1, transform.rename_count)
+
+    def test_reuse_name(self):
+        """Avoid reusing the same limbo name for different files"""
+        transform, root = self.get_transform()
+        parent = transform.new_directory('parent', root)
+        child1 = transform.new_directory('child', parent)
+        try:
+            child2 = transform.new_directory('child', parent)
+        except OSError:
+            self.fail('Tranform tried to use the same limbo name twice')
+        transform.adjust_path('child2', parent, child2)
+        transform.apply()
+        # limbo/new-1 => parent, limbo/new-3 => parent/child2
+        # child2 is put into top-level limbo because child1 has already
+        # claimed the direct limbo path when child2 is created.  There is no
+        # advantage in renaming files once they're in top-level limbo, except
+        # as part of apply.
+        self.assertEqual(2, transform.rename_count)
+
+    def test_reuse_when_first_moved(self):
+        """Don't avoid direct paths when it is safe to use them"""
+        transform, root = self.get_transform()
+        parent = transform.new_directory('parent', root)
+        child1 = transform.new_directory('child', parent)
+        transform.adjust_path('child1', parent, child1)
+        child2 = transform.new_directory('child', parent)
+        transform.apply()
+        # limbo/new-1 => parent
+        self.assertEqual(1, transform.rename_count)
+
+    def test_reuse_after_cancel(self):
+        """Don't avoid direct paths when it is safe to use them"""
+        transform, root = self.get_transform()
+        parent2 = transform.new_directory('parent2', root)
+        child1 = transform.new_directory('child1', parent2)
+        transform.cancel_creation(parent2)
+        transform.create_directory(parent2)
+        child2 = transform.new_directory('child1', parent2)
+        transform.adjust_path('child2', parent2, child1)
+        transform.apply()
+        # limbo/new-1 => parent2, limbo/new-2 => parent2/child1
+        self.assertEqual(2, transform.rename_count)
+
+    def test_finalize_order(self):
+        """Finalize must be done in child-to-parent order"""
+        transform, root = self.get_transform()
+        parent = transform.new_directory('parent', root)
+        child = transform.new_directory('child', parent)
+        try:
+            transform.finalize()
+        except OSError:
+            self.fail('Tried to remove parent before child1')
+
+
 class TransformGroup(object):
     def __init__(self, dirname, root_id):
         self.name = dirname
@@ -1115,6 +1276,23 @@
         self.assertRaises(errors.WorkingTreeAlreadyPopulated, 
             build_tree, source.basis_tree(), target)
 
+    def test_build_tree_rename_count(self):
+        source = self.make_branch_and_tree('source')
+        self.build_tree(['source/file1', 'source/dir1/'])
+        source.add(['file1', 'dir1'])
+        source.commit('add1')
+        target1 = self.make_branch_and_tree('target1')
+        transform_result = build_tree(source.basis_tree(), target1)
+        self.assertEqual(2, transform_result.rename_count)
+
+        self.build_tree(['source/dir1/file2'])
+        source.add(['dir1/file2'])
+        source.commit('add3')
+        target2 = self.make_branch_and_tree('target2')
+        transform_result = build_tree(source.basis_tree(), target2)
+        # children of non-root directories should not be renamed
+        self.assertEqual(2, transform_result.rename_count)
+
 
 class MockTransform(object):
 
@@ -1127,6 +1305,7 @@
                 return True
         return False
 
+
 class MockEntry(object):
     def __init__(self):
         object.__init__(self)

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2007-06-01 05:43:11 +0000
+++ b/bzrlib/transform.py	2007-06-05 14:14:43 +0000
@@ -51,16 +51,21 @@
 
 
 class _TransformResults(object):
-    def __init__(self, modified_paths):
+    def __init__(self, modified_paths, rename_count):
         object.__init__(self)
         self.modified_paths = modified_paths
+        self.rename_count = rename_count
 
 
 class TreeTransform(object):
     """Represent a tree transformation.
     
     This object is designed to support incremental generation of the transform,
-    in any order.  
+    in any order.
+
+    However, it gives optimum performance when parent directories are created
+    before their contents.  The transform is then able to put child files
+    directly in their parent directory, avoiding later renames.
     
     It is easy to produce malformed transforms, but they are generally
     harmless.  Attempting to apply a malformed transform will cause an
@@ -84,7 +89,8 @@
     def __init__(self, tree, pb=DummyProgress()):
         """Note: a tree_write lock is taken on the tree.
         
-        Use TreeTransform.finalize() to release the lock
+        Use TreeTransform.finalize() to release the lock (can be omitted if
+        TreeTransform.apply() called).
         """
         object.__init__(self)
         self._tree = tree
@@ -106,6 +112,15 @@
         self._new_name = {}
         self._new_parent = {}
         self._new_contents = {}
+        # A mapping of transform ids to their limbo filename
+        self._limbo_files = {}
+        # A mapping of transform ids to a set of the transform ids of children
+        # that their limbo directory has
+        self._limbo_children = {}
+        # Map transform ids to maps of child filename to child transform id
+        self._limbo_children_names = {}
+        # List of transform ids that need to be renamed from limbo into place
+        self._needs_rename = set()
         self._removed_contents = set()
         self._new_executability = {}
         self._new_reference_revision = {}
@@ -115,13 +130,14 @@
         self._removed_id = set()
         self._tree_path_ids = {}
         self._tree_id_paths = {}
+        # Cache of realpath results, to speed up canonical_path
         self._realpaths = {}
-        # Cache of realpath results, to speed up canonical_path
+        # Cache of relpath results, to speed up canonical_path
         self._relpaths = {}
-        # Cache of relpath results, to speed up canonical_path
         self._new_root = self.trans_id_tree_file_id(tree.get_root_id())
         self.__done = False
         self._pb = pb
+        self.rename_count = 0
 
     def __get_root(self):
         return self._new_root
@@ -129,12 +145,18 @@
     root = property(__get_root)
 
     def finalize(self):
-        """Release the working tree lock, if held, clean up limbo dir."""
+        """Release the working tree lock, if held, clean up limbo dir.
+
+        This is required if apply has not been invoked, but can be invoked
+        even after apply.
+        """
         if self._tree is None:
             return
         try:
-            for trans_id, kind in self._new_contents.iteritems():
-                path = self._limbo_name(trans_id)
+            entries = [(self._limbo_name(t), t, k) for t, k in
+                       self._new_contents.iteritems()]
+            entries.sort(reverse=True)
+            for path, trans_id, kind in entries:
                 if kind == "directory":
                     os.rmdir(path)
                 else:
@@ -165,8 +187,28 @@
         """Change the path that is assigned to a transaction id."""
         if trans_id == self._new_root:
             raise CantMoveRoot
+        previous_parent = self._new_parent.get(trans_id)
+        previous_name = self._new_name.get(trans_id)
         self._new_name[trans_id] = name
         self._new_parent[trans_id] = parent
+        if (trans_id in self._limbo_files and
+            trans_id not in self._needs_rename):
+            self._rename_in_limbo([trans_id])
+            self._limbo_children[previous_parent].remove(trans_id)
+            del self._limbo_children_names[previous_parent][previous_name]
+
+    def _rename_in_limbo(self, trans_ids):
+        """Fix limbo names so that the right final path is produced.
+
+        This means we outsmarted ourselves-- we tried to avoid renaming
+        these files later by creating them with their final names in their
+        final parents.  But now the previous name or parent is no longer
+        suitable, so we have to rename them.
+        """
+        for trans_id in trans_ids:
+            old_path = self._limbo_files[trans_id]
+            new_path = self._limbo_name(trans_id, from_scratch=True)
+            os.rename(old_path, new_path)
 
     def adjust_root_path(self, name, parent):
         """Emulate moving the root by moving all children, instead.
@@ -330,6 +372,13 @@
     def cancel_creation(self, trans_id):
         """Cancel the creation of new file contents."""
         del self._new_contents[trans_id]
+        children = self._limbo_children.get(trans_id)
+        # if this is a limbo directory with children, move them before removing
+        # the directory
+        if children is not None:
+            self._rename_in_limbo(children)
+            del self._limbo_children[trans_id]
+            del self._limbo_children_names[trans_id]
         delete_any(self._limbo_name(trans_id))
 
     def delete_contents(self, trans_id):
@@ -734,6 +783,8 @@
         
         If filesystem or inventory conflicts are present, MalformedTransform
         will be thrown.
+
+        If apply succeeds, finalize is not necessary.
         """
         conflicts = self.find_conflicts()
         if len(conflicts) != 0:
@@ -751,11 +802,41 @@
         self._tree.apply_inventory_delta(inventory_delta)
         self.__done = True
         self.finalize()
-        return _TransformResults(modified_paths)
+        return _TransformResults(modified_paths, self.rename_count)
 
-    def _limbo_name(self, trans_id):
+    def _limbo_name(self, trans_id, from_scratch=False):
         """Generate the limbo name of a file"""
-        return pathjoin(self._limbodir, trans_id)
+        if not from_scratch:
+            limbo_name = self._limbo_files.get(trans_id)
+            if limbo_name is not None:
+                return limbo_name
+        parent = self._new_parent.get(trans_id)
+        # if the parent directory is already in limbo (e.g. when building a
+        # tree), choose a limbo name inside the parent, to reduce further
+        # renames.
+        use_direct_path = False
+        if self._new_contents.get(parent) == 'directory':
+            filename = self._new_name.get(trans_id)
+            if filename is not None:
+                if parent not in self._limbo_children:
+                    self._limbo_children[parent] = set()
+                    self._limbo_children_names[parent] = {}
+                    use_direct_path = True
+                # the direct path can only be used if no other file has
+                # already taken this pathname, i.e. if the name is unused, or
+                # if it is already associated with this trans_id.
+                elif (self._limbo_children_names[parent].get(filename)
+                      in (trans_id, None)):
+                    use_direct_path = True
+        if use_direct_path:
+            limbo_name = pathjoin(self._limbo_files[parent], filename)
+            self._limbo_children[parent].add(trans_id)
+            self._limbo_children_names[parent][filename] = trans_id
+        else:
+            limbo_name = pathjoin(self._limbodir, trans_id)
+            self._needs_rename.add(trans_id)
+        self._limbo_files[trans_id] = limbo_name
+        return limbo_name
 
     def _apply_removals(self, inv, inventory_delta):
         """Perform tree operations that remove directory/inventory names.
@@ -781,6 +862,8 @@
                     except OSError, e:
                         if e.errno != errno.ENOENT:
                             raise
+                    else:
+                        self.rename_count += 1
                 if trans_id in self._removed_id:
                     if trans_id == self._new_root:
                         file_id = self._tree.inventory.root.file_id
@@ -812,12 +895,15 @@
                 if trans_id in self._new_contents or \
                     self.path_changed(trans_id):
                     full_path = self._tree.abspath(path)
-                    try:
-                        os.rename(self._limbo_name(trans_id), full_path)
-                    except OSError, e:
-                        # We may be renaming a dangling inventory id
-                        if e.errno != errno.ENOENT:
-                            raise
+                    if trans_id in self._needs_rename:
+                        try:
+                            os.rename(self._limbo_name(trans_id), full_path)
+                        except OSError, e:
+                            # We may be renaming a dangling inventory id
+                            if e.errno != errno.ENOENT:
+                                raise
+                        else:
+                            self.rename_count += 1
                     if trans_id in self._new_contents:
                         modified_paths.append(full_path)
                         del self._new_contents[trans_id]
@@ -1151,9 +1237,9 @@
     top_pb = bzrlib.ui.ui_factory.nested_progress_bar()
     pp = ProgressPhase("Build phase", 2, top_pb)
     if tree.inventory.root is not None:
-        # this is kindof a hack: we should be altering the root 
-        # as partof the regular tree shape diff logic.
-        # the conditional test hereis to avoid doing an
+        # This is kind of a hack: we should be altering the root
+        # as part of the regular tree shape diff logic.
+        # The conditional test here is to avoid doing an
         # expensive operation (flush) every time the root id
         # is set within the tree, nor setting the root and thus
         # marking the tree as dirty, because we use two different
@@ -1219,10 +1305,11 @@
             wt.add_conflicts(conflicts)
         except errors.UnsupportedOperation:
             pass
-        tt.apply()
+        result = tt.apply()
     finally:
         tt.finalize()
         top_pb.finished()
+    return result
 
 
 def _reparent_children(tt, old_parent, new_parent):
@@ -1469,9 +1556,9 @@
                 if kind[0] == 'file' and (backups or kind[1] is None):
                     wt_sha1 = working_tree.get_file_sha1(file_id)
                     if merge_modified.get(file_id) != wt_sha1:
-                        # acquire the basis tree lazyily to prevent the expense
-                        # of accessing it when its not needed ? (Guessing, RBC,
-                        # 200702)
+                        # acquire the basis tree lazily to prevent the
+                        # expense of accessing it when it's not needed ?
+                        # (Guessing, RBC, 200702)
                         if basis_tree is None:
                             basis_tree = working_tree.basis_tree()
                             basis_tree.lock_read()




More information about the bazaar-commits mailing list