PreviewTree.inventory, and iter_changes

[Robert Collins]

So, I'm change iter_changes to prevent InconsistentDeltas. We had a
previous discussion about inventories, and trees, and avoiding the use
of inventories... but PreviewTree doesn't implement .inventory.

So the general thrust to avoid using inventories is because of several
aspects:
 - in working trees the inventory can be stale with respect to tree
(e.g. the kind can be wrong), so code that just looks at the inventory
will tend to end up dealing with out of date data.
 - our in memory, mutable inventories are not very lazy, and difficult
   to make be effectively safely lazy
 - making changes to the inventory won't change the actual tree
 - there are changes that are easy to make to an inventory that are
   physically impossible to a tree (which is a reason why we have
TreeTransform to manage making changes to a tree).
 - Trees provide a better interface for getting file contents

However, we don't have an interface that permits determining if a single
fileid has changed between two trees without using inventory entries.
iter_changes builds on iter_entries_by_dir, and then calls
_comparison_data - which takes an inventory entry.

For the changes I'm making to iter_changes, I need to compare a fileid
between two trees without recursing. This should be rarely called,
because the paths to the root of a tree from a users specific files
selection are usually way less than the number of paths in a tree.

One possibility is a publicish method on InterTree, which doesn't help
with PreviewTree, as the obvious implementation using .inventory, will
still fail. So, I can see the following possibilities:
 - implement a PreviewTree specific InterTree
 - implement .inventory on PreviewTree
 - implement a method on Tree to get an arbitrary inventory entry. (
   which for most trees will be equivalent to .inventory[file_id]).

I think a PreviewTree specific InterTree isn't particularly useful,
because PreviewTree's could be diffed against any tree, but only the
basis tree could be special cased.

I'm inclined to implement .inventory on PreviewTree, using a decorator
approach - a PreviewTreeInventory which answers with the transform
changes first, and after that falls back to the original tree's
inventory. This seems like it would be pretty easy to me (after all
PreviewTree.iter_entries_by_dir already knows how to get entries
appropriately), and not intrinsically more expensive or problematic than
having a method to get an arbitrary inventory entry. It also makes
PreviewTree more like all our other trees, which I think is a good
thing.

I don't think a method to get a single inventory entry is any more or
less 'not using the inventory' - if we take this route, all a user has
to do is 'tree.get_entry(tree.root)'.children, and they can walk the
entire inventory with precisely the same bugs implicit as if we
permitted access to .inventory.

For now, I'm just not running the PreviewTree tests for my new code,
until this is resolved - but I'm keen to get it working with
PreviewTree.

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090723/2b5c31dd/attachment-0001.pgp