[patch] change WorkingTree to use LockDir

Martin Pool mbp at sourcefrog.net
Thu Mar 9 03:28:01 GMT 2006


This puts responsibility for building the lock and control_files into
the WorkingTreeFormat, which passes it to the WorkingTree.  In
particular, is my use of the previously present but unused
_control_files parameter to the WorkingTree initializer OK?

-- 
Martin
-------------- next part --------------
=== modified file 'a/bzrlib/tests/test_lockdir.py'
--- a/bzrlib/tests/test_lockdir.py	
+++ b/bzrlib/tests/test_lockdir.py	
@@ -170,7 +170,8 @@
     def test_32_lock_wait_succeed(self):
         """Succeed when trying to acquire a lock that gets released
 
-        One thread holds on a lock and then releases it; another tries to lock it.
+        One thread holds on a lock and then releases it; another 
+        tries to lock it.
         """
         t = self.get_transport()
         lf1 = LockDir(t, 'test_lock')

=== modified file 'a/bzrlib/tests/test_workingtree.py'
--- a/bzrlib/tests/test_workingtree.py	
+++ b/bzrlib/tests/test_workingtree.py	
@@ -1,4 +1,4 @@
-# (C) 2005,2006 Canonical Ltd
+# Copyright (C) 2005, 2006 Canonical Ltd
 # Authors:  Robert Collins <robert.collins at canonical.com>
 #
 # This program is free software; you can redistribute it and/or modify
@@ -24,6 +24,7 @@
 from bzrlib.bzrdir import BzrDir
 import bzrlib.errors as errors
 from bzrlib.errors import NotBranchError, NotVersionedError
+from bzrlib.lockdir import LockDir
 from bzrlib.osutils import pathjoin, getcwd, has_symlinks
 from bzrlib.tests import TestCaseWithTransport
 from bzrlib.trace import mutter
@@ -160,7 +161,6 @@
         tree = workingtree.WorkingTreeFormat3().initialize(control)
         # we want:
         # format 'Bazaar-NG Working Tree format 3'
-        # lock ''
         # inventory = blank inventory
         # pending-merges = ''
         # stat-cache = ??
@@ -168,7 +168,6 @@
         t = control.get_workingtree_transport(None)
         self.assertEqualDiff('Bazaar-NG Working Tree format 3',
                              t.get('format').read())
-        self.assertEqualDiff('', t.get('lock').read())
         self.assertEqualDiff('<inventory format="5">\n'
                              '</inventory>\n',
                              t.get('inventory').read())
@@ -179,3 +178,25 @@
         self.assertFalse(t.has('last-revision'))
         # TODO RBC 20060210 do a commit, check the inventory.basis is created 
         # correctly and last-revision file becomes present.
+
+    def test_uses_lockdir(self):
+        """WorkingTreeFormat3 uses its own LockDir:
+            
+            - lock is a directory
+            - when the WorkingTree is locked, LockDir can see that
+        """
+        t = self.get_transport()
+        url = self.get_url()
+        dir = bzrdir.BzrDirMetaFormat1().initialize(url)
+        repo = dir.create_repository()
+        branch = dir.create_branch()
+        tree = workingtree.WorkingTreeFormat3().initialize(dir)
+        self.assertIsDirectory('.bzr', t)
+        self.assertIsDirectory('.bzr/checkout', t)
+        self.assertIsDirectory('.bzr/checkout/lock', t)
+        our_lock = LockDir(t, '.bzr/checkout/lock')
+        self.assertEquals(our_lock.peek(), None)
+        tree.lock_write()
+        self.assertTrue(our_lock.peek())
+        tree.unlock()
+        self.assertEquals(our_lock.peek(), None)

=== modified file 'a/bzrlib/workingtree.py'
--- a/bzrlib/workingtree.py	
+++ b/bzrlib/workingtree.py	
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 Canonical Ltd
+# Copyright (C) 2005, 2006 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
@@ -29,16 +29,12 @@
 WorkingTree.open(dir).
 """
 
-
-# FIXME: I don't know if writing out the cache from the destructor is really a
-# good idea, because destructors are considered poor taste in Python, and it's
-# not predictable when it will be written out.
-
 # TODO: Give the workingtree sole responsibility for the working inventory;
 # remove the variable and references to it from the branch.  This may require
 # updating the commit code so as to update the inventory within the working
 # copy, and making sure there's only one WorkingTree for any directory on disk.
-# At the momenthey may alias the inventory and have old copies of it in memory.
+# At the moment they may alias the inventory and have old copies of it in
+# memory.  (Now done? -- mbp 20060309)
 
 from copy import deepcopy
 from cStringIO import StringIO
@@ -63,6 +59,7 @@
                            NotVersionedError)
 from bzrlib.inventory import InventoryEntry, Inventory
 from bzrlib.lockable_files import LockableFiles, TransportLock
+from bzrlib.lockdir import LockDir
 from bzrlib.merge import merge_inner, transform_tree
 from bzrlib.osutils import (
                             abspath,
@@ -247,16 +244,13 @@
         if isinstance(self._format, WorkingTreeFormat2):
             # share control object
             self._control_files = self.branch.control_files
-        elif _control_files is not None:
-            assert False, "not done yet"
-#            self._control_files = _control_files
         else:
             # only ready for format 3
             assert isinstance(self._format, WorkingTreeFormat3)
-            self._control_files = LockableFiles(
-                self.bzrdir.get_workingtree_transport(None),
-                'lock', TransportLock)
-
+            assert isinstance(_control_files, LockableFiles), \
+                    "_control_files must be a LockableFiles, not %r" \
+                    % _control_files
+            self._control_files = _control_files
         # update the whole cache up front and write to disk if anything changed;
         # in the future we might want to do this more selectively
         # two possible ways offer themselves : in self._unlock, write the cache
@@ -1234,6 +1228,8 @@
     This differs from the base WorkingTree by:
      - having its own file lock
      - having its own last-revision property.
+
+    This is new in bzr 0.8
     """
 
     @needs_read_lock
@@ -1413,12 +1409,26 @@
 class WorkingTreeFormat3(WorkingTreeFormat):
     """The second working tree format updated to record a format marker.
 
-    This format modified the hash cache from the format 1 hash cache.
+    This format:
+        - exists within a metadir controlling .bzr
+        - includes an explicit version marker for the workingtree control
+          files, separate from the BzrDir format
+        - modifies the hash cache format
+        - is new in bzr 0.8
+        - uses a LockDir to guard access to the repository
     """
 
     def get_format_string(self):
         """See WorkingTreeFormat.get_format_string()."""
         return "Bazaar-NG Working Tree format 3"
+
+    _lock_file_name = 'lock'
+    _lock_class = LockDir
+
+    def _open_control_files(self, a_bzrdir):
+        transport = a_bzrdir.get_workingtree_transport(None)
+        return LockableFiles(transport, self._lock_file_name, 
+                             self._lock_class)
 
     def initialize(self, a_bzrdir, revision_id=None):
         """See WorkingTreeFormat.initialize().
@@ -1429,7 +1439,8 @@
         if not isinstance(a_bzrdir.transport, LocalTransport):
             raise errors.NotLocalUrl(a_bzrdir.transport.base)
         transport = a_bzrdir.get_workingtree_transport(self)
-        control_files = LockableFiles(transport, 'lock', TransportLock)
+        control_files = self._open_control_files(a_bzrdir)
+        control_files.create_lock()
         control_files.put_utf8('format', self.get_format_string())
         branch = a_bzrdir.open_branch()
         if revision_id is None:
@@ -1440,7 +1451,8 @@
                          inv,
                          _internal=True,
                          _format=self,
-                         _bzrdir=a_bzrdir)
+                         _bzrdir=a_bzrdir,
+                         _control_files=control_files)
         wt._write_inventory(inv)
         wt.set_root_id(inv.root.file_id)
         wt.set_last_revision(revision_id)
@@ -1463,10 +1475,12 @@
             raise NotImplementedError
         if not isinstance(a_bzrdir.transport, LocalTransport):
             raise errors.NotLocalUrl(a_bzrdir.transport.base)
+        control_files = self._open_control_files(a_bzrdir)
         return WorkingTree3(a_bzrdir.root_transport.base,
                            _internal=True,
                            _format=self,
-                           _bzrdir=a_bzrdir)
+                           _bzrdir=a_bzrdir,
+                           _control_files=control_files)
 
     def __str__(self):
         return self.get_format_string()



More information about the bazaar mailing list