[MERGE] Fix RemoteRepository.get_parent_map stacking

Andrew Bennetts andrew.bennetts at canonical.com
Tue Nov 18 22:42:47 GMT 2008


Aaron Bentley wrote:
> John Arbash Meinel wrote:
[...]
> > It also seems a bit circuitous to have get_parent_map() return a locally
> > defined class whose only purpose is to call _get_parent_map().
> 
> I agree.  The only reason Repositories support get_parent_map is so they
> can act as ParentsProviders.  But now, that has been hijacked and we can
> no longer expect that sane callers will call get_graph.  So we must
> provide a ParentsProvider for RemoteRepository.

RemoteRepository isn't fundamentally different to other Repository classes
in this respect, AFAICT.  Other repository classes also need a separate
object to pass to their ParentsProvider constructor — the difference is that
non-remote repositories already have a self.revisions attribute that can
serve that purpose.

Eventually I expect to have RemoteRepository's
revisions/texts/inventories/signatures attributes use a RemoteVersionedFiles
object as their implementation (rather than the _real_repository), which
will remove that difference too (remote_repo.revisions.get_parent_map would
then invoke the current Repository.get_parent_map RPC, so you wouldn't need
your new class anymore).

Whether or not sensible callers use repo.get_graph rather than
repo.get_parent_map seems orthogonal to me; surely implementing get_parent_map
with a ParentsProvider class has always required that there is another
object that can be used for the underlying get_parent_map?  Or am I
misunderstanding something fundamental?

-Andrew.




More information about the bazaar mailing list