[MERGE] Tree.revision_tree

[Robert Collins]

On Sun, 2006-09-10 at 18:28 -0400, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > On Fri, 2006-09-08 at 09:22 -0500, John Arbash Meinel wrote:
> >> Robert Collins wrote:
> >>> Hi,
> >>> * Add a new method ``Tree.revision_tree`` which allows access to cached
> >>>   trees for arbitrary revisions. This allows the in development dirstate
> >>>   tree format to provide access to the callers to cached copies of 
> >>>   inventory data which are cheaper to access than inventories from the
> >>>   repository.
> >>>   (Robert Collins, Martin Pool)
> 
> I'm not very comfortable with this proposed interface.  It takes a
> special case and tries to make it look general, when it's really not.
> Providing the basis tree is supposed to be just an optimization.
> 
> Providing a call that looks like the Repository call, but only works for
> the basis tree is confusing.  I wasn't especially happy with having
> WorkingTree.basis_tree(), but this just compounds it.

It works for more than just the basis tree. On dirstate based trees it
works for every non-ghost parent.

> I don't think that the working tree is the right place to be storing a
> cache.  I would much rather make the Repository be aware of caches, and
> use them where possible.
> 
> Why not have working trees register themselves with Repositories?  That
> would allow us to use Repository.revision_tree(), but get cached
> inventories if available.
> 
> Now that we've got a clean separation between repositories, branches,
> and trees, I think we should be hesitant to go and blur those boundaries
> again.

I agree. Thats why the api is strict about only returning a tree if it
actually has the data, rather than having it ever go and fetch the
inventory from the repository.

> >> In my mental model, a tree is a snapshot of a specific revision. It
> >> doesn't have other revisions present in its tree.
> 
> Fully agreed.
> 
> >> It seems like if you are going to add this at the Tree level, then it
> >> should be implemented in RevisionTree(), as calling
> >> self._repository.revision_tree()
> > 
> > No, because the client is asking the tree on the basis of 'locally
> > present' and a RevisionTree's content may not be locally present. I had
> > hoped the docstring on Tree.revision_tree() would make that clear - can
> > you think of ways to improve that?
> 
> Part of the problem is the term "locally present".  My default
> interpretation is that everything on my hard disk is "locally present".
> 
> And since only part of a tree is ever local to the WorkingTree (the
> inventory), it's hard to say that the basis tree is "locally present",
> either.

Well like I say above, its about exposing what dirstate offers cleanly.

Other suggestions appreciated, but I need some way of getting at it that
is of broad scope, to change other code to use it.

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/20060911/bfa838d3/attachment.pgp