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

John Arbash Meinel john at arbash-meinel.com
Fri Apr 18 21:46:13 BST 2008


-----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.

Specifically, the bug is only present over the smart protocol because of
differing behaviors.

On a non-smart repository, doing:
	repo.get_graph().iter_ancestry([None])
just returns nothing, because the None revision isn't present in the repository.

However, for a RemoteRepository, it tries to serialize None as a string, which
fails. (Well it fails in an assert that all revision_ids are strings, since None
is not str.)


The "bug" is that the calling code was passing the full list, when the first
entry in the list was meant to be thrown away. It exists because 'merge_sort'
used it to figure out what revision to stop at. Since 'bzr log -r X..Y'
*includes* X and Y, you need to *stop* at the revision before X, because we want
to show the revisions merged into X (and not just revision X).

So one test could be to strip decorate a Repository object to return a custom
"graph" object and then assert that the first revision passed in is not passed
to graph.iter_ancestry.

That seems a bit hackish, though, and prone to breaking if we change how that
function works (iter_ancestry is not the best function to be using, what we
really want is sort of a graph.find_unique_with_parents(mainline_revs[1:],
stop=mainline_revs[0]))

Another possible fix would be to prefix the mainline revisions with "null:"
instead of None, but I'm not 100% positive what that does in stuff like
'merge_sort' when you have multiple roots. And it still is technically incorrect
to be passing in the first revision.

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://.

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://)

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkgJCJUACgkQJdeBCYSNAANIwACeJoNFURXGme87F6zsa4ZAvcOJ
I04AoJ3AGAGzckgbGM+zn30zVJpX/LpR
=oV6a
-----END PGP SIGNATURE-----
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: dont_log_view_none_211661-3374.patch
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20080418/304220cc/attachment.diff 


More information about the bazaar mailing list