Merge lp:~jameinel/bzr/2.0.4-change-root-id-494269 into lp:bzr/2.0

[John A Meinel]

[John A Meinel (jameinel)]

This patch is done to address bug #494269.

Namely, if a transform included changing the root's file-id, it would fail in a variety of ways. One way is that 'adjust_path' would get called, and it would fail claiming that you can't have more than one root. Aaron fixed some of that in his nested-trees branch, so I'm just cherrypicking that out here.

This means that 'bzr revert' can now handle the root-id changing.

In testing, 'revert' works, but 'update' is still broken, because of bug #504390. I'll start working on that one next.

[Robert Collins (lifeless)]

 review: +1

review: Approve

=== modified file 'NEWS'
--- NEWS	2010-01-08 01:08:40 +0000
+++ NEWS	2010-01-12 04:33:14 +0000
@@ -51,6 +51,13 @@
   changed underneath it (like another autopack). Now concurrent
   autopackers will properly succeed. (John Arbash Meinel, #495000)
 
+* ``TreeTransform`` can now handle when a delta says that the file id for
+  the tree root changes. Rather than trying to rename your working
+  directory, or failing early saying that you can't have multiple
+  tree roots. This also fixes ``bzr revert`` when the root id changes.
+  (Currently ``bzr update`` is still broken.)
+  (John Arbash Meinel, #494269)
+
 * ``_update_current_block`` no longer suppresses exceptions, so ^C at just
   the right time will get propagated, rather than silently failing to move
   the block pointer. (John Arbash Meinel, Gareth White, #495023)

=== modified file 'bzrlib/tests/test_revert.py'
--- bzrlib/tests/test_revert.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/test_revert.py	2010-01-12 04:33:14 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 Canonical Ltd
+# Copyright (C) 2006, 2007, 2009, 2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -157,3 +157,14 @@
         self.failUnlessExists('dir/file1')
         self.failIfExists('dir/file2')
         self.assertEqual('dir-id', tree.path2id('dir'))
+
+    def test_revert_root_id_change(self):
+        tree = self.make_branch_and_tree('.')
+        tree.set_root_id('initial-root-id')
+        self.build_tree(['file1'])
+        tree.add(['file1'])
+        tree.commit('first')
+        tree.set_root_id('temp-root-id')
+        self.assertEqual('temp-root-id', tree.get_root_id())
+        tree.revert()
+        self.assertEqual('initial-root-id', tree.get_root_id())

=== modified file 'bzrlib/tests/test_transform.py'
--- bzrlib/tests/test_transform.py	2009-10-14 18:37:13 +0000
+++ bzrlib/tests/test_transform.py	2010-01-12 04:33:14 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006, 2007, 2008 Canonical Ltd
+# Copyright (C) 2006, 2007, 2008, 2009, 2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -136,6 +136,36 @@
         transform.finalize()
         transform.finalize()
 
+    def test_change_root_id(self):
+        transform, root = self.get_transform()
+        self.assertNotEqual('new-root-id', self.wt.get_root_id())
+        transform.new_directory('', ROOT_PARENT, 'new-root-id')
+        transform.delete_contents(root)
+        transform.unversion_file(root)
+        transform.fixup_new_roots()
+        transform.apply()
+        self.assertEqual('new-root-id', self.wt.get_root_id())
+
+    def test_change_root_id_add_files(self):
+        transform, root = self.get_transform()
+        self.assertNotEqual('new-root-id', self.wt.get_root_id())
+        new_trans_id = transform.new_directory('', ROOT_PARENT, 'new-root-id')
+        transform.new_file('file', new_trans_id, ['new-contents\n'],
+                           'new-file-id')
+        transform.delete_contents(root)
+        transform.unversion_file(root)
+        transform.fixup_new_roots()
+        transform.apply()
+        self.assertEqual('new-root-id', self.wt.get_root_id())
+        self.assertEqual('new-file-id', self.wt.path2id('file'))
+        self.assertFileEqual('new-contents\n', self.wt.abspath('file'))
+
+    def test_add_two_roots(self):
+        transform, root = self.get_transform()
+        new_trans_id = transform.new_directory('', ROOT_PARENT, 'new-root-id')
+        new_trans_id = transform.new_directory('', ROOT_PARENT, 'alt-root-id')
+        self.assertRaises(ValueError, transform.fixup_new_roots)
+
     def test_hardlink(self):
         self.requireFeature(HardlinkFeature)
         transform, root = self.get_transform()
@@ -746,7 +776,7 @@
         create.apply()
         transform, root = self.get_transform()
         transform.adjust_root_path('oldroot', fun)
-        new_root=transform.trans_id_tree_path('')
+        new_root = transform.trans_id_tree_path('')
         transform.version_file('new-root', new_root)
         transform.apply()
 
@@ -2428,7 +2458,7 @@
 
     def test_file_content_summary_executable(self):
         if not osutils.supports_executable():
-            raise TestNotApplicable()
+            raise tests.TestNotApplicable()
         preview = self.get_empty_preview()
         path_id = preview.new_file('path', preview.root, 'contents', 'path-id')
         preview.set_executability(True, path_id)
@@ -2443,8 +2473,6 @@
         self.assertIs(None, summary[3])
 
     def test_change_executability(self):
-        if not osutils.supports_executable():
-            raise TestNotApplicable()
         tree = self.make_branch_and_tree('tree')
         self.build_tree(['tree/path'])
         tree.add('path')
@@ -2616,7 +2644,7 @@
         preview = self.get_empty_preview()
         root = preview.new_directory('', ROOT_PARENT, 'tree-root')
         # FIXME: new_directory should mark root.
-        preview.adjust_path('', ROOT_PARENT, root)
+        preview.fixup_new_roots()
         preview_tree = preview.get_preview_tree()
         file_trans_id = preview.new_file('a', preview.root, 'contents',
                                          'a-id')

=== modified file 'bzrlib/transform.py'
--- bzrlib/transform.py	2009-10-28 07:34:55 +0000
+++ bzrlib/transform.py	2010-01-12 04:33:14 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006, 2007, 2008, 2009 Canonical Ltd
+# Copyright (C) 2006, 2007, 2008, 2009, 2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -161,14 +161,12 @@
 
     def adjust_path(self, name, parent, trans_id):
         """Change the path that is assigned to a transaction id."""
+        if parent is None:
+            raise ValueError("Parent trans-id may not be None")
         if trans_id == self._new_root:
             raise CantMoveRoot
         self._new_name[trans_id] = name
         self._new_parent[trans_id] = parent
-        if parent == ROOT_PARENT:
-            if self._new_root is not None:
-                raise ValueError("Cannot have multiple roots.")
-            self._new_root = trans_id
 
     def adjust_root_path(self, name, parent):
         """Emulate moving the root by moving all children, instead.
@@ -202,6 +200,67 @@
         self.version_file(old_root_file_id, old_root)
         self.unversion_file(self._new_root)
 
+    def fixup_new_roots(self):
+        """Reinterpret requests to change the root directory
+
+        Instead of creating a root directory, or moving an existing directory,
+        all the attributes and children of the new root are applied to the
+        existing root directory.
+
+        This means that the old root trans-id becomes obsolete, so it is
+        recommended only to invoke this after the root trans-id has become
+        irrelevant.
+        """
+        new_roots = [k for k, v in self._new_parent.iteritems() if v is
+                     ROOT_PARENT]
+        if len(new_roots) < 1:
+            return
+        if len(new_roots) != 1:
+            raise ValueError('A tree cannot have two roots!')
+        if self._new_root is None:
+            self._new_root = new_roots[0]
+            return
+        old_new_root = new_roots[0]
+        # TODO: What to do if a old_new_root is present, but self._new_root is
+        #       not listed as being removed? This code explicitly unversions
+        #       the old root and versions it with the new file_id. Though that
+        #       seems like an incomplete delta
+
+        # unversion the new root's directory.
+        file_id = self.final_file_id(old_new_root)
+        if old_new_root in self._new_id:
+            self.cancel_versioning(old_new_root)
+        else:
+            self.unversion_file(old_new_root)
+        # if, at this stage, root still has an old file_id, zap it so we can
+        # stick a new one in.
+        if (self.tree_file_id(self._new_root) is not None and
+            self._new_root not in self._removed_id):
+            self.unversion_file(self._new_root)
+        self.version_file(file_id, self._new_root)
+
+        # Now move children of new root into old root directory.
+        # Ensure all children are registered with the transaction, but don't
+        # use directly-- some tree children have new parents
+        list(self.iter_tree_children(old_new_root))
+        # Move all children of new root into old root directory.
+        for child in self.by_parent().get(old_new_root, []):
+            self.adjust_path(self.final_name(child), self._new_root, child)
+
+        # Ensure old_new_root has no directory.
+        if old_new_root in self._new_contents:
+            self.cancel_creation(old_new_root)
+        else:
+            self.delete_contents(old_new_root)
+
+        # prevent deletion of root directory.
+        if self._new_root in self._removed_contents:
+            self.cancel_deletion(self._new_root)
+
+        # destroy path info for old_new_root.
+        del self._new_parent[old_new_root]
+        del self._new_name[old_new_root]
+
     def trans_id_tree_file_id(self, inventory_id):
         """Determine the transaction id of a working tree file.
 
@@ -253,6 +312,8 @@
 
     def delete_contents(self, trans_id):
         """Schedule the contents of a path entry for deletion"""
+        # Ensure that the object exists in the WorkingTree, this will raise an
+        # exception if there is a problem
         self.tree_kind(trans_id)
         self._removed_contents.add(trans_id)
 
@@ -1075,8 +1136,10 @@
         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]
+            if previous_parent != parent:
+                self._limbo_children[previous_parent].remove(trans_id)
+            if previous_parent != parent or previous_name != name:
+                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.
@@ -1542,10 +1605,10 @@
                 child_pb.update('removing file', num, len(tree_paths))
                 full_path = self._tree.abspath(path)
                 if trans_id in self._removed_contents:
-                    mover.pre_delete(full_path, os.path.join(self._deletiondir,
-                                     trans_id))
-                elif trans_id in self._new_name or trans_id in \
-                    self._new_parent:
+                    delete_path = os.path.join(self._deletiondir, trans_id)
+                    mover.pre_delete(full_path, delete_path)
+                elif (trans_id in self._new_name
+                      or trans_id in self._new_parent):
                     try:
                         mover.rename(full_path, self._limbo_name(trans_id))
                     except OSError, e:
@@ -2636,7 +2699,10 @@
                     parent_trans = ROOT_PARENT
                 else:
                     parent_trans = tt.trans_id_file_id(parent[1])
-                tt.adjust_path(name[1], parent_trans, trans_id)
+                if parent[0] is None and versioned[0]:
+                    tt.adjust_root_path(name[1], parent_trans)
+                else:
+                    tt.adjust_path(name[1], parent_trans, trans_id)
             if executable[0] != executable[1] and kind[1] == "file":
                 tt.set_executability(executable[1], trans_id)
         if working_tree.supports_content_filtering():
@@ -2655,6 +2721,7 @@
             for (trans_id, mode_id), bytes in target_tree.iter_files_bytes(
                 deferred_files):
                 tt.create_file(bytes, trans_id, mode_id)
+        tt.fixup_new_roots()
     finally:
         if basis_tree is not None:
             basis_tree.unlock()