[MERGE] Move various weave specific code out of the base Repository class to weaverepo.py.

[Ian Clatworthy]

Robert Collins wrote:
> This patch gets rid of some cruft that we put in for weaves in the
> repository interface; moving it instead to the weaverepo format specific
> location.

I've reviewed this now and it looks pretty close to me. I've voted
comment rather than tweak because I'm not 100% confident as to why the
duplication is required. If you can briefly explain why on IRC or email,
I'll upgrade my vote, assuming the issue is my lack of knowledge in this
area.

bb:comment

> Due to the current subclassing structure in weaverepo.py there is some
> duplicate code; but frankly I could not care less - its in our legacy,
> deprecated, warn-on-use formats anyway.

I'm ok with the duplicated code given it's legacy code. BUT *I* don't
understand why the code needs to be in WeaveMetaDirRepository at all.
(Taking it out of Repository into AllInOneRepository
on the other hand makes sense to me.)

Other comments below ...

>      @needs_read_lock
>      def get_revisions(self, revision_ids):
> +        """Get many revisions at once."""
> +        return self._get_revisions(revision_ids)
> +
> +    @needs_read_lock
> +    def _get_revisions(self, revision_ids):
> +        """Core work logic to get many revisions without sanity checks."""
>          revision_ids = [osutils.safe_revision_id(r) for r in revision_ids]
> +        for rev_id in revision_ids:
> +            if not rev_id or not isinstance(rev_id, basestring):
> +                raise errors.InvalidRevisionId(revision_id=rev_id, branch=self)
>          revs = self._revision_store.get_revisions(revision_ids,
>                                                    self.get_transaction())

Something needs to change here IMO. Either the checks needs to move from
_get_revisions into get_revisions or the docstring for _get_revisions
needs to be more explicit about what checks are really required in it.

>      @needs_read_lock
> -    def get_revision_graph(self, revision_id=None):
> -        """Return a dictionary containing the revision graph.

get_revision_graph() is a public API on Repository and has just been
removed from the base class. It might be implemented in all the
subclasses but if it's part of the Repository API, then it ought to be
declared there (together with a NotImplementedError given that's our
policy for abstract methods.)

Ian C.