loggerhead

Merge lp:~mwhudson/loggerhead/bug-390029 into lp:loggerhead

  1. bug-390029
  2. Merge into trunk-rich
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: j.c.sackett
Approved revision: 462
Merged at revision: 464
Proposed branch: lp:~mwhudson/loggerhead/bug-390029
Merge into: lp:loggerhead
Diff against target: 13 lines (+2/-1)
1 file modified
loggerhead/history.py (+2/-1)
To merge this branch: bzr merge lp:~mwhudson/loggerhead/bug-390029
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Richard Harding (community) code* Approve
Review via email: mp+87878@code.launchpad.net

Description of the change

This change should fix the bug, but the strange thing is that I can't reproduce it locally. It still occurs on LP though -- has something changed about whether revision_trees([revid, revid]) returns one or two trees in a recent bzr? If so, the bug will fix itself, but maybe we should apply this anyway.

To post a comment you must log in.
Revision history for this message
Richard Harding (rharding) wrote :

Michael, in peeking at the code it seems like you'd be able to test this by creating a test like the "test_native" that sets up the linear history and then grabs the top revision off of it and feeds that to the history.get_file_changes. This might require cheating and turning the revision into an entry as in _change_from_revision, but then you'd be able to test the corner case of the revision compared to itself?

The change here makes sense, but I just want to make sure that there's not a good way to test before approving.

Thanks

review: Needs Information (code*)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Sure, creating the scenario that results in an OOPS locally is simple enough -- the thing is, it doesn't cause an error for me! Poking around and asking people suggests that bzrlib has changed behaviour here, so we can either (a) just wait until the new bzrlib is deployed on LP or (b) apply the simple patch. I'd favour (b), friendlier to people who have not upgraded bzr yet for whatever reason.

Revision history for this message
Richard Harding (rharding) wrote :

Ok, thanks for the details.

review: Approve (code*)
Revision history for this message
j.c.sackett (jcsackett) wrote :

This looks fine, I have nothing to add.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'loggerhead/history.py'
2--- loggerhead/history.py 2011-11-23 08:29:50 +0000
3+++ loggerhead/history.py 2012-01-08 22:08:24 +0000
4@@ -789,7 +789,8 @@
5 """
6 repo = self._branch.repository
7 if (bzrlib.revision.is_null(old_revid) or
8- bzrlib.revision.is_null(new_revid)):
9+ bzrlib.revision.is_null(new_revid) or
10+ old_revid == new_revid):
11 old_tree, new_tree = map(
12 repo.revision_tree, [old_revid, new_revid])
13 else:

Subscribers

People subscribed via source and target branches