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:42:20 GMT 2008


At lp:~vila/bzr/300055-log-forward

------------------------------------------------------------
revno: 3847
revision-id: v.ladeuil+lp at free.fr-20081121164218-ritervlawtqbo6e0
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:42:18 +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:42:18 +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:42:18 +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:42:18 +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,40 @@
                          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):
+            import pdb; pdb.set_trace()
+            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 +948,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 +1002,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