bzr.dev smart client fails on log
Bug #211661 reported by
Matthew Fuller
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Bazaar |
Fix Released
|
Critical
|
John A Meinel |
Bug Description
With bzr.dev, doing a 'log' over a smart protocol dies with a traceback ending:
File "/home/
assert type(key) is str
AssertionError
Easily reproducible via, e.g. `bzr log bzr+ssh:
Related branches
Changed in bzr: | |
assignee: | nobody → lifeless |
status: | New → Triaged |
Changed in bzr: | |
importance: | Undecided → Medium |
status: | Triaged → Confirmed |
status: | Confirmed → Triaged |
Changed in bzr: | |
status: | Fix Committed → Fix Released |
Changed in bzr: | |
status: | Confirmed → Fix Released |
To post a comment you must log in.
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:
"Key type is %s not a plain string" % (type(key),)
for key in keys:
assert type(key) is str, \
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:
mainline_ revs.insert( 0, None)
mainline_ revs.insert( 0, which_revs[ start_revno- 2][1])
# 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:
else:
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):
...
revision_ ids = mainline_revs[1:]
revision_ ids.reverse( ) nos[revision_ id]), 0
if include_merges is False:
if direction == 'reverse':
for revision_id in revision_ids:
yield revision_id, str(rev_
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... ).
self. _mainline_ revisions = list(mainline_ revisions)
self. _stop_revision = self._mainline_ revisions[ 0]
And 'merge_sort' seems designed to stop at the first revision in the list:
So I think the real fix is just:
graph. iter_ancestry( mainline_ revs[1: ]) if value is not None))
parent_map = dict(((key, value) for key, value in
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.