[MERGE] Factor out the Graph.heads() cache from _RevisionTextVersionCache for reuse, and use it in commit.

[Andrew Bennetts]

Robert Collins wrote:
> On Wed, 2007-10-17 at 21:51 -0500, John Arbash Meinel wrote:
[...]
> > I would argue that you let them create their own sets. Rather than forcibly
> > creating a new object every time you enter this function.
> > 
> > If *all* callers need to modify the set, then fine. But I doubt that is the case.
> > 
> > Maybe it is an API break... but probably a rather minor one at that.
> 
> Well the cache is new, but it seems like a good idea to me to have its
> api match that of the function it wraps, so that the cache can be
> inserted and removed seamlessly.

I think it's good to have an API that's safe by default (and just like the API
it is a cache for).  If we make it easy to corrupt the cache we're inviting
confusing bugs down the track.  

So, I think returning a copy is fine, just add a brief comment in the code to
make it clear that making a copy is the intent, so that future reviewers don't
look at the code and think “huh, that's odd”.

Caching and returning a frozenset would be just as fine from a safety point of
view, so I'd say Robert should just do whichever way is more convenient for him.

-Andrew.