[MERGE REVIEW] Use InventoryDirectory to represent tree root

Aaron Bentley aaron.bentley at utoronto.ca
Mon Aug 7 19:32:58 BST 2006

Hash: SHA1

Robert Collins wrote:

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

Agreed, but I didn't do this, because I was trying to do one step at a time.

> @@ -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. 

Doesn't iter_entries now yield the root entry?

> 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

These all assume that a tree has one root directory, which is not true
in nested-trees, where this came from.

> 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.

It is sometimes None in nested-trees, where this came from.

> 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 think that it depends on whether the origin revision's tree has a root
id or not.  However, these changes don't force us to take either path.

Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org


More information about the bazaar mailing list