[MERGE REVIEW] Use InventoryDirectory to represent tree root

[Robert Collins]

On Sun, 2006-08-06 at 18:02 -0400, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi all,
> 
> Here's another hopefully-uncontroversial patch split out from the
> nested-trees branch.  Michael Ellerman's Shelf plugin was invaluable for
> this work.
> 
> It uses InventoryDirectory to represent root entries, rather than
> RootEntry, which is now deprecated.  This means that the inventory kind
> of a root entry is 'directory', not 'root_directory'.
> 
> Inventory.is_root is provided to determine whether an inventory id is
> the root inventory id, instead of checking the file kind.

+++ bzrlib/bzrdir.py    2006-08-05 22:30:26 +0000
@@ -1558,9 +1558,9 @@
         # the XML is now updated with text versions
         if __debug__:
             for file_id in inv:
+                if inv.is_root(file_id):
+                    continue
                 ie = inv[file_id]
-                if ie.kind == 'root_directory':
-                    continue
                 assert hasattr(ie, 'revision'), \
                     'no revision on {%s} in {%s}' % \
                     (file_id, rev.revision_id)

^^
This seems to be a place where giving the root entry a 'revision' value
- synthetic that is - would simplify the code. 


@@ -1580,6 +1580,8 @@
                rev_id)
         parent_invs = map(self._load_updated_inventory,
present_parents)
         for file_id in inv:
+            if inv.is_root(file_id):
+                continue

^^ if we switched to iter_entries here, we would not need to call
is_root. 


=== modified file 'bzrlib/info.py'
--- bzrlib/info.py      2006-07-26 07:11:35 +0000
+++ bzrlib/info.py      2006-08-05 07:56:41 +0000
@@ -208,7 +208,8 @@
 
     dir_cnt = 0
     for file_id in work_inv:
-        if work_inv.get_file_kind(file_id) == 'directory':
+        if (work_inv.get_file_kind(file_id) == 'directory' and 
+            not work_inv.is_root(file_id)):
             dir_cnt += 1
     print '  %8d versioned %s' \
           % (dir_cnt,

This might be simpler and more efficient in a different style:
for instance:
    # will do more work than the minimum, but is trivial to write :)
    dir_cnt = len(work_inv.walkdirs()) - 1
or:
    #should be about the cheapest:
    dir_cnt = len(file_id for file_id in work_inv
        if work_inv[file_id].kind == 'directory') - 1
or:
    # bit more readable:
    dirs = [file_id for file_id in work_inv if work_inv[file_id].kind ==
'directory']
    dir_cnt = len(dirs) - 1

I'm suggesting this because the 'is_root' call does not seem to add a
great deal here. No big deal either way.


In inventory.py:
+    def is_root(self, file_id):
+        return self.root is not None and file_id == self.root.file_id
+
 
self.root is a property - its never None, so theres no need to check for
that.


We dont use textinv.py at all - perhaps we should delete it?

Thats the last use of is_root I see in the patch other than the tests
for it - it looks to me like we may not need it at all. What do you
think?

I like the way this is looking - but not +1 just yet.

Cheers,
Rob


-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060807/d48b6cba/attachment.pgp