Rev 4722: Hook up IndirectedDirState to --development-rich-root. in http://bazaar.launchpad.net/~lifeless/bzr/dirstate2

Robert Collins robertc at robertcollins.net
Tue Sep 29 06:21:43 BST 2009


At http://bazaar.launchpad.net/~lifeless/bzr/dirstate2

------------------------------------------------------------
revno: 4722
revision-id: robertc at robertcollins.net-20090929052138-u2po4lj409sohiqa
parent: robertc at robertcollins.net-20090929040224-ofiqnl04w3e8v1i2
committer: Robert Collins <robertc at robertcollins.net>
branch nick: dirstate2
timestamp: Tue 2009-09-29 15:21:38 +1000
message:
  Hook up IndirectedDirState to --development-rich-root.
=== modified file 'NEWS'
--- a/NEWS	2009-09-29 04:02:24 +0000
+++ b/NEWS	2009-09-29 05:21:38 +0000
@@ -115,6 +115,18 @@
   See also <https://answers.launchpad.net/bzr/+faq/703>.
   (Martin Pool, #406113, #430529)
 
+* The ``development-rich-root`` format now includes a new WorkingTree
+  that allows readonly operations like ``bzr diff`` while a writing
+  operation such as ``commit`` is taking place on the tree. This no longer
+  uses operating system file locks to arbitrate access and is thus
+  insulated from various operating system issues. It also uses separate
+  files for each update, so slow readers can complete in their own time,
+  and old state is only removed once all the relevant files have been
+  handed off to the file system, making it more robust to bzr being killed
+  or interrupted.
+  (Robert Collins, #98836, #108605, #114528, #131111, #174055, #206406,
+   #379582)
+
 Documentation
 *************
 

=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py	2009-09-24 09:45:23 +0000
+++ b/bzrlib/bzrdir.py	2009-09-29 05:21:38 +0000
@@ -3827,7 +3827,7 @@
     )
 # The following un-numbered 'development' formats should always just be aliases.
 format_registry.register_metadir('development-rich-root',
-    'bzrlib.repofmt.groupcompress_repo.RepositoryFormatCHK1',
+    'bzrlib.repofmt.groupcompress_repo.RepositoryFormat2a',
     help='Current development format. Supports rich roots. Can convert data '
         'to and from rich-root-pack (and anything compatible with '
         'rich-root-pack) format repositories. Repositories and branches in '
@@ -3835,7 +3835,7 @@
         'http://doc.bazaar-vcs.org/latest/developers/development-repo.html '
         'before use.',
     branch_format='bzrlib.branch.BzrBranchFormat7',
-    tree_format='bzrlib.workingtree.WorkingTreeFormat6',
+    tree_format='bzrlib.workingtree.WorkingTreeFormat7',
     experimental=True,
     alias=True,
     )
@@ -3856,26 +3856,14 @@
     )
 
 # And the development formats above will have aliased one of the following:
-format_registry.register_metadir('development6-rich-root',
-    'bzrlib.repofmt.groupcompress_repo.RepositoryFormatCHK1',
-    help='pack-1.9 with 255-way hashed CHK inv, group compress, rich roots '
-        'Please read '
-        'http://doc.bazaar-vcs.org/latest/developers/development-repo.html '
-        'before use.',
-    branch_format='bzrlib.branch.BzrBranchFormat7',
-    tree_format='bzrlib.workingtree.WorkingTreeFormat6',
-    hidden=True,
-    experimental=True,
-    )
-
 format_registry.register_metadir('development7-rich-root',
-    'bzrlib.repofmt.groupcompress_repo.RepositoryFormatCHK2',
+    'bzrlib.repofmt.groupcompress_repo.RepositoryFormat2a',
     help='pack-1.9 with 255-way hashed CHK inv, bencode revision, group compress, '
         'rich roots. Please read '
         'http://doc.bazaar-vcs.org/latest/developers/development-repo.html '
         'before use.',
     branch_format='bzrlib.branch.BzrBranchFormat7',
-    tree_format='bzrlib.workingtree.WorkingTreeFormat6',
+    tree_format='bzrlib.workingtree.WorkingTreeFormat7',
     hidden=True,
     experimental=True,
     )

=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2009-09-29 04:02:24 +0000
+++ b/bzrlib/dirstate.py	2009-09-29 05:21:38 +0000
@@ -3323,10 +3323,21 @@
     def _save(self):
         bytes = ''.join(self.get_lines())
         name = osutils.md5(bytes).hexdigest()
+        if name == self._last_pointer:
+            # No-change has occured at all.
+            return True
         name_check = name + '.check'
         current_new = name + '.current'
         name_delete = name + '.delete'
-        self._transport.put_bytes_non_atomic(name, bytes)
+        try:
+            self._transport.put_bytes_non_atomic(name, bytes)
+        except errors.PermissionDenied:
+            if self._lock_state == 'r':
+                # Read only file system, or lacking permissions - only a stat
+                # cache change - ignore it.
+                return True
+            else:
+                raise
         self._transport.put_bytes_non_atomic(current_new, name + '\n')
         # TODO spin on this in 'w' locks, for concurrency with stat cache
         # updates.
@@ -3360,6 +3371,7 @@
             self._transport.put_bytes_non_atomic('format',
                 IndirectedDirState.DIRFORMAT)
         self._state_file = self._transport.get(name)
+        self._last_pointer = name
         
     def _set_try_state_file(self):
         pointer = None
@@ -3391,6 +3403,7 @@
 
     def _unlock(self):
         self._state_file = None
+        self._last_pointer = None
         # XXX: TODO garbage collect state files on disk.
 
     def _upgrade_physical_lock(self):

=== modified file 'bzrlib/tests/test_dirstate.py'
--- a/bzrlib/tests/test_dirstate.py	2009-09-29 04:02:24 +0000
+++ b/bzrlib/tests/test_dirstate.py	2009-09-29 05:21:38 +0000
@@ -590,27 +590,7 @@
         state = self._dirstate_class.on_file('dirstate')
         state.lock_read()
         try:
-            entry = state._get_entry(0, path_utf8='a-file')
-            # The current size should be 0 (default)
-            self.assertEqual(0, entry[1][0][2])
-            # We should have a real entry.
-            self.assertNotEqual((None, None), entry)
-            # Make sure everything is old enough
-            state._sha_cutoff_time()
-            state._cutoff_time += 10
-            # Change the file length
-            self.build_tree_contents([('a-file', 'shorter')])
-            sha1sum = dirstate.update_entry(state, entry, 'a-file',
-                os.lstat('a-file'))
-            # new file, no cached sha:
-            self.assertEqual(None, sha1sum)
-
-            # The dirblock has been updated
-            self.assertEqual(7, entry[1][0][2])
-            self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
-                             state._dirblock_state)
-
-            del entry
+            self.update_entry('a-file', state)
             # Now, since we are the only one holding a lock, we should be able
             # to save and have it written to disk
             state.save()
@@ -626,6 +606,49 @@
         finally:
             state.unlock()
 
+    def test_save_in_read_lock_readonly_no_error(self):
+        self.build_tree(['a-file'])
+        state = self._dirstate_class.initialize('dirstate')
+        try:
+            # No stat and no sha1 sum.
+            state.add('a-file', 'a-file-id', 'file', None, '')
+            state.save()
+        finally:
+            state.unlock()
+        self.addCleanup(os.chmod, 'dirstate', 0755)
+        os.chmod('dirstate', 0555)
+        # Now open in readonly mode
+        state = self._dirstate_class.on_file('dirstate')
+        state.lock_read()
+        try:
+            self.update_entry('a-file', state)
+            state.save()
+        finally:
+            state.unlock()
+
+    def update_entry(self, name_utf8, state):
+        entry = state._get_entry(0, path_utf8=name_utf8)
+        # The current size should be 0 (default)
+        self.assertEqual(0, entry[1][0][2])
+        # We should have a real entry.
+        self.assertNotEqual((None, None), entry)
+        # Make sure everything is old enough
+        state._sha_cutoff_time()
+        state._cutoff_time += 10
+        # Change the file length
+        self.build_tree_contents([(name_utf8, 'shorter')])
+        sha1sum = dirstate.update_entry(state, entry, name_utf8,
+            os.lstat(name_utf8))
+        # new file, no cached sha:
+        self.assertEqual(None, sha1sum)
+
+        # The dirblock has been updated
+        self.assertEqual(7, entry[1][0][2])
+        self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
+                         state._dirblock_state)
+
+        del entry
+
     def test_save_from_read_lock_preserves_concurrent_changes(self):
         """If dirstate is locked, save will fail without complaining."""
         self.build_tree(['a-file', 'b-file'])

=== modified file 'bzrlib/tests/test_workingtree_4.py'
--- a/bzrlib/tests/test_workingtree_4.py	2009-08-28 05:00:33 +0000
+++ b/bzrlib/tests/test_workingtree_4.py	2009-09-29 05:21:38 +0000
@@ -666,6 +666,28 @@
         self.assertEqual(["contents of foo\n"], file_obj.readlines())
 
 
+class TestWorkingTreeFormat7(TestCaseWithTransport):
+    """Tests specific to WorkingTreeFormat7."""
+
+    def test_disk_layout(self):
+        control = bzrdir.BzrDirMetaFormat1().initialize(self.get_url())
+        control.create_repository()
+        control.create_branch()
+        tree = workingtree_4.WorkingTreeFormat7().initialize(control)
+        # we want:
+        # format 'Bazaar Working Tree format 7'
+        t = control.get_workingtree_transport(None)
+        self.assertEqualDiff('Bazaar Working Tree Format 7 (bzr 2.1)\n',
+                             t.get('format').read())
+        self.assertFalse(t.has('inventory.basis'))
+        state = dirstate.IndirectedDirState.on_file(
+            t.local_abspath('dirstate'))
+        state.lock_read()
+        try:
+            self.assertEqual([], state.get_parent_ids())
+        finally:
+            state.unlock()
+
 class TestCorruptDirstate(TestCaseWithTransport):
     """Tests for how we handle when the dirstate has been corrupted."""
 

=== modified file 'bzrlib/workingtree.py'
--- a/bzrlib/workingtree.py	2009-08-26 05:38:16 +0000
+++ b/bzrlib/workingtree.py	2009-09-29 05:21:38 +0000
@@ -79,6 +79,7 @@
     WorkingTreeFormat4,
     WorkingTreeFormat5,
     WorkingTreeFormat6,
+    WorkingTreeFormat7,
     )
 """)
 
@@ -3037,6 +3038,7 @@
 
 __default_format = WorkingTreeFormat6()
 WorkingTreeFormat.register_format(__default_format)
+WorkingTreeFormat.register_format(WorkingTreeFormat7())
 WorkingTreeFormat.register_format(WorkingTreeFormat5())
 WorkingTreeFormat.register_format(WorkingTreeFormat4())
 WorkingTreeFormat.register_format(WorkingTreeFormat3())

=== modified file 'bzrlib/workingtree_4.py'
--- a/bzrlib/workingtree_4.py	2009-08-25 04:43:21 +0000
+++ b/bzrlib/workingtree_4.py	2009-09-29 05:21:38 +0000
@@ -74,16 +74,21 @@
                  branch,
                  _control_files=None,
                  _format=None,
-                 _bzrdir=None):
+                 _bzrdir=None,
+                 _dirstate_class=None):
         """Construct a WorkingTree for basedir.
 
         If the branch is not supplied, it is opened automatically.
         If the branch is supplied, it must be the branch for this basedir.
         (branch.base is not cross checked, because for remote branches that
         would be meaningless).
+
+        :ivar _dirstate_class: The class of 'DirState' to be used for this
+            tree.
         """
         self._format = _format
         self.bzrdir = _bzrdir
+        self._dirstate_class = _dirstate_class
         basedir = safe_unicode(basedir)
         mutter("opening working tree %r", basedir)
         self._branch = branch
@@ -159,7 +164,7 @@
             else:
                 clear = False
             state = self._current_dirstate()
-            if state._lock_token is not None:
+            if state._lock_state is not None:
                 # we already have it locked. sheese, cant break our own lock.
                 raise errors.LockActive(self.basedir)
             else:
@@ -218,7 +223,7 @@
             return self._dirstate
         local_path = self.bzrdir.get_workingtree_transport(None
             ).local_abspath('dirstate')
-        self._dirstate = dirstate.DirState.on_file(local_path,
+        self._dirstate = self._dirstate_class.on_file(local_path,
             self._sha1_provider())
         return self._dirstate
 
@@ -574,7 +579,7 @@
             self._control_files.lock_read()
             try:
                 state = self.current_dirstate()
-                if not state._lock_token:
+                if not state._lock_state:
                     state.lock_read()
                 # set our support for tree references from the repository in
                 # use.
@@ -594,7 +599,7 @@
             self._control_files.lock_write()
             try:
                 state = self.current_dirstate()
-                if not state._lock_token:
+                if not state._lock_state:
                     state.lock_write()
                 # set our support for tree references from the repository in
                 # use.
@@ -1364,6 +1369,10 @@
 
 
 class DirStateWorkingTreeFormat(WorkingTreeFormat3):
+
+    # what dirstate class should be used for a given format.
+    _dirstate_class = dirstate.DirState
+
     def initialize(self, a_bzrdir, revision_id=None, from_branch=None,
                    accelerator_tree=None, hardlink=False):
         """See WorkingTreeFormat.initialize().
@@ -1396,14 +1405,15 @@
             revision_id = branch.last_revision()
         local_path = transport.local_abspath('dirstate')
         # write out new dirstate (must exist when we create the tree)
-        state = dirstate.DirState.initialize(local_path)
+        state = self._dirstate_class.initialize(local_path)
         state.unlock()
         del state
         wt = self._tree_class(a_bzrdir.root_transport.local_abspath('.'),
                          branch,
                          _format=self,
                          _bzrdir=a_bzrdir,
-                         _control_files=control_files)
+                         _control_files=control_files,
+                         _dirstate_class=self._dirstate_class)
         wt._new_tree()
         wt.lock_tree_write()
         try:
@@ -1489,7 +1499,8 @@
                            branch=a_bzrdir.open_branch(),
                            _format=self,
                            _bzrdir=a_bzrdir,
-                           _control_files=control_files)
+                           _control_files=control_files,
+                           _dirstate_class=self._dirstate_class)
 
     def __get_matchingbzrdir(self):
         return self._get_matchingbzrdir()
@@ -1548,21 +1559,24 @@
         return True
 
 
-class WorkingTreeFormat6(DirStateWorkingTreeFormat):
-    """WorkingTree format supporting views.
+class WorkingTreeFormat7(DirStateWorkingTreeFormat):
+    """WorkingTree with:
+        - views
+        - IndirectedDirState
     """
 
+    _dirstate_class = dirstate.IndirectedDirState
     upgrade_recommended = False
 
     _tree_class = WorkingTree6
 
     def get_format_string(self):
         """See WorkingTreeFormat.get_format_string()."""
-        return "Bazaar Working Tree Format 6 (bzr 1.14)\n"
+        return "Bazaar Working Tree Format 7 (bzr 2.1)\n"
 
     def get_format_description(self):
         """See WorkingTreeFormat.get_format_description()."""
-        return "Working tree format 6"
+        return "Working tree format 7"
 
     def _init_custom_control_files(self, wt):
         """Subclasses with custom control files should override this method."""
@@ -1575,6 +1589,24 @@
         return True
 
 
+class WorkingTreeFormat6(WorkingTreeFormat7):
+    """WorkingTree with:
+        - views
+        - single file dirstate (OSLocks needed)
+    """
+
+    _dirstate_class = dirstate.DirState
+
+    def get_format_string(self):
+        """See WorkingTreeFormat.get_format_string()."""
+        return "Bazaar Working Tree Format 6 (bzr 1.14)\n"
+
+    def get_format_description(self):
+        """See WorkingTreeFormat.get_format_description()."""
+        return "Working tree format 6"
+
+
+
 class DirStateRevisionTree(Tree):
     """A revision tree pulling the inventory from a dirstate.
     
@@ -1878,7 +1910,7 @@
         """Lock the tree for a set of operations."""
         if not self._locked:
             self._repository.lock_read()
-            if self._dirstate._lock_token is None:
+            if self._dirstate._lock_state is None:
                 self._dirstate.lock_read()
                 self._dirstate_locked = True
         self._locked += 1
@@ -2136,7 +2168,7 @@
         """Create the dirstate based data for tree."""
         local_path = tree.bzrdir.get_workingtree_transport(None
             ).local_abspath('dirstate')
-        state = dirstate.DirState.from_tree(tree, local_path)
+        state = self._dirstate_class.from_tree(tree, local_path)
         state.save()
         state.unlock()
 




More information about the bazaar-commits mailing list