[MERGE][Bug #211661][1.4] bzr log bzr:// was failing

[Andrew Bennetts]

John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> This patch addresses:
> https://bugs.edge.launchpad.net/bzr/+bug/211661
>
> I didn't add a test case, because I wasn't sure how to do it.
>
[...]
> Another possibility is to change "graph.iter_ancestry()" to fail for all
> implementations if None (or anything that isn't a string) is passed. The the fix
> will be implicit, because otherwise log will fail locally, as well as for bzr://.

I agree with adding a repository_implementations test for that, even if it isn't
the whole answer to testing this bug.  The more consistent we can make our
repository implementations, the less we need to think of ways to test specific
combinations like "bzr log on RemoteRepository with ...".

> Anyway, this fixes it, and I'd like to get something like it merged for 1.4.
> Otherwise any 'bzr log' that includes the first revision will fail over the
> smart server. (aka, bzr log -r ..2 bzr://)

[...]
> === modified file 'bzrlib/remote.py'
> --- bzrlib/remote.py	2008-04-10 16:41:50 +0000
> +++ bzrlib/remote.py	2008-04-18 20:30:59 +0000
> @@ -859,7 +859,8 @@
>          body = self._serialise_search_recipe(recipe)
>          path = self.bzrdir._path_for_remote_call(self._client)
>          for key in keys:
> -            assert type(key) is str
> +            assert type(key) is str, \
> +                "Key type is %s not a plain string" % (type(key),)

I'd probably do:

            assert type(key) is str, "Key %r is not a plain str." % (key,)

Because the repr is probably more helpful than just the type (and generally
makes the type obvious as well).  Not a big deal, though.  

Although, if we do write a repository_implementations test that non-str revision
IDs should fail here, then we'll need a raise rather than an assert.  Which is
probably a good idea anyway.

-Andrew.