Rev 2339: Remove now-unecessary encode/decode calls for revision ids. in http://bazaar.launchpad.net/%7Ebzr/bzr/dirstate

John Arbash Meinel john at arbash-meinel.com
Sat Feb 17 04:07:58 GMT 2007


At http://bazaar.launchpad.net/%7Ebzr/bzr/dirstate

------------------------------------------------------------
revno: 2339
revision-id: john at arbash-meinel.com-20070217040647-1kl4s4bfi01cz1jb
parent: john at arbash-meinel.com-20070217033450-q6dtvrwl24ckzg5o
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: dirstate
timestamp: Fri 2007-02-16 22:06:47 -0600
message:
  Remove now-unecessary encode/decode calls for revision ids.
  Restore some file_id encode/decode calls because the code still expects unicode file ids.
modified:
  bzrlib/dirstate.py             dirstate.py-20060728012006-d6mvoihjb3je9peu-1
  bzrlib/tests/tree_implementations/test_walkdirs.py test_walkdirs.py-20060729160421-gmjnkotqgxdh98ce-1
  bzrlib/workingtree_4.py        workingtree_4.py-20070208044105-5fgpc5j3ljlh5q6c-1
-------------- next part --------------
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2007-02-16 06:44:26 +0000
+++ b/bzrlib/dirstate.py	2007-02-17 04:06:47 +0000
@@ -353,7 +353,7 @@
         root_parents = []
         for parent_tree in parent_trees:
             root_parents.append((
-                    parent_tree.inventory.root.revision.encode('utf8'),
+                    parent_tree.inventory.root.revision,
                     'directory', '',
                     '',
                     '',
@@ -431,14 +431,12 @@
 
     def _get_ghosts_line(self, ghost_ids):
         """Create a line for the state file for ghost information."""
-        return '\0'.join([str(len(ghost_ids))] +
-                         [g.encode('utf8') for g in ghost_ids])
-        
+        return '\0'.join([str(len(ghost_ids))] + ghost_ids)
+
     def _get_parents_line(self, parent_ids):
         """Create a line for the state file for parents information."""
-        return '\0'.join([str(len(parent_ids))] +
-                         [p.encode('utf8') for p in parent_ids])
-        
+        return '\0'.join([str(len(parent_ids))] + parent_ids)
+
     def get_parent_ids(self):
         """Return a list of the parent tree ids for the directory state."""
         self._read_header_if_needed()
@@ -450,6 +448,7 @@
         :param path_utf8: An utf8 path to be looked up.
         :return: The dirstate row tuple for path, or (None, None)
         """
+        assert isinstance(path_utf8, str), 'path_utf8 is not a str: %s %s' % (type(path), path)
         self._read_dirblocks_if_needed()
         if path_utf8 == '':
             return self._root_row
@@ -497,7 +496,7 @@
 
         The new dirstate will be an empty tree - that is it has no parents,
         and only a root node - which has id ROOT_ID.
-        
+
         :param path: The name of the file for the dirstate.
         :return: A DirState object.
         """
@@ -508,7 +507,7 @@
         # persist.
         result = DirState()
         result._state_file = open(path, 'wb+')
-        # a new root directory, with a pack_stat (the x's) that is just noise and will 
+        # a new root directory, with a pack_stat (the x's) that is just noise and will
         # never match the output of base64 encode.
         root_row_data = ('', '', 'directory', bzrlib.inventory.ROOT_ID, 0,
             DirState.NULLSTAT, '')
@@ -536,7 +535,7 @@
 
     def _get_output_lines(self, lines):
         """format lines for final output.
-        
+
         :param lines: A sequece of lines containing the parents list and the
             path lines.
         """
@@ -565,21 +564,21 @@
     def _parent_info(self, parent_tree, fileid):
         """Generate the parent information for file_id in parent_tree."""
         # FIXME: This is probably expensive - we encode the path that in the
-        # common case will be the same across all parents, twice. 
+        # common case will be the same across all parents, twice.
         # also, id2path is slow in inventory, and we should make that fast.
         try:
             parent_entry = parent_tree.inventory[fileid]
         except errors.NoSuchId:
-            # this parent does not have fileid - return a bogus entry, which 
+            # this parent does not have fileid - return a bogus entry, which
             # will be filtered out on iteration etc.
             # an empty revision id is bogus and safe to put on disk
-            # we make it be a 'file', just because. We give it the 
+            # we make it be a 'file', just because. We give it the
             # deleted file path dirname and basename, 0 size, not executable
             # and no sha1.
             return DirState.NULL_PARENT_ROW
         parent_path = parent_tree.inventory.id2path(fileid)
         dirname, basename = os.path.split(parent_path.encode('utf8'))
-        return (parent_entry.revision.encode('utf8'),
+        return (parent_entry.revision,
             parent_entry.kind,
             # FIXME: set these from the parent
             dirname, basename,
@@ -675,13 +674,13 @@
         info = parent_line.split('\0')
         num_parents = int(info[0])
         assert num_parents == len(info)-2, 'incorrect parent info line'
-        self._parents = [p.decode('utf8') for p in info[1:-1]]
+        self._parents = info[1:-1]
 
         ghost_line = self._state_file.readline()
         info = ghost_line.split('\0')
         num_ghosts = int(info[1])
         assert num_ghosts == len(info)-3, 'incorrect ghost info line'
-        self._ghosts = [p.decode('utf8') for p in info[2:-1]]
+        self._ghosts = info[2:-1]
         self._header_state = DirState.IN_MEMORY_UNMODIFIED
 
     def _read_header_if_needed(self):

=== modified file 'bzrlib/tests/tree_implementations/test_walkdirs.py'
--- a/bzrlib/tests/tree_implementations/test_walkdirs.py	2007-02-13 05:30:56 +0000
+++ b/bzrlib/tests/tree_implementations/test_walkdirs.py	2007-02-17 04:06:47 +0000
@@ -41,8 +41,8 @@
 
     def test_walkdir_root(self):
         tree = self.get_tree_with_subdirs_and_all_content_types()
+        tree.lock_read()
         expected_dirblocks = self.get_all_subdirs_expected(tree)
-        tree.lock_read()
         # test that its iterable by iterating
         result = []
         for dirinfo, block in tree.walkdirs():
@@ -61,10 +61,10 @@
             
     def test_walkdir_subtree(self):
         tree = self.get_tree_with_subdirs_and_all_content_types()
+        # test that its iterable by iterating
+        result = []
+        tree.lock_read()
         expected_dirblocks = self.get_all_subdirs_expected(tree)[1:]
-        # test that its iterable by iterating
-        result = []
-        tree.lock_read()
         for dirinfo, block in tree.walkdirs('1top-dir'):
             newblock = []
             for row in block:

=== modified file 'bzrlib/workingtree_4.py'
--- a/bzrlib/workingtree_4.py	2007-02-16 14:23:21 +0000
+++ b/bzrlib/workingtree_4.py	2007-02-17 04:06:47 +0000
@@ -312,7 +312,7 @@
             fileid_utf8 = file_id.encode('utf8')
         if path is not None:
             # path lookups are faster
-            row = state._get_row(path)
+            row = state._get_row(path.encode('utf8'))
             if file_id:
                 if row[0][3] != fileid_utf8:
                     raise BzrError('integrity error ? : mismatching file_id and path')
@@ -361,7 +361,7 @@
         """See Mutable.last_revision."""
         parent_ids = self.current_dirstate().get_parent_ids()
         if parent_ids:
-            return parent_ids[0].decode('utf8')
+            return parent_ids[0]
         else:
             return None
 
@@ -548,6 +548,7 @@
 
         WorkingTree4 supplies revision_trees for any basis tree.
         """
+        revision_id = osutils.safe_revision_id(revision_id)
         dirstate = self.current_dirstate()
         parent_ids = dirstate.get_parent_ids()
         if revision_id not in parent_ids:
@@ -560,6 +561,7 @@
     @needs_tree_write_lock
     def set_last_revision(self, new_revision):
         """Change the last revision in the working tree."""
+        new_revision = osutils.safe_revision_id(new_revision)
         parents = self.get_parent_ids()
         if new_revision in (NULL_REVISION, None):
             assert len(parents) < 2, (
@@ -583,6 +585,7 @@
         :param revision_ids: The revision_ids to set as the parent ids of this
             working tree. Any of these may be ghosts.
         """
+        revision_ids = [osutils.safe_revision_id(r) for r in revision_ids]
         trees = []
         for revision_id in revision_ids:
             try:
@@ -601,7 +604,7 @@
     def set_parent_trees(self, parents_list, allow_leftmost_as_ghost=False):
         """Set the parents of the working tree.
 
-        :param parents_list: A list of (revision_id, tree) tuples. 
+        :param parents_list: A list of (revision_id, tree) tuples.
             If tree is None, then that element is treated as an unreachable
             parent tree - i.e. a ghost.
         """
@@ -611,9 +614,10 @@
                 raise errors.GhostRevisionUnusableHere(parents_list[0][0])
         real_trees = []
         ghosts = []
-        # convert absent trees to the null tree, which we convert back to 
+        # convert absent trees to the null tree, which we convert back to
         # missing on access.
         for rev_id, tree in parents_list:
+            rev_id = osutils.safe_revision_id(rev_id)
             if tree is not None:
                 real_trees.append((rev_id, tree))
             else:
@@ -666,14 +670,14 @@
         paths_to_unversion = set()
         # sketch:
         # check if the root is to be unversioned, if so, assert for now.
-        # make a copy of the _dirblocks data 
+        # make a copy of the _dirblocks data
         # during the copy,
         #  skip paths in paths_to_unversion
         #  skip ids in ids_to_unversion, and add their paths to
         #  paths_to_unversion if they are a directory
         # if there are any un-unversioned ids at the end, raise
         if state._root_row[0][3] in ids_to_unversion:
-            # I haven't written the code to unversion / yet - it should be 
+            # I haven't written the code to unversion / yet - it should be
             # supported.
             raise errors.BzrError('Unversioning the / is not currently supported')
         new_blocks = []
@@ -748,10 +752,11 @@
 
     def initialize(self, a_bzrdir, revision_id=None):
         """See WorkingTreeFormat.initialize().
-        
+
         revision_id allows creating a working tree at a different
         revision than the branch is at.
         """
+        revision_id = osutils.safe_revision_id(revision_id)
         if not isinstance(a_bzrdir.transport, LocalTransport):
             raise errors.NotLocalUrl(a_bzrdir.transport.base)
         transport = a_bzrdir.get_workingtree_transport(self)
@@ -787,7 +792,7 @@
 
     def _open(self, a_bzrdir, control_files):
         """Open the tree itself.
-        
+
         :param a_bzrdir: the dir for the tree.
         :param control_files: the control files for the tree.
         """
@@ -803,7 +808,7 @@
 
     def __init__(self, dirstate, revision_id, repository):
         self._dirstate = dirstate
-        self._revision_id = revision_id
+        self._revision_id = osutils.safe_revision_id(revision_id)
         self._repository = repository
         self._inventory = None
         self._locked = 0
@@ -835,7 +840,7 @@
 
     def _generate_inventory(self):
         """Create and set self.inventory from the dirstate object.
-        
+
         This is relatively expensive: we have to walk the entire dirstate.
         Ideally we would not, and instead would """
         assert self._locked, 'cannot generate inventory of an unlocked '\
@@ -852,11 +857,12 @@
         rows = self._dirstate._iter_rows()
         parent_rows = []
         for row in rows:
-            parent_rows.append((row[1][parent_index], row[0][3].decode('utf8')))
+            parent_rows.append((row[1][parent_index], row[0][3]))
         parent_rows = iter(sorted(parent_rows, key=lambda x:x[0][2:3]))
         root_row = parent_rows.next()
-        inv = Inventory(root_id=root_row[1], revision_id=self._revision_id)
-        inv.root.revision = root_row[1][parent_index][0].decode('utf8')
+        inv = Inventory(root_id=root_row[1].decode('utf8'),
+                        revision_id=self._revision_id)
+        inv.root.revision = root_row[1][parent_index][0]
         # we could do this straight out of the dirstate; it might be fast
         # and should be profiled - RBC 20070216
         parent_ids = {'' : inv.root.file_id}
@@ -866,9 +872,9 @@
                 # not in this revision tree.
                 continue
             parent_id = parent_ids[dirname]
-            file_id = line[1]
+            file_id = line[1].decode('utf8')
             entry = entry_factory[kind](file_id, name.decode('utf8'), parent_id)
-            entry.revision = revid.decode('utf8')
+            entry.revision = revid
             if kind == 'file':
                 entry.executable = executable
                 entry.text_size = size
@@ -926,7 +932,7 @@
     def is_executable(self, file_id, path=None):
         ie = self.inventory[file_id]
         if ie.kind != "file":
-            return None 
+            return None
         return ie.executable
 
     def list_files(self, include_root=False):
@@ -938,7 +944,7 @@
             entries.next()
         for path, entry in entries:
             yield path, 'V', entry.kind, entry.file_id, entry
-        
+
     def lock_read(self):
         """Lock the tree for a set of operations."""
         if not self._locked:



More information about the bazaar-commits mailing list