Comment 1 for bug 211661

Revision history for this message
John A Meinel (jameinel) wrote :

Do you have a specific revision where this is occurring? I see the assert in a slightly different place (line 862), with bzr.dev revno 3373

If I change the assert in question to:
        for key in keys:
            assert type(key) is str, \
                "Key type is %s not a plain string" % (type(key),)

I can see that it is failing because it is getting a key of None, rather than a real value.

Tracing it back up the stack, it seems to be because _get_mainline_revs() is returning [None, all, the, branch, mainline] for 'mainline_revs'.

I can't figure out why, but it is very explicitly adding a revision at the beginning:
    # override the mainline to look like the revision history.
    mainline_revs = [revision_id for index, revision_id in cut_revs]
    if cut_revs[0][0] == 1:
        mainline_revs.insert(0, None)
    else:
        mainline_revs.insert(0, which_revs[start_revno-2][1])

It seems that if you *don't* insert something there, and you do:

bzr log ---short -r ..10

you end up getting 2-10 instead of 1-10, this seems to be because of this line:

def get_view_revisions(mainline_revs, rev_nos, branch, direction,
                       include_merges=True):

...
    if include_merges is False:
        revision_ids = mainline_revs[1:]
        if direction == 'reverse':
            revision_ids.reverse()
        for revision_id in revision_ids:
            yield revision_id, str(rev_nos[revision_id]), 0

So get_view_revisions explicitly assumes that there will be an extra entry at the beginning of 'mainline_revs'.

This seems to be required, because the code used "merge_sort(..., tip=mainline_revs[-1], mainline_revs=mainline_revs...).
And 'merge_sort' seems designed to stop at the first revision in the list:
            self._mainline_revisions = list(mainline_revisions)
            self._stop_revision = self._mainline_revisions[0]

So I think the real fix is just:
    parent_map = dict(((key, value) for key, value in
        graph.iter_ancestry(mainline_revs[1:]) if value is not None))

Because 'mainline_revs' seems to be defined as including 1 more node that the whole graph has. It certainly could be considered a bug that 'graph.iter_ancestry()' is allowing None to be passed, though arguably it normally just ignores it as a node that isn't in the graph.

Anyway, I'll post the fix to the list.