Rev 2499: Update the code so that symlinks aren't cached at incorrect times in http://bzr.arbash-meinel.com/branches/bzr/0.17-dev/dirstate_optim_2

John Arbash Meinel john at arbash-meinel.com
Thu May 31 21:29:16 BST 2007


At http://bzr.arbash-meinel.com/branches/bzr/0.17-dev/dirstate_optim_2

------------------------------------------------------------
revno: 2499
revision-id: john at arbash-meinel.com-20070531202904-34h7ygudo7qq9ha1
parent: john at arbash-meinel.com-20070531141758-i5f7gqddjm497ij8
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: dirstate_optim_2
timestamp: Thu 2007-05-31 15:29:04 -0500
message:
  Update the code so that symlinks aren't cached at incorrect times
  and fix the tests so that they don't assume files and symlinks
  get cached even when the timestamp doesn't declare them 'safe'.
modified:
  bzrlib/dirstate.py             dirstate.py-20060728012006-d6mvoihjb3je9peu-1
  bzrlib/tests/test_dirstate.py  test_dirstate.py-20060728012006-d6mvoihjb3je9peu-2
-------------- next part --------------
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2007-05-17 18:22:43 +0000
+++ b/bzrlib/dirstate.py	2007-05-31 20:29:04 +0000
@@ -1149,8 +1149,15 @@
                                    osutils.pathjoin(entry[0][0], entry[0][1]))
         elif minikind == 'l':
             link_or_sha1 = self._read_link(abspath, saved_link_or_sha1)
-            entry[1][0] = ('l', link_or_sha1, stat_value.st_size,
-                           False, packed_stat)
+            if self._cutoff_time is None:
+                self._sha_cutoff_time()
+            if (stat_value.st_mtime < self._cutoff_time
+                and stat_value.st_ctime < self._cutoff_time):
+                entry[1][0] = ('l', link_or_sha1, stat_value.st_size,
+                               False, packed_stat)
+            else:
+                entry[1][0] = ('l', '', stat_value.st_size,
+                               False, DirState.NULLSTAT)
         self._dirblock_state = DirState.IN_MEMORY_MODIFIED
         return link_or_sha1
 

=== modified file 'bzrlib/tests/test_dirstate.py'
--- a/bzrlib/tests/test_dirstate.py	2007-05-15 23:27:20 +0000
+++ b/bzrlib/tests/test_dirstate.py	2007-05-31 20:29:04 +0000
@@ -558,6 +558,9 @@
             self.assertEqual('', entry[1][0][1])
             # 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
             sha1sum = state.update_entry(entry, 'a-file', os.lstat('a-file'))
             # We should have gotten a real sha1
             self.assertEqual('ecc5374e9ed82ad3ea3b4d452ea995a5fd3e70e3',
@@ -1421,8 +1424,8 @@
         self.assertEqual('b50e5406bb5e153ebbeb20268fcf37c87e1ecfb6',
                          link_or_sha1)
 
-        # The dirblock entry should be updated with the new info
-        self.assertEqual([('f', link_or_sha1, 14, False, packed_stat)],
+        # The dirblock entry should not cache the file's sha1
+        self.assertEqual([('f', '', 14, False, dirstate.DirState.NULLSTAT)],
                          entry[1])
         self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
                          state._dirblock_state)
@@ -1447,18 +1450,35 @@
                          link_or_sha1)
         self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
                          state._dirblock_state)
+        self.assertEqual([('f', '', 14, False, dirstate.DirState.NULLSTAT)],
+                         entry[1])
         state.save()
 
         # However, if we move the clock forward so the file is considered
-        # "stable", it should just returned the cached value.
-        state.adjust_time(20)
-        link_or_sha1 = state.update_entry(entry, abspath='a',
-                                          stat_value=stat_value)
-        self.assertEqual('b50e5406bb5e153ebbeb20268fcf37c87e1ecfb6',
-                         link_or_sha1)
-        self.assertEqual([('sha1', 'a'), ('is_exec', mode, False),
-                          ('sha1', 'a'), ('is_exec', mode, False),
-                         ], state._log)
+        # "stable", it should just cache the value.
+        state.adjust_time(+20)
+        link_or_sha1 = state.update_entry(entry, abspath='a',
+                                          stat_value=stat_value)
+        self.assertEqual('b50e5406bb5e153ebbeb20268fcf37c87e1ecfb6',
+                         link_or_sha1)
+        self.assertEqual([('sha1', 'a'), ('is_exec', mode, False),
+                          ('sha1', 'a'), ('is_exec', mode, False),
+                          ('sha1', 'a'), ('is_exec', mode, False),
+                         ], state._log)
+        self.assertEqual([('f', link_or_sha1, 14, False, packed_stat)],
+                         entry[1])
+
+        # Subsequent calls will just return the cached value
+        link_or_sha1 = state.update_entry(entry, abspath='a',
+                                          stat_value=stat_value)
+        self.assertEqual('b50e5406bb5e153ebbeb20268fcf37c87e1ecfb6',
+                         link_or_sha1)
+        self.assertEqual([('sha1', 'a'), ('is_exec', mode, False),
+                          ('sha1', 'a'), ('is_exec', mode, False),
+                          ('sha1', 'a'), ('is_exec', mode, False),
+                         ], state._log)
+        self.assertEqual([('f', link_or_sha1, 14, False, packed_stat)],
+                         entry[1])
 
     def test_update_entry_symlink(self):
         """Update entry should read symlinks."""
@@ -1478,8 +1498,8 @@
                                           stat_value=stat_value)
         self.assertEqual('target', link_or_sha1)
         self.assertEqual([('read_link', 'a', '')], state._log)
-        # Dirblock is updated
-        self.assertEqual([('l', link_or_sha1, 6, False, packed_stat)],
+        # Dirblock is not updated (the link is too new)
+        self.assertEqual([('l', '', 6, False, dirstate.DirState.NULLSTAT)],
                          entry[1])
         self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
                          state._dirblock_state)
@@ -1489,18 +1509,32 @@
                                           stat_value=stat_value)
         self.assertEqual('target', link_or_sha1)
         self.assertEqual([('read_link', 'a', ''),
-                          ('read_link', 'a', 'target'),
+                          ('read_link', 'a', ''),
                          ], state._log)
+        self.assertEqual([('l', '', 6, False, dirstate.DirState.NULLSTAT)],
+                         entry[1])
         state.adjust_time(+20) # Skip into the future, all files look old
         link_or_sha1 = state.update_entry(entry, abspath='a',
                                           stat_value=stat_value)
         self.assertEqual('target', link_or_sha1)
-        # There should not be a new read_link call.
-        # (this is a weak assertion, because read_link is fairly inexpensive,
-        # versus the number of symlinks that we would have)
-        self.assertEqual([('read_link', 'a', ''),
-                          ('read_link', 'a', 'target'),
-                         ], state._log)
+        # We need to re-read the link because only now can we cache it
+        self.assertEqual([('read_link', 'a', ''),
+                          ('read_link', 'a', ''),
+                          ('read_link', 'a', ''),
+                         ], state._log)
+        self.assertEqual([('l', 'target', 6, False, packed_stat)],
+                         entry[1])
+
+        # Another call won't re-read the link
+        self.assertEqual([('read_link', 'a', ''),
+                          ('read_link', 'a', ''),
+                          ('read_link', 'a', ''),
+                         ], state._log)
+        link_or_sha1 = state.update_entry(entry, abspath='a',
+                                          stat_value=stat_value)
+        self.assertEqual('target', link_or_sha1)
+        self.assertEqual([('l', 'target', 6, False, packed_stat)],
+                         entry[1])
 
     def do_update_entry(self, state, entry, abspath):
         stat_value = os.lstat(abspath)
@@ -1514,9 +1548,7 @@
     def test_update_entry_dir_unchanged(self):
         state, entry = self.get_state_with_a()
         self.build_tree(['a/'])
-        # make sure the last modified time is in the future
-        state._sha_cutoff_time()
-        state._cutoff_time += 20
+        state.adjust_time(+20)
         self.assertIs(None, self.do_update_entry(state, entry, 'a'))
         self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
                          state._dirblock_state)
@@ -1531,9 +1563,7 @@
         state, entry = self.get_state_with_a()
         self.build_tree(['a'])
         sha1sum = 'b50e5406bb5e153ebbeb20268fcf37c87e1ecfb6'
-        # make sure the last modified time is in the future
-        state._sha_cutoff_time()
-        state._cutoff_time += 20
+        state.adjust_time(+20)
         self.assertEqual(sha1sum, self.do_update_entry(state, entry, 'a'))
         self.assertEqual(dirstate.DirState.IN_MEMORY_MODIFIED,
                          state._dirblock_state)
@@ -1603,6 +1633,8 @@
         We also update the inventory record.
         """
         state, entry = self.get_state_with_a()
+        # The file sha1 won't be cached unless the file is old
+        state.adjust_time(+10)
         self.create_and_test_file(state, entry)
         os.remove('a')
         self.create_and_test_dir(state, entry)
@@ -1613,6 +1645,8 @@
             # PlatformDeficiency / TestSkipped
             raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
+        # The file sha1 won't be cached unless the file is old
+        state.adjust_time(+10)
         self.create_and_test_file(state, entry)
         os.remove('a')
         self.create_and_test_symlink(state, entry)
@@ -1620,6 +1654,8 @@
     def test_update_dir_to_file(self):
         """Directory becoming a file updates the entry."""
         state, entry = self.get_state_with_a()
+        # The file sha1 won't be cached unless the file is old
+        state.adjust_time(+10)
         self.create_and_test_dir(state, entry)
         os.rmdir('a')
         self.create_and_test_file(state, entry)
@@ -1630,6 +1666,8 @@
             # PlatformDeficiency / TestSkipped
             raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
+        # The symlink target won't be cached if it isn't old
+        state.adjust_time(+10)
         self.create_and_test_dir(state, entry)
         os.rmdir('a')
         self.create_and_test_symlink(state, entry)
@@ -1639,6 +1677,8 @@
         if not has_symlinks():
             raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
+        # The symlink and file info won't be cached unless old
+        state.adjust_time(+10)
         self.create_and_test_symlink(state, entry)
         os.remove('a')
         self.create_and_test_file(state, entry)
@@ -1648,6 +1688,8 @@
         if not has_symlinks():
             raise TestSkipped("No symlink support")
         state, entry = self.get_state_with_a()
+        # The symlink target won't be cached if it isn't old
+        state.adjust_time(+10)
         self.create_and_test_symlink(state, entry)
         os.remove('a')
         self.create_and_test_dir(state, entry)
@@ -1667,11 +1709,16 @@
         packed_stat = dirstate.pack_stat(stat_value)
 
         state.adjust_time(-10) # Make sure everything is new
-        # Make sure it wants to kkkkkkkk
         state.update_entry(entry, abspath='a', stat_value=stat_value)
 
         # The row is updated, but the executable bit stays set.
+        self.assertEqual([('f', '', 14, True, dirstate.DirState.NULLSTAT)],
+                         entry[1])
+
+        # Make the disk object look old enough to cache
+        state.adjust_time(+20)
         digest = 'b50e5406bb5e153ebbeb20268fcf37c87e1ecfb6'
+        state.update_entry(entry, abspath='a', stat_value=stat_value)
         self.assertEqual([('f', digest, 14, True, packed_stat)], entry[1])
 
 



More information about the bazaar-commits mailing list