[MERGE] Tree.revision_tree

[Robert Collins]

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)
> > 
> > 
> > Cheers,
> > Rob
> > 
> 
> I like the ability to query WorkingTree for the other trees that it has
> cached.
> 
> I'd like you to explain why you think it belongs on the root level Tree
> object, rather than elsewhere.

There is no good reason to put it any lower in the hierarchy: Anything
conforming to Tree can be used as a source for merge, or to get a
differences. All Trees have parent_ids. So if you want access to the
parent trees, it makes sense to try the tree first -
aTree.revision_tree(aTree.get_parent_ids()[0]).

> Calling it 'Tree.revision_tree' doesn't quite seem right. Especially
> when I saw this exception:

Its called revision_tree to match the Repository api which behaves
identically: If the repository has a revision, calling revision_tree()
on it returns a RevisionTree, if it does not you get NoSuchRevision
raised. Martin and I both feel that the revision_tree method on
Repository could well be renamed.

> class NoSuchRevisionInTree(NoSuchRevision):
>     """The revision id %(revision_id)s is not present in the tree
> %(tree)s."""
> 
> In my mental model, a tree is a snapshot of a specific revision. It
> doesn't have other revisions present in its tree.

Well, this isn't a user facing error, and the wording can be improved -
how about 'The revision id %(revision_id)s is not cached by the tree
%(tree)s."""

> Now, you could do 'Tree.cached_revision_tree()', and say:
>
> """The cached tree corresponding to revision id %(revision_id)s is not
> available from tree %(trees)"""
> 
> 
> 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?

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/4ba2704e/attachment.pgp