What to do about 'revid in ancestry' checks.

[Robert Collins]

On Thu, 2007-07-19 at 10:53 -0400, Aaron Bentley wrote:

> > So on Graph, I propose reachable as a method name.
> > 
> > reachability = graph.reachable([(start_keys, target_keys)...])
> > 
> > This is a little awkward but handles the following use cases:
> > 
> >  - has A been merged into B ?
> >     reachable([(B, A)]) -> ((True, ), ) or ((False, ), )
> 
> A little more explanation of the return value would be helpful.
> 
> I've certainly wanted a get_relationship([(A, B), (A, C)]) method that
> could tell me if A was a descendant or ancestor of B, or whether they
> were siblings (cousins?).

So there are four relationships - parent, child, sibling, unrelated.

It might be interesting to have a generic call to establish the
relationship between two nodes. I guess this was just heading in that
direction for one relationship. There is probably a performance cost to
establishing *which* relationship rather than 'is a specific
relationship' though. Consider a 'whats the relationship' of A and B
where A is the first commit of a branch, and B is a commit in an
unrelated branch several thousand from the origin. While we can quickly
ascertain that B is not reachable from A, its several thousand
iterations to determine that A is not reachable from B.

So I'd be inclined to only use a 'give me the relationship' method for
cases where you would otherwise write 
  reachable([((A, ), (B, )),   ((B, ), (A, ))])

> > As for using get_revision_graph, thats because its the right name for
> > what we are trying to convey.
> 
> It's the wrong interface, though.  It would be an API break to
> substitute Graph for a dict, and it would probably be a bad idea to make
> Graph behave as a dict.

I agree that __item__ on Graph would be a mistake, its misleading about
the cost of dereferencing.

> > I did mention in my mail that I'd be happy
> > to change the interface to return the Graph object.
> > 
> > How do you feel about us renaming get_graph to get_revision_graph, if
> > its for doing graphs at the revision level?
> 
> Nice in theory, painful in practice.  Are you offering to do it?

yes, and I did for your other patch IIRC.

> > Or if you object to changing the interface of get_revision_graph, how
> > about get_graph('revisions') to indicate we're getting the revision
> > graph?
> 
> I'd want to know what the other invocations of get_graph would be like.
>  If you're proposing that 'revisions' be substituted for a file_id, I'm
> not keen on that.

Here are the graphs I'm aware of that are actively used:
revision graph
inventory graph (for compression only)
per-file graph (for last-changed calculation only)

I was proposing something like
get_graph('revisions')
get_graph('inventory')
get_graph('file', FILE_ID)

-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: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070720/6c466ac2/attachment.pgp