[MERGE] Refactor fetch: add get_data_about_revision_ids to repository, and other changes.

[Andrew Bennetts]

John Arbash Meinel wrote:
[...]
> 
> !tweak
> 
> There was one thing that wasn't clear to me. Did you change the code so that
> only new revisions have their signatures copied?
> 
> How do you propagate signatures that you put on older revisions? The current
> code just does a full knit join (I believe) for all keys.

I don't think this branch changes the behaviour here.  This still happens (or
not) in _fetch_revision_texts, and I haven't changed what the various
implementations of that do.

Also, the tests pass... is there a test for the behaviour you're concerned
about? :)

> _same_repo() really seems like it should be a Repository member. Especially
> since not all repos will have a 'control_files._transport' member.
> 
> It actually seems like a "__eq__" would be a reasonable way to do it.

Hmm.  I think you're right.

I've implemented this, although the tests to make sure it works properly turned
out to be more involved that I expected!  There's lots of possible pitfalls,
like forgetting to define __ne__, and making sure cross-class comparisons don't
cause exceptions.  And of course the case that fetch.py is already taking care
of without much fanfare: repositories in the same bzrdir but with different
control files.  The subtleties here probably mean doing this on repository is
the right thing, so places like fetch don't risk making a mistake with an ad hoc
comparison.

I've submitted this change as a separate bundle though, I think the change
turned out to be subtle enough to deserve its own review, and it isn't dependent
on any other code.  It's mainly test code, so hopefully it'll be an easy review.
:)

> For Robert's comments, what about "get_data_to_fetch_for_revision_ids"
> 
> Eventually you have to write sentences, and our functions are already getting
> pretty long. Maybe:
> 
> get_fetch_info()
> get_data_to_fetch()

I think I prefer your first suggestion.  It's a method with a fairly specific
purpose, so I don't think it's too burdensome to give it a longish name.  It
only barely addresses Robert's concern though, which is that “data” is a pretty
opaque term for this.  “data_to_fetch_for” is enough of an improvement on
“data_about” that I'll use that, but I'm still not 100% happy with it.

-Andrew.