Rev 3847: Better fix for bug #300055. in lp:~vila/bzr/300055-log-forward
Vincent Ladeuil
v.ladeuil+lp at free.fr
Fri Nov 21 16:43:55 GMT 2008
At lp:~vila/bzr/300055-log-forward
------------------------------------------------------------
revno: 3847
revision-id: v.ladeuil+lp at free.fr-20081121164353-8d07go33ycibzbwl
parent: v.ladeuil+lp at free.fr-20081121114918-lekth2kvpncyiaqz
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 300055-log-forward
timestamp: Fri 2008-11-21 17:43:53 +0100
message:
Better fix for bug #300055.
* bzrlib/tests/test_log.py:
(TestGetViewRevisions.make_tree_with_many_merges): Hijack the
helper to test for revisions touching file on a more significant
tree.
(TestGetViewRevisions.test_file_id_for_range): Better test to
highlight bug #300055: starting revision is a dotted revno and the
log should start right there, not at the mainline revision where
merging occured. But that uncovers yet another bug...
(TestGetRevisionsTouchingFileID.assertAllRevisionsForFileID):
_filter_revisions_touching_file_id doesn't have a 'direction'
parameter anymore.
* bzrlib/tests/blackbox/test_log.py:
(TestCaseWithoutPropsHandler): Fix line too long.
(TestLog.test_log_with_tags,
TestLogMerges.test_merges_partial_range): Fix whitespaces.
* bzrlib/log.py:
(calculate_view_revisions): Delete gratuitous split line. Push
direction handling closer to the needed point. Delete 'direction'
parameter when calling _filter_revisions_touching_file_id.
(_filter_revisions_touching_file_id): Delete 'direction'
parameter. This was used for calling reverse_by_depth which can't
handle an already reversed list.
-------------- next part --------------
=== modified file 'bzrlib/log.py'
--- a/bzrlib/log.py 2008-11-21 11:49:18 +0000
+++ b/bzrlib/log.py 2008-11-21 16:43:53 +0000
@@ -242,14 +242,11 @@
and specific_fileid is None):
return _linear_view_revisions(branch)
- mainline_revs, rev_nos, start_rev_id, end_rev_id = \
- _get_mainline_revs(branch, start_revision, end_revision)
+ mainline_revs, rev_nos, start_rev_id, end_rev_id = _get_mainline_revs(
+ branch, start_revision, end_revision)
if not mainline_revs:
return []
- if direction == 'reverse':
- start_rev_id, end_rev_id = end_rev_id, start_rev_id
-
generate_single_revision = False
if ((not generate_merge_revisions)
and ((start_rev_id and (start_rev_id not in rev_nos))
@@ -262,6 +259,9 @@
generate_merge_revisions = generate_single_revision
view_revs_iter = get_view_revisions(mainline_revs, rev_nos, branch,
direction, include_merges=generate_merge_revisions)
+
+ if direction == 'reverse':
+ start_rev_id, end_rev_id = end_rev_id, start_rev_id
view_revisions = _filter_revision_range(list(view_revs_iter),
start_rev_id,
end_rev_id)
@@ -269,9 +269,8 @@
view_revisions = view_revisions[0:1]
if specific_fileid:
view_revisions = _filter_revisions_touching_file_id(branch,
- specific_fileid,
- view_revisions,
- direction)
+ specific_fileid,
+ view_revisions)
# rebase merge_depth - unless there are no revisions or
# either the first or last revision have merge_depth = 0.
@@ -538,8 +537,7 @@
return view_revisions
-def _filter_revisions_touching_file_id(branch, file_id, view_revisions,
- direction):
+def _filter_revisions_touching_file_id(branch, file_id, view_revisions):
r"""Return the list of revision ids which touch a given file id.
The function filters view_revisions and returns a subset.
@@ -564,13 +562,14 @@
This will also be restricted based on a subset of the mainline.
:param branch: The branch where we can get text revision information.
+
:param file_id: Filter out revisions that do not touch file_id.
+
:param view_revisions: A list of (revision_id, dotted_revno, merge_depth)
tuples. This is the list of revisions which will be filtered. It is
- assumed that view_revisions is in merge_sort order (either forward or
- reverse).
- :param direction: The direction of view_revisions. See also
- reverse_by_depth, and get_view_revisions
+ assumed that view_revisions is in merge_sort order (i.e. newest
+ revision first ).
+
:return: A list of (revision_id, dotted_revno, merge_depth) tuples.
"""
# Lookup all possible text keys to determine which ones actually modified
@@ -594,11 +593,6 @@
del text_keys, next_keys
result = []
- if direction == 'forward':
- # TODO: The algorithm for finding 'merges' of file changes expects
- # 'reverse' order (the default from 'merge_sort()'). Instead of
- # forcing this, we could just use the reverse_by_depth order.
- view_revisions = reverse_by_depth(view_revisions)
# Track what revisions will merge the current revision, replace entries
# with 'None' when they have been added to result
current_merge_stack = [None]
@@ -617,8 +611,6 @@
if node is not None:
result.append(node)
current_merge_stack[idx] = None
- if direction == 'forward':
- result = reverse_by_depth(result)
return result
=== modified file 'bzrlib/tests/blackbox/test_log.py'
--- a/bzrlib/tests/blackbox/test_log.py 2008-10-01 05:40:45 +0000
+++ b/bzrlib/tests/blackbox/test_log.py 2008-11-21 16:43:53 +0000
@@ -29,7 +29,8 @@
from bzrlib.tests import test_log
-class TestCaseWithoutPropsHandler(ExternalBase, test_log.TestCaseWithoutPropsHandler):
+class TestCaseWithoutPropsHandler(ExternalBase,
+ test_log.TestCaseWithoutPropsHandler):
pass
@@ -170,8 +171,8 @@
branch = tree.branch
branch.tags.set_tag('tag1', branch.get_rev_id(1))
branch.tags.set_tag('tag1.1', branch.get_rev_id(1))
- branch.tags.set_tag('tag3', branch.last_revision())
-
+ branch.tags.set_tag('tag3', branch.last_revision())
+
log = self.run_bzr("log -r-1")[0]
self.assertTrue('tags: tag3' in log)
@@ -302,7 +303,7 @@
def test_merges_partial_range(self):
self._prepare()
- out,err = self.run_bzr('log -r1.1.1..1.1.2')
+ out, err = self.run_bzr('log -r1.1.1..1.1.2')
self.assertEqual('', err)
log = normalize_log(out)
self.assertEqualDiff(log, """\
@@ -339,7 +340,7 @@
out,err = self.run_bzr('log --short -r1.1.1..1.1.2', retcode=3)
self.assertContainsRe(err, err_msg)
-
+
class TestLogEncodings(TestCaseInTempDir):
_mu = u'\xb5'
=== modified file 'bzrlib/tests/test_log.py'
--- a/bzrlib/tests/test_log.py 2008-11-21 11:39:43 +0000
+++ b/bzrlib/tests/test_log.py 2008-11-21 16:43:53 +0000
@@ -744,18 +744,26 @@
def make_tree_with_many_merges(self):
"""Create a tree with well-known revision ids"""
wt = self.make_branch_and_tree('tree1')
+ self.build_tree_contents([('tree1/f', '1\n')])
+ wt.add(['f'], ['f-id'])
wt.commit('commit one', rev_id='1')
wt.commit('commit two', rev_id='2')
+
tree3 = wt.bzrdir.sprout('tree3').open_workingtree()
+ self.build_tree_contents([('tree3/f', '1\n2\n3a\n')])
tree3.commit('commit three a', rev_id='3a')
+
tree2 = wt.bzrdir.sprout('tree2').open_workingtree()
tree2.merge_from_branch(tree3.branch)
tree2.commit('commit three b', rev_id='3b')
+
wt.merge_from_branch(tree2.branch)
wt.commit('commit three c', rev_id='3c')
tree2.commit('four-a', rev_id='4a')
+
wt.merge_from_branch(tree2.branch)
wt.commit('four-b', rev_id='4b')
+
mainline_revs = [None, '1', '2', '3c', '4b']
rev_nos = {'1':1, '2':2, '3c': 3, '4b':4}
full_rev_nos_for_reference = {
@@ -850,6 +858,39 @@
revisions)
+ def test_file_id_for_range(self):
+ mainline_revs, rev_nos, wt = self.make_tree_with_many_merges()
+ wt.lock_read()
+ self.addCleanup(wt.unlock)
+
+ def rev_from_rev_id(revid, branch):
+ revspec = revisionspec.RevisionSpec.from_string('revid:%s' % revid)
+ return revspec.in_history(branch)
+
+ def view_revs(start_rev, end_rev, file_id, direction):
+ revs = log.calculate_view_revisions(
+ wt.branch,
+ start_rev, # start_revision
+ end_rev, # end_revision
+ direction, # direction
+ file_id, # specific_fileid
+ True, # generate_merge_revisions
+ True, # allow_single_merge_revision
+ )
+ return revs
+
+ rev_3a = rev_from_rev_id('3a', wt.branch)
+ rev_4b = rev_from_rev_id('4b', wt.branch)
+ self.assertEquals([('3c', '3', 0), ('3a', '2.1.1', 1)],
+ view_revs(rev_3a, rev_4b, 'f-id', 'reverse'))
+ # Note that the depth is 0 for 3a because depths are normalized, but
+ # there is still a bug somewhere... most probably in
+ # _filter_revision_range and/or get_view_revisions still around a bad
+ # use of reverse_by_depth
+ self.assertEquals([('3a', '2.1.1', 0)],
+ view_revs(rev_3a, rev_4b, 'f-id', 'forward'))
+
+
class TestGetRevisionsTouchingFileID(tests.TestCaseWithTransport):
def create_tree_with_single_merge(self):
@@ -906,7 +947,7 @@
tree.commit('D', rev_id='D')
# Switch to a read lock for this tree.
- # We still have an addCleanup(unlock) pending
+ # We still have an addCleanup(tree.unlock) pending
tree.unlock()
tree.lock_read()
return tree
@@ -960,8 +1001,7 @@
actual_revs = log._filter_revisions_touching_file_id(
tree.branch,
file_id,
- list(view_revs_iter),
- 'reverse')
+ list(view_revs_iter))
self.assertEqual(revisions, [r for r, revno, depth in actual_revs])
def test_file_id_f1(self):
More information about the bazaar-commits
mailing list