Rev 3721: Review feedback. in http://people.ubuntu.com/~robertc/baz2.0/readdir

Robert Collins robertc at robertcollins.net
Wed Sep 24 04:08:19 BST 2008


At http://people.ubuntu.com/~robertc/baz2.0/readdir

------------------------------------------------------------
revno: 3721
revision-id: robertc at robertcollins.net-20080924030814-67isjttmo6jksr8p
parent: robertc at robertcollins.net-20080924020433-3ppk1orrmddt81r2
committer: Robert Collins <robertc at robertcollins.net>
branch nick: process-entry-optimised
timestamp: Wed 2008-09-24 13:08:14 +1000
message:
  Review feedback.
modified:
  bzrlib/_dirstate_helpers_c.pyx dirstate_helpers.pyx-20070503201057-u425eni465q4idwn-3
  bzrlib/_dirstate_helpers_py.py _dirstate_helpers_py-20070710145033-90nz6cqglsk150jy-1
  bzrlib/dirstate.py             dirstate.py-20060728012006-d6mvoihjb3je9peu-1
  bzrlib/tests/test_dirstate.py  test_dirstate.py-20060728012006-d6mvoihjb3je9peu-2
=== modified file 'bzrlib/_dirstate_helpers_c.pyx'
--- a/bzrlib/_dirstate_helpers_c.pyx	2008-09-18 06:25:16 +0000
+++ b/bzrlib/_dirstate_helpers_c.pyx	2008-09-24 03:08:14 +0000
@@ -26,7 +26,7 @@
 import stat
 
 from bzrlib import cache_utf8, errors, osutils
-from bzrlib.dirstate import DirState, pack_stat
+from bzrlib.dirstate import DirState
 from bzrlib.osutils import pathjoin, splitpath
 
 
@@ -60,8 +60,6 @@
 cdef extern from "stdlib.h":
     unsigned long int strtoul(char *nptr, char **endptr, int base)
 
-cdef extern from "stdio.h":
-    void printf(char *format, ...)
 
 cdef extern from 'sys/stat.h':
     int S_ISDIR(int mode)
@@ -776,9 +774,9 @@
     return 0
 
 
-#cdef object _encode
 _encode = binascii.b2a_base64
 
+
 from struct import pack
 cdef _pack_stat(stat_value):
     """return a string representing the stat value's key fields.
@@ -815,19 +813,17 @@
 cdef _update_entry(self, entry, abspath, stat_value):
     """Update the entry based on what is actually on disk.
 
+    :param self: The dirstate object this is operating on.
     :param entry: This is the dirblock entry for the file in question.
     :param abspath: The path on disk for this file.
-    :param stat_value: (optional) if we already have done a stat on the
-        file, re-use it.
+    :param stat_value: The stat value done on the path.
     :return: The sha1 hexdigest of the file (40 bytes) or link target of a
             symlink.
     """
-    # TODO - require pyrex 0.8, then use a pyd file to define access to the _st
-    # mode of the compiled stat objects.
+    # TODO - require pyrex 0.9.8, then use a pyd file to define access to the
+    # _st mode of the compiled stat objects.
     cdef int minikind, saved_minikind
     cdef void * details
-    # pyrex 0.9.7 would allow cdef list details_list, and direct access rather
-    # than PyList_GetItem_void_void below
     minikind = minikind_from_mode(stat_value.st_mode)
     if 0 == minikind:
         return None
@@ -883,7 +879,7 @@
             block_index, entry_index, dir_present, file_present = \
                 self._get_block_entry_index(entry[0][0], entry[0][1], 0)
             self._ensure_block(block_index, entry_index,
-                               osutils.pathjoin(entry[0][0], entry[0][1]))
+                               pathjoin(entry[0][0], entry[0][1]))
     elif minikind == c'l':
         link_or_sha1 = self._read_link(abspath, saved_link_or_sha1)
         if self._cutoff_time is None:
@@ -977,6 +973,7 @@
     cdef int path_index
     cdef object root_dir_info
     cdef object bisect_left
+    cdef object pathjoin
 
     def __init__(self, include_unchanged, use_filesystem_for_exec,
         search_specific_files, state, source_index, target_index,
@@ -1023,6 +1020,7 @@
         self.path_index = 0
         self.root_dir_info = None
         self.bisect_left = bisect.bisect_left
+        self.pathjoin = osutils.pathjoin
 
     cdef _process_entry(self, entry, path_info):
         """Compare an entry and real disk to generate delta information.
@@ -1084,7 +1082,7 @@
                 # as well.
                 old_path = source_details[1]
                 old_dirname, old_basename = os.path.split(old_path)
-                path = pathjoin(entry[0][0], entry[0][1])
+                path = self.pathjoin(entry[0][0], entry[0][1])
                 old_entry = self.state._get_entry(self.source_index,
                                              path_utf8=old_path)
                 # update the source details variable to be the real
@@ -1106,7 +1104,7 @@
                 target_kind = path_info[2]
                 if target_kind == 'directory':
                     if path is None:
-                        old_path = path = pathjoin(old_dirname, old_basename)
+                        old_path = path = self.pathjoin(old_dirname, old_basename)
                     file_id = entry[0][2]
                     self.new_dirname_to_file_id[path] = file_id
                     if source_minikind != c'd':
@@ -1145,7 +1143,7 @@
                     raise Exception, "unknown kind %s" % path_info[2]
             if source_minikind == c'd':
                 if path is None:
-                    old_path = path = pathjoin(old_dirname, old_basename)
+                    old_path = path = self.pathjoin(old_dirname, old_basename)
                 if file_id is None:
                     file_id = entry[0][2]
                 self.old_dirname_to_file_id[old_path] = file_id
@@ -1196,7 +1194,7 @@
                 or source_exec != target_exec
                 ):
                 if old_path is None:
-                    path = pathjoin(old_dirname, old_basename)
+                    path = self.pathjoin(old_dirname, old_basename)
                     old_path = path
                     old_path_u = self.utf8_decode(old_path)[0]
                     path_u = old_path_u
@@ -1219,7 +1217,7 @@
                 return self.uninteresting
         elif source_minikind == c'a' and _versioned_minikind(target_minikind):
             # looks like a new file
-            path = pathjoin(entry[0][0], entry[0][1])
+            path = self.pathjoin(entry[0][0], entry[0][1])
             # parent id is the entry for the path in the target tree
             # TODO: these are the same for an entire directory: cache em.
             parent_id = self.state._get_entry(self.target_index,
@@ -1259,7 +1257,7 @@
             # if its still on disk, *and* theres no other entry at this
             # path [we dont know this in this routine at the moment -
             # perhaps we should change this - then it would be an unknown.
-            old_path = pathjoin(entry[0][0], entry[0][1])
+            old_path = self.pathjoin(entry[0][0], entry[0][1])
             # parent id is the entry for the path in the target tree
             parent_id = self.state._get_entry(self.source_index, path_utf8=entry[0][0])[0][2]
             if parent_id == entry[0][2]:

=== modified file 'bzrlib/_dirstate_helpers_py.py'
--- a/bzrlib/_dirstate_helpers_py.py	2008-09-02 18:51:03 +0000
+++ b/bzrlib/_dirstate_helpers_py.py	2008-09-24 03:08:14 +0000
@@ -289,4 +289,3 @@
     # To convert from format 3 => format 2
     # state._dirblocks = sorted(state._dirblocks)
     state._dirblock_state = DirState.IN_MEMORY_UNMODIFIED
-

=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2008-09-18 06:36:28 +0000
+++ b/bzrlib/dirstate.py	2008-09-24 03:08:14 +0000
@@ -211,7 +211,6 @@
 import time
 import zlib
 
-import bzrlib
 from bzrlib import (
     cache_utf8,
     debug,
@@ -221,7 +220,6 @@
     osutils,
     trace,
     )
-from bzrlib.osutils import pathjoin, splitpath
 
 
 # This is the Windows equivalent of ENOTDIR
@@ -2738,15 +2736,15 @@
             raise errors.ObjectNotLocked(self)
 
 
-def py_update_entry(self, entry, abspath, stat_value,
+def py_update_entry(state, entry, abspath, stat_value,
                  _stat_to_minikind=DirState._stat_to_minikind,
                  _pack_stat=pack_stat):
     """Update the entry based on what is actually on disk.
 
+    :param state: The dirstate this entry is in.
     :param entry: This is the dirblock entry for the file in question.
     :param abspath: The path on disk for this file.
-    :param stat_value: (optional) if we already have done a stat on the
-        file, re-use it.
+    :param stat_value: The stat value done on the path.
     :return: The sha1 hexdigest of the file (40 bytes) or link target of a
             symlink.
     """
@@ -2774,13 +2772,13 @@
     # process this entry.
     link_or_sha1 = None
     if minikind == 'f':
-        link_or_sha1 = self._sha1_file(abspath)
-        executable = self._is_executable(stat_value.st_mode,
+        link_or_sha1 = state._sha1_file(abspath)
+        executable = state._is_executable(stat_value.st_mode,
                                          saved_executable)
-        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):
+        if state._cutoff_time is None:
+            state._sha_cutoff_time()
+        if (stat_value.st_mtime < state._cutoff_time
+            and stat_value.st_ctime < state._cutoff_time):
             entry[1][0] = ('f', link_or_sha1, stat_value.st_size,
                            executable, packed_stat)
         else:
@@ -2794,21 +2792,21 @@
             # have a directory block for it. This doesn't happen very
             # often, so this doesn't have to be super fast.
             block_index, entry_index, dir_present, file_present = \
-                self._get_block_entry_index(entry[0][0], entry[0][1], 0)
-            self._ensure_block(block_index, entry_index,
+                state._get_block_entry_index(entry[0][0], entry[0][1], 0)
+            state._ensure_block(block_index, entry_index,
                                osutils.pathjoin(entry[0][0], entry[0][1]))
     elif minikind == 'l':
-        link_or_sha1 = self._read_link(abspath, saved_link_or_sha1)
-        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):
+        link_or_sha1 = state._read_link(abspath, saved_link_or_sha1)
+        if state._cutoff_time is None:
+            state._sha_cutoff_time()
+        if (stat_value.st_mtime < state._cutoff_time
+            and stat_value.st_ctime < state._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
+    state._dirblock_state = DirState.IN_MEMORY_MODIFIED
     return link_or_sha1
 update_entry = py_update_entry
 
@@ -2848,7 +2846,7 @@
         self.want_unversioned = want_unversioned
         self.tree = tree
 
-    def _process_entry(self, entry, path_info):
+    def _process_entry(self, entry, path_info, pathjoin=osutils.pathjoin):
         """Compare an entry and real disk to generate delta information.
 
         :param path_info: top_relpath, basename, kind, lstat, abspath for
@@ -3109,11 +3107,12 @@
     def iter_changes(self):
         """Iterate over the changes."""
         utf8_decode = cache_utf8._utf8_decode
-        cmp_by_dirs = bzrlib.dirstate.cmp_by_dirs
+        _cmp_by_dirs = cmp_by_dirs
         _process_entry = self._process_entry
         uninteresting = self.uninteresting
         search_specific_files = self.search_specific_files
         searched_specific_files = self.searched_specific_files
+        splitpath = osutils.splitpath
         # sketch: 
         # compare source_index and target_index at or under each element of search_specific_files.
         # follow the following comparison table. Note that we only want to do diff operations when
@@ -3251,7 +3250,7 @@
                    current_block is not None):
                 if (current_dir_info and current_block
                     and current_dir_info[0][0] != current_block[0]):
-                    if cmp_by_dirs(current_dir_info[0][0], current_block[0]) < 0:
+                    if _cmp_by_dirs(current_dir_info[0][0], current_block[0]) < 0:
                         # filesystem data refers to paths not covered by the dirblock.
                         # this has two possibilities:
                         # A) it is versioned but empty, so there is no block for it
@@ -3450,6 +3449,7 @@
         _bisect_path_right_c as _bisect_path_right,
         cmp_by_dirs_c as cmp_by_dirs,
         ProcessEntryC as _process_entry,
+        update_entry as update_entry,
         )
 except ImportError:
     from bzrlib._dirstate_helpers_py import (

=== modified file 'bzrlib/tests/test_dirstate.py'
--- a/bzrlib/tests/test_dirstate.py	2008-09-13 07:59:05 +0000
+++ b/bzrlib/tests/test_dirstate.py	2008-09-24 03:08:14 +0000
@@ -571,7 +571,7 @@
             # 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'))
+            sha1sum = dirstate.update_entry(state, entry, 'a-file', os.lstat('a-file'))
             # We should have gotten a real sha1
             self.assertEqual('ecc5374e9ed82ad3ea3b4d452ea995a5fd3e70e3',
                              sha1sum)
@@ -612,7 +612,7 @@
         state.lock_read()
         try:
             entry = state._get_entry(0, path_utf8='a-file')
-            sha1sum = state.update_entry(entry, 'a-file', os.lstat('a-file'))
+            sha1sum = dirstate.update_entry(state, entry, 'a-file', os.lstat('a-file'))
             # We should have gotten a real sha1
             self.assertEqual('ecc5374e9ed82ad3ea3b4d452ea995a5fd3e70e3',
                              sha1sum)




More information about the bazaar-commits mailing list