Merge lp:~bialix/bzr/bzr-1.18-oslocks-win32 into lp:bzr/1.18

[Alexander Belchenko]

[Alexander Belchenko (bialix)]

I've cherrypicked 2 revisions from bzr.dev (4635 and 4650) which contains fixes for OS locks problems on Windows (local push and shelve).

Will be nice if Robert will review result of cherrypick and then I'd like to propose merge it back to 1.18 branch and prepare 1.18.1 release soon. Even if this will be win32-only release.

I think this will help to get more testing of these changes and provides more windows users benefits of fixes.

[Robert Collins (lifeless)]

review +1

[Vincent Ladeuil (vila)]

Robert forgets the leading space so his votes get ignored.

review: Approve

=== modified file 'NEWS'
--- NEWS	2009-08-25 05:32:07 +0000
+++ NEWS	2009-08-27 08:35:10 +0000
@@ -17,9 +17,22 @@
   conversion will commit too many copies a file.
   (Martin Pool, #415508)
 
+Improvements
+************
+
+* ``bzr push`` locally on windows will no longer give a locking error with
+  dirstate based formats. (Robert Collins)
+
+* ``bzr shelve`` and ``bzr unshelve`` now work on windows.
+  (Robert Collins, #305006)
+
 API Changes
 ***********
 
+* ``bzrlib.shelf_ui`` has had the ``from_args`` convenience methods of its
+  classes changed to manage lock lifetime of the trees they open in a way
+  consistent with reader-exclusive locks. (Robert Collins, #305006)
+
 * ``Tree.path_content_summary`` may return a size of None, when called on
   a tree with content filtering where the size of the canonical form
   cannot be cheaply determined.  (Martin Pool)

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2009-07-27 06:22:57 +0000
+++ bzrlib/builtins.py	2009-08-27 08:35:10 +0000
@@ -120,6 +120,15 @@
 
 
 def _get_one_revision_tree(command_name, revisions, branch=None, tree=None):
+    """Get a revision tree. Not suitable for commands that change the tree.
+    
+    Specifically, the basis tree in dirstate trees is coupled to the dirstate
+    and doing a commit/uncommit/pull will at best fail due to changing the
+    basis revision data.
+
+    If tree is passed in, it should be already locked, for lifetime management
+    of the trees internal cached state.
+    """
     if branch is None:
         branch = tree.branch
     if revisions is None:
@@ -5627,8 +5636,12 @@
         if writer is None:
             writer = bzrlib.option.diff_writer_registry.get()
         try:
-            Shelver.from_args(writer(sys.stdout), revision, all, file_list,
-                              message, destroy=destroy).run()
+            shelver = Shelver.from_args(writer(sys.stdout), revision, all,
+                file_list, message, destroy=destroy)
+            try:
+                shelver.run()
+            finally:
+                shelver.work_tree.unlock()
         except errors.UserAbort:
             return 0
 
@@ -5673,7 +5686,11 @@
 
     def run(self, shelf_id=None, action='apply'):
         from bzrlib.shelf_ui import Unshelver
-        Unshelver.from_args(shelf_id, action).run()
+        unshelver = Unshelver.from_args(shelf_id, action)
+        try:
+            unshelver.run()
+        finally:
+            unshelver.tree.unlock()
 
 
 class cmd_clean_tree(Command):

=== modified file 'bzrlib/merge.py'
--- bzrlib/merge.py	2009-07-02 13:07:14 +0000
+++ bzrlib/merge.py	2009-08-27 08:35:10 +0000
@@ -64,8 +64,12 @@
 
 
 def transform_tree(from_tree, to_tree, interesting_ids=None):
-    merge_inner(from_tree.branch, to_tree, from_tree, ignore_zero=True,
-                interesting_ids=interesting_ids, this_tree=from_tree)
+    from_tree.lock_tree_write()
+    try:
+        merge_inner(from_tree.branch, to_tree, from_tree, ignore_zero=True,
+                    interesting_ids=interesting_ids, this_tree=from_tree)
+    finally:
+        from_tree.unlock()
 
 
 class Merger(object):
@@ -102,6 +106,17 @@
         self._is_criss_cross = None
         self._lca_trees = None
 
+    def cache_trees_with_revision_ids(self, trees):
+        """Cache any tree in trees if it has a revision_id."""
+        for maybe_tree in trees:
+            if maybe_tree is None:
+                continue
+            try:
+                rev_id = maybe_tree.get_revision_id()
+            except AttributeError:
+                continue
+            self._cached_trees[rev_id] = maybe_tree
+
     @property
     def revision_graph(self):
         if self._revision_graph is None:
@@ -1516,6 +1531,7 @@
     get_revision_id = getattr(base_tree, 'get_revision_id', None)
     if get_revision_id is None:
         get_revision_id = base_tree.last_revision
+    merger.cache_trees_with_revision_ids([other_tree, base_tree, this_tree])
     merger.set_base_revision(get_revision_id(), this_branch)
     return merger.do_merge()
 

=== modified file 'bzrlib/shelf.py'
--- bzrlib/shelf.py	2009-07-13 17:35:09 +0000
+++ bzrlib/shelf.py	2009-08-27 08:35:10 +0000
@@ -37,8 +37,10 @@
     def __init__(self, work_tree, target_tree, file_list=None):
         """Constructor.
 
-        :param work_tree: The working tree to apply changes to
+        :param work_tree: The working tree to apply changes to. This is not
+            required to be locked - a tree_write lock will be taken out.
         :param target_tree: The tree to make the working tree more similar to.
+            This is not required to be locked - a read_lock will be taken out.
         :param file_list: The files to make more similar to the target.
         """
         self.work_tree = work_tree

=== modified file 'bzrlib/shelf_ui.py'
--- bzrlib/shelf_ui.py	2009-07-13 13:00:27 +0000
+++ bzrlib/shelf_ui.py	2009-08-27 08:35:10 +0000
@@ -149,6 +149,9 @@
                   message=None, directory='.', destroy=False):
         """Create a shelver from commandline arguments.
 
+        The returned shelver wil have a work_tree that is locked and should
+        be unlocked.
+
         :param revision: RevisionSpec of the revision to compare to.
         :param all: If True, shelve all changes without prompting.
         :param file_list: If supplied, only files in this list may be  shelved.
@@ -158,9 +161,16 @@
             changes.
         """
         tree, path = workingtree.WorkingTree.open_containing(directory)
-        target_tree = builtins._get_one_revision_tree('shelf2', revision,
-            tree.branch, tree)
-        files = builtins.safe_relpath_files(tree, file_list)
+        # Ensure that tree is locked for the lifetime of target_tree, as
+        # target tree may be reading from the same dirstate.
+        tree.lock_tree_write()
+        try:
+            target_tree = builtins._get_one_revision_tree('shelf2', revision,
+                tree.branch, tree)
+            files = builtins.safe_relpath_files(tree, file_list)
+        except:
+            tree.unlock()
+            raise
         return klass(tree, target_tree, diff_writer, all, all, files, message,
                      destroy)
 
@@ -313,32 +323,40 @@
     def from_args(klass, shelf_id=None, action='apply', directory='.'):
         """Create an unshelver from commandline arguments.
 
+        The returned shelver wil have a tree that is locked and should
+        be unlocked.
+
         :param shelf_id: Integer id of the shelf, as a string.
         :param action: action to perform.  May be 'apply', 'dry-run',
             'delete'.
         :param directory: The directory to unshelve changes into.
         """
         tree, path = workingtree.WorkingTree.open_containing(directory)
-        manager = tree.get_shelf_manager()
-        if shelf_id is not None:
-            try:
-                shelf_id = int(shelf_id)
-            except ValueError:
-                raise errors.InvalidShelfId(shelf_id)
-        else:
-            shelf_id = manager.last_shelf()
-            if shelf_id is None:
-                raise errors.BzrCommandError('No changes are shelved.')
-            trace.note('Unshelving changes with id "%d".' % shelf_id)
-        apply_changes = True
-        delete_shelf = True
-        read_shelf = True
-        if action == 'dry-run':
-            apply_changes = False
-            delete_shelf = False
-        if action == 'delete-only':
-            apply_changes = False
-            read_shelf = False
+        tree.lock_tree_write()
+        try:
+            manager = tree.get_shelf_manager()
+            if shelf_id is not None:
+                try:
+                    shelf_id = int(shelf_id)
+                except ValueError:
+                    raise errors.InvalidShelfId(shelf_id)
+            else:
+                shelf_id = manager.last_shelf()
+                if shelf_id is None:
+                    raise errors.BzrCommandError('No changes are shelved.')
+                trace.note('Unshelving changes with id "%d".' % shelf_id)
+            apply_changes = True
+            delete_shelf = True
+            read_shelf = True
+            if action == 'dry-run':
+                apply_changes = False
+                delete_shelf = False
+            if action == 'delete-only':
+                apply_changes = False
+                read_shelf = False
+        except:
+            tree.unlock()
+            raise
         return klass(tree, manager, shelf_id, apply_changes, delete_shelf,
                      read_shelf)
 
@@ -364,7 +382,7 @@
 
     def run(self):
         """Perform the unshelving operation."""
-        self.tree.lock_write()
+        self.tree.lock_tree_write()
         cleanups = [self.tree.unlock]
         try:
             if self.read_shelf:

=== modified file 'bzrlib/tests/per_workingtree/test_executable.py'
--- bzrlib/tests/per_workingtree/test_executable.py	2009-07-10 07:14:02 +0000
+++ bzrlib/tests/per_workingtree/test_executable.py	2009-08-27 08:35:10 +0000
@@ -30,7 +30,6 @@
 
     def setUp(self):
         super(TestExecutable, self).setUp()
-
         self.a_id = "a-20051208024829-849e76f7968d7a86"
         self.b_id = "b-20051208024829-849e76f7968d7a86"
         wt = self.make_branch_and_tree('b1')

=== modified file 'bzrlib/tests/per_workingtree/test_set_root_id.py'
--- bzrlib/tests/per_workingtree/test_set_root_id.py	2009-07-10 07:14:02 +0000
+++ bzrlib/tests/per_workingtree/test_set_root_id.py	2009-08-27 08:35:10 +0000
@@ -23,6 +23,8 @@
 class TestSetRootId(TestCaseWithWorkingTree):
 
     def test_set_and_read_unicode(self):
+        # This test tests that setting the root doesn't flush, so it
+        # deliberately tests concurrent access that isn't possible on windows.
         tree = self.make_branch_and_tree('a-tree')
         # setting the root id allows it to be read via get_root_id.
         root_id = u'\xe5n-id'.encode('utf8')

=== modified file 'bzrlib/tests/test_merge.py'
--- bzrlib/tests/test_merge.py	2009-04-29 17:02:36 +0000
+++ bzrlib/tests/test_merge.py	2009-08-27 08:35:10 +0000
@@ -218,13 +218,15 @@
         tree_a.add('file')
         tree_a.commit('commit base')
         # basis_tree() is only guaranteed to be valid as long as it is actually
-        # the basis tree. This mutates the tree after grabbing basis, so go to
-        # the repository.
+        # the basis tree. This test commits to the tree after grabbing basis,
+        # so we go to the repository.
         base_tree = tree_a.branch.repository.revision_tree(tree_a.last_revision())
         tree_b = tree_a.bzrdir.sprout('tree_b').open_workingtree()
         self.build_tree_contents([('tree_a/file', 'content_2')])
         tree_a.commit('commit other')
         other_tree = tree_a.basis_tree()
+        # 'file' is now missing but isn't altered in any commit in b so no
+        # change should be applied.
         os.unlink('tree_b/file')
         merge_inner(tree_b.branch, other_tree, base_tree, this_tree=tree_b)
 
@@ -1232,6 +1234,27 @@
 
 class TestMergerInMemory(TestMergerBase):
 
+    def test_cache_trees_with_revision_ids_None(self):
+        merger = self.make_Merger(self.setup_simple_graph(), 'C-id')
+        original_cache = dict(merger._cached_trees)
+        merger.cache_trees_with_revision_ids([None])
+        self.assertEqual(original_cache, merger._cached_trees)
+
+    def test_cache_trees_with_revision_ids_no_revision_id(self):
+        merger = self.make_Merger(self.setup_simple_graph(), 'C-id')
+        original_cache = dict(merger._cached_trees)
+        tree = self.make_branch_and_memory_tree('tree')
+        merger.cache_trees_with_revision_ids([tree])
+        self.assertEqual(original_cache, merger._cached_trees)
+
+    def test_cache_trees_with_revision_ids_having_revision_id(self):
+        merger = self.make_Merger(self.setup_simple_graph(), 'C-id')
+        original_cache = dict(merger._cached_trees)
+        tree = merger.this_branch.repository.revision_tree('B-id')
+        original_cache['B-id'] = tree
+        merger.cache_trees_with_revision_ids([tree])
+        self.assertEqual(original_cache, merger._cached_trees)
+
     def test_find_base(self):
         merger = self.make_Merger(self.setup_simple_graph(), 'C-id')
         self.assertEqual('A-id', merger.base_rev_id)

=== modified file 'bzrlib/tests/test_shelf.py'
--- bzrlib/tests/test_shelf.py	2009-07-13 17:35:09 +0000
+++ bzrlib/tests/test_shelf.py	2009-08-27 08:35:10 +0000
@@ -46,6 +46,8 @@
         tree.add(['foo'], ['foo-id'])
         tree.commit('foo')
         tree.rename_one('foo', 'bar')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
         self.addCleanup(creator.finalize)
         self.assertEqual([('rename', 'foo-id', 'foo', 'bar')],
@@ -76,6 +78,8 @@
         tree.add(['foo', 'bar', 'foo/baz'], ['foo-id', 'bar-id', 'baz-id'])
         tree.commit('foo')
         tree.rename_one('foo/baz', 'bar/baz')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
         self.addCleanup(creator.finalize)
         self.assertEqual([('rename', 'baz-id', 'foo/baz', 'bar/baz')],
@@ -310,6 +314,8 @@
         tree.add('foo', 'foo-id')
         tree.commit('Added file and directory')
         os.unlink('tree/foo')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
         self.addCleanup(creator.finalize)
         self.assertEqual([('delete file', 'foo-id', 'file', 'foo')],
@@ -325,6 +331,8 @@
         tree.commit('Added file and directory')
         os.unlink('tree/foo')
         os.mkdir('tree/foo')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
         self.addCleanup(creator.finalize)
         self.assertEqual([('change kind', 'foo-id', 'file', 'directory',
@@ -352,6 +360,8 @@
 
     def test_shelve_change_unknown_change(self):
         tree = self.make_branch_and_tree('tree')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
         self.addCleanup(creator.finalize)
         e = self.assertRaises(ValueError, creator.shelve_change, ('unknown',))
@@ -363,6 +373,8 @@
         tree.add('foo', 'foo-id')
         tree.commit('Added file and directory')
         tree.unversion(['foo-id'])
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
         self.addCleanup(creator.finalize)
         self.assertEqual([('delete file', 'foo-id', 'file', 'foo')],
@@ -373,6 +385,8 @@
 
     def test_shelve_serialization(self):
         tree = self.make_branch_and_tree('.')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
         self.addCleanup(creator.finalize)
         shelf_file = open('shelf', 'wb')
@@ -387,6 +401,8 @@
         tree = self.make_branch_and_tree('tree')
         self.build_tree(['tree/foo'])
         tree.add('foo', 'foo-id')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
         self.addCleanup(creator.finalize)
         list(creator.iter_shelvable())
@@ -411,8 +427,12 @@
 
     def test_shelve_unversioned(self):
         tree = self.make_branch_and_tree('tree')
-        self.assertRaises(errors.PathsNotVersionedError,
-                          shelf.ShelfCreator, tree, tree.basis_tree(), ['foo'])
+        tree.lock_tree_write()
+        try:
+            self.assertRaises(errors.PathsNotVersionedError,
+                              shelf.ShelfCreator, tree, tree.basis_tree(), ['foo'])
+        finally:
+            tree.unlock()
         # We should be able to lock/unlock the tree if ShelfCreator cleaned
         # after itself.
         wt = workingtree.WorkingTree.open('tree')
@@ -420,8 +440,15 @@
         wt.unlock()
         # And a second tentative should raise the same error (no
         # limbo/pending_deletion leftovers).
-        self.assertRaises(errors.PathsNotVersionedError,
-                          shelf.ShelfCreator, tree, tree.basis_tree(), ['foo'])
+        tree.lock_tree_write()
+        try:
+            self.assertRaises(errors.PathsNotVersionedError,
+                              shelf.ShelfCreator, tree, tree.basis_tree(), ['foo'])
+        finally:
+            tree.unlock()
+
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
 
 
 class TestUnshelver(tests.TestCaseWithTransport):
@@ -525,6 +552,7 @@
         shelf_file = open('shelf', 'rb')
         self.addCleanup(shelf_file.close)
         unshelver = shelf.Unshelver.from_tree_and_shelf(tree, shelf_file)
+        unshelver.finalize()
 
     def test_corrupt_shelf(self):
         tree = self.make_branch_and_tree('.')
@@ -644,7 +672,10 @@
 
     def test_get_metadata(self):
         tree = self.make_branch_and_tree('.')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         creator = shelf.ShelfCreator(tree, tree.basis_tree())
+        self.addCleanup(creator.finalize)
         shelf_manager = tree.get_shelf_manager()
         shelf_id = shelf_manager.shelve_changes(creator, 'foo')
         metadata = shelf_manager.get_metadata(shelf_id)

=== modified file 'bzrlib/tests/test_shelf_ui.py'
--- bzrlib/tests/test_shelf_ui.py	2009-07-14 13:19:52 +0000
+++ bzrlib/tests/test_shelf_ui.py	2009-08-27 08:35:10 +0000
@@ -68,12 +68,16 @@
 
     def test_unexpected_prompt_failure(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         e = self.assertRaises(AssertionError, shelver.run)
         self.assertEqual('Unexpected prompt: Shelve? [yNfq?]', str(e))
 
     def test_wrong_prompt_failure(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('foo', 'y')
         e = self.assertRaises(AssertionError, shelver.run)
@@ -81,6 +85,8 @@
 
     def test_shelve_not_diff(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve? [yNfq?]', 'n')
         shelver.expect('Shelve? [yNfq?]', 'n')
@@ -90,6 +96,8 @@
 
     def test_shelve_diff_no(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve? [yNfq?]', 'y')
         shelver.expect('Shelve? [yNfq?]', 'y')
@@ -99,6 +107,8 @@
 
     def test_shelve_diff(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve? [yNfq?]', 'y')
         shelver.expect('Shelve? [yNfq?]', 'y')
@@ -108,6 +118,8 @@
 
     def test_shelve_one_diff(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve? [yNfq?]', 'y')
         shelver.expect('Shelve? [yNfq?]', 'n')
@@ -118,6 +130,8 @@
     def test_shelve_binary_change(self):
         tree = self.create_shelvable_tree()
         self.build_tree_contents([('tree/foo', '\x00')])
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve binary changes? [yNfq?]', 'y')
         shelver.expect('Shelve 1 change(s)? [yNfq?]', 'y')
@@ -127,6 +141,8 @@
     def test_shelve_rename(self):
         tree = self.create_shelvable_tree()
         tree.rename_one('foo', 'bar')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve renaming "foo" => "bar"? [yNfq?]', 'y')
         shelver.expect('Shelve? [yNfq?]', 'y')
@@ -138,6 +154,8 @@
     def test_shelve_deletion(self):
         tree = self.create_shelvable_tree()
         os.unlink('tree/foo')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve removing file "foo"? [yNfq?]', 'y')
         shelver.expect('Shelve 1 change(s)? [yNfq?]', 'y')
@@ -149,6 +167,8 @@
         tree.commit('add tree root')
         self.build_tree(['tree/foo'])
         tree.add('foo')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve adding file "foo"? [yNfq?]', 'y')
         shelver.expect('Shelve 1 change(s)? [yNfq?]', 'y')
@@ -159,6 +179,8 @@
         tree = self.create_shelvable_tree()
         os.unlink('tree/foo')
         os.mkdir('tree/foo')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve changing "foo" from file to directory? [yNfq?]',
                        'y')
@@ -172,6 +194,8 @@
         tree.commit("Add symlink")
         os.unlink('tree/baz')
         os.symlink('vax', 'tree/baz')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve changing target of "baz" from "bar" to '
                 '"vax"? [yNfq?]', 'y')
@@ -181,6 +205,8 @@
 
     def test_shelve_finish(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve? [yNfq?]', 'f')
         shelver.expect('Shelve 2 change(s)? [yNfq?]', 'y')
@@ -189,6 +215,8 @@
 
     def test_shelve_quit(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve? [yNfq?]', 'q')
         self.assertRaises(errors.UserAbort, shelver.run)
@@ -196,13 +224,20 @@
 
     def test_shelve_all(self):
         tree = self.create_shelvable_tree()
-        ExpectShelver.from_args(sys.stdout, all=True, directory='tree').run()
+        shelver = ExpectShelver.from_args(sys.stdout, all=True,
+            directory='tree')
+        try:
+            shelver.run()
+        finally:
+            shelver.work_tree.unlock()
         self.assertFileEqual(LINES_AJ, 'tree/foo')
 
     def test_shelve_filename(self):
         tree = self.create_shelvable_tree()
         self.build_tree(['tree/bar'])
         tree.add('bar')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(), file_list=['bar'])
         shelver.expect('Shelve adding file "bar"? [yNfq?]', 'y')
         shelver.expect('Shelve 1 change(s)? [yNfq?]', 'y')
@@ -210,6 +245,8 @@
 
     def test_shelve_help(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree())
         shelver.expect('Shelve? [yNfq?]', '?')
         shelver.expect('Shelve? [(y)es, (N)o, (f)inish, or (q)uit]', 'f')
@@ -220,7 +257,10 @@
         tree = self.create_shelvable_tree()
         shelver = shelf_ui.Shelver.from_args(sys.stdout, all=True,
                                              directory='tree', destroy=True)
-        shelver.run()
+        try:
+            shelver.run()
+        finally:
+            shelver.work_tree.unlock()
         self.assertIs(None, tree.get_shelf_manager().last_shelf())
         self.assertFileEqual(LINES_AJ, 'tree/foo')
 
@@ -229,6 +269,8 @@
 
     def test_shelve_not_diff(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                 reporter=shelf_ui.ApplyReporter())
         shelver.expect('Apply change? [yNfq?]', 'n')
@@ -239,6 +281,8 @@
 
     def test_shelve_diff_no(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                 reporter=shelf_ui.ApplyReporter())
         shelver.expect('Apply change? [yNfq?]', 'y')
@@ -249,6 +293,8 @@
 
     def test_shelve_diff(self):
         tree = self.create_shelvable_tree()
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                 reporter=shelf_ui.ApplyReporter())
         shelver.expect('Apply change? [yNfq?]', 'y')
@@ -260,6 +306,8 @@
     def test_shelve_binary_change(self):
         tree = self.create_shelvable_tree()
         self.build_tree_contents([('tree/foo', '\x00')])
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                 reporter=shelf_ui.ApplyReporter())
         shelver.expect('Apply binary changes? [yNfq?]', 'y')
@@ -270,6 +318,8 @@
     def test_shelve_rename(self):
         tree = self.create_shelvable_tree()
         tree.rename_one('foo', 'bar')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                 reporter=shelf_ui.ApplyReporter())
         shelver.expect('Rename "bar" => "foo"? [yNfq?]', 'y')
@@ -282,6 +332,8 @@
     def test_shelve_deletion(self):
         tree = self.create_shelvable_tree()
         os.unlink('tree/foo')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                 reporter=shelf_ui.ApplyReporter())
         shelver.expect('Add file "foo"? [yNfq?]', 'y')
@@ -294,6 +346,8 @@
         tree.commit('add tree root')
         self.build_tree(['tree/foo'])
         tree.add('foo')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                 reporter=shelf_ui.ApplyReporter())
         shelver.expect('Delete file "foo"? [yNfq?]', 'y')
@@ -305,6 +359,8 @@
         tree = self.create_shelvable_tree()
         os.unlink('tree/foo')
         os.mkdir('tree/foo')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                reporter=shelf_ui.ApplyReporter())
         shelver.expect('Change "foo" from directory to a file? [yNfq?]', 'y')
@@ -318,6 +374,8 @@
         tree.commit("Add symlink")
         os.unlink('tree/baz')
         os.symlink('vax', 'tree/baz')
+        tree.lock_tree_write()
+        self.addCleanup(tree.unlock)
         shelver = ExpectShelver(tree, tree.basis_tree(),
                                 reporter=shelf_ui.ApplyReporter())
         shelver.expect('Change target of "baz" from "vax" to "bar"? [yNfq?]',
@@ -331,12 +389,16 @@
 
     def create_tree_with_shelf(self):
         tree = self.make_branch_and_tree('tree')
-        self.build_tree_contents([('tree/foo', LINES_AJ)])
-        tree.add('foo', 'foo-id')
-        tree.commit('added foo')
-        self.build_tree_contents([('tree/foo', LINES_ZY)])
-        shelf_ui.Shelver(tree, tree.basis_tree(), auto_apply=True,
-                         auto=True).run()
+        tree.lock_write()
+        try:
+            self.build_tree_contents([('tree/foo', LINES_AJ)])
+            tree.add('foo', 'foo-id')
+            tree.commit('added foo')
+            self.build_tree_contents([('tree/foo', LINES_ZY)])
+            shelf_ui.Shelver(tree, tree.basis_tree(), auto_apply=True,
+                             auto=True).run()
+        finally:
+            tree.unlock()
         return tree
 
     def test_unshelve(self):
@@ -349,13 +411,22 @@
 
     def test_unshelve_args(self):
         tree = self.create_tree_with_shelf()
-        shelf_ui.Unshelver.from_args(directory='tree').run()
+        unshelver = shelf_ui.Unshelver.from_args(directory='tree')
+        try:
+            unshelver.run()
+        finally:
+            unshelver.tree.unlock()
         self.assertFileEqual(LINES_ZY, 'tree/foo')
         self.assertIs(None, tree.get_shelf_manager().last_shelf())
 
     def test_unshelve_args_dry_run(self):
         tree = self.create_tree_with_shelf()
-        shelf_ui.Unshelver.from_args(directory='tree', action='dry-run').run()
+        unshelver = shelf_ui.Unshelver.from_args(directory='tree',
+            action='dry-run')
+        try:
+            unshelver.run()
+        finally:
+            unshelver.tree.unlock()
         self.assertFileEqual(LINES_AJ, 'tree/foo')
         self.assertEqual(1, tree.get_shelf_manager().last_shelf())
 
@@ -369,7 +440,10 @@
             shelf_file.close()
         unshelver = shelf_ui.Unshelver.from_args(directory='tree',
                                                  action='delete-only')
-        unshelver.run()
+        try:
+            unshelver.run()
+        finally:
+            unshelver.tree.unlock()
         self.assertIs(None, manager.last_shelf())
 
     def test_unshelve_args_invalid_shelf_id(self):