[MERGE] make it possible to pass NULL_REVISION as the branch_tip parameter to merge_sort

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael Hudson wrote:
| This is currently breaking loggerhead when you try to view an empty
| branch -- easy to work around, but still a bug.
|
| Cheers,
| mwh
|

So the problem/confusion as I understand it is that doing:

b.repository.get_revision_graph(NULL_REVISION) returns {} (the empty dictionary).

And then you pass that into topo_sort which can't find the key that you are
looking for.

We've slowly been moving towards an api which exposes NULL_REVISION a bit more.
Specifically a Parent Provider will return parents = (NULL_REVISION,) where it
used to return () (an empty list/tuple). The empty list is now reserved for ghosts.

This causes a bit of friction, as Pack (and Knit) repositories have to check
every parent list to see if it is empty, and then insert (NULL_REVISION,) in its
place. And then get_revision_graph() has to check each one and filter it out
again. (This is in a branch I have that tries to use the Graph interface for
get_revision_graph().)

So the Graph apis seem to explicitly track NULL_REVISION, but they are then
hidden by all the other apis. I would like to standardize on one, and start
deprecating the others.

~ from bzrlib import errors
+import bzrlib.revision


~ __all__ = ["topo_sort", "TopoSorter", "merge_sort", "MergeSorter"]
@@ -406,7 +407,8 @@
~         self._left_subtree_pushed_stack = []

~         # seed the search with the tip of the branch
- -        if branch_tip is not None:
+        if (branch_tip is not None and
+            branch_tip != bzrlib.revision.NULL_REVISION):
~             parents = self._graph.pop(branch_tip)
~             self._push_node(branch_tip, 0, parents)

^- We prefer to do:

from bzrlib import revision [as _mod_revision]

~  if (... branch_tip != revision.NULL_REVISION)

The specific reason is that accessing "bzrlib...." can make it easy to forget an
import and it still works as long as that module happened to get imported first.
~ (Basically an import race condition.) By explicitly importing just the module
you know whether you have it or not.


BB:tweak

I'm okay with this patch, as it works around a bug. What I would *like* is to
have our apis be more consistent. If 'branch.last_revision()' can return
NULL_REVISION, then branch.repository.get_revision_graph(NULL_REVISION) should
include NULL_REVISION in the returned dictionary.
We can just have merge_sort(get_revision_graph()) start counting at 0, so that
NULL_REVISION == 0, and the rest count up from there.

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

iD8DBQFHxYBoJdeBCYSNAAMRAjFBAKDR38++y1XNI9sGkpmWio2gl5vmdwCeJngp
opkmSbe4s3H8PLEjP7JvfGg=
=ssnI
-----END PGP SIGNATURE-----