Rev 4979: Check ancestry so we don't output random revisions. in file:///home/vila/src/bzr/bugs/476293-log-check-ancestor/

Vincent Ladeuil v.ladeuil+lp at free.fr
Mon Feb 1 16:26:37 GMT 2010


At file:///home/vila/src/bzr/bugs/476293-log-check-ancestor/

------------------------------------------------------------
revno: 4979
revision-id: v.ladeuil+lp at free.fr-20100201162637-um0ei9j2yktu6zob
parent: v.ladeuil+lp at free.fr-20100127082621-dsmoag3f8nwca2qu
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: respect-direction
timestamp: Mon 2010-02-01 17:26:37 +0100
message:
  Check ancestry so we don't output random revisions.
  
  * bzrlib/tests/blackbox/test_log.py:
  (TestBug476293): Reproduce the bug.
  
  * bzrlib/log.py:
  (_generate_all_revisions): If merged revisions are involved, the
  ancestor check should be explicit.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2010-01-22 15:18:39 +0000
+++ b/NEWS	2010-02-01 16:26:37 +0000
@@ -20,6 +20,9 @@
 Bug Fixes
 *********
 
+* Fix ``log`` to better check ancestors even if merged revisions are involved.
+  (Vincent Ladeuil, #476293)
+
 Improvements
 ************
 

=== modified file 'bzrlib/log.py'
--- a/bzrlib/log.py	2010-01-23 11:37:40 +0000
+++ b/bzrlib/log.py	2010-02-01 16:26:37 +0000
@@ -534,16 +534,30 @@
 
 
 def _generate_all_revisions(branch, start_rev_id, end_rev_id, direction,
-    delayed_graph_generation):
+                            delayed_graph_generation):
     # On large trees, generating the merge graph can take 30-60 seconds
     # so we delay doing it until a merge is detected, incrementally
-    # returning initial (non-merge) revisions while we can.
+    # returning initial (non-merge) revisions while we can. 
+
+    # The above is only true for old formats (<= 0.92), for newer formats, a
+    # couple of seconds only should be needed to load the whole graph and the
+    # other graph operations needed are even faster than that -- vila 100201
     initial_revisions = []
     if delayed_graph_generation:
         try:
-            for rev_id, revno, depth in \
-                _linear_view_revisions(branch, start_rev_id, end_rev_id):
+            for rev_id, revno, depth in  _linear_view_revisions(
+                branch, start_rev_id, end_rev_id):
                 if _has_merges(branch, rev_id):
+                    # The end_rev_id can be nested down somewhere. We need an
+                    # explicit ancestry check. There is an ambiguity here as we
+                    # may not raise _StartNotLinearAncestor for a revision that
+                    # is an ancestor but not a *linear* one. But since we have
+                    # loaded the graph to do the check (or calculate a dotted
+                    # revno), we may as well accept to show the log... 
+                    # -- vila 100201
+                    graph = branch.repository.get_graph()
+                    if not graph.is_ancestor(start_rev_id, end_rev_id):
+                        raise _StartNotLinearAncestor()
                     end_rev_id = rev_id
                     break
                 else:

=== modified file 'bzrlib/tests/blackbox/test_log.py'
--- a/bzrlib/tests/blackbox/test_log.py	2010-01-25 17:48:22 +0000
+++ b/bzrlib/tests/blackbox/test_log.py	2010-02-01 16:26:37 +0000
@@ -23,6 +23,7 @@
 
 from bzrlib import (
     branchbuilder,
+    errors,
     log,
     osutils,
     tests,
@@ -202,6 +203,50 @@
                              ['1.1.1', '1.1.2', '1.1.3', '1.1.4'])
 
 
+class TestBug476293(TestLogWithLogCatcher):
+
+    def make_tagged_branch(self, path='.', format=None):
+        builder = branchbuilder.BranchBuilder(self.get_transport())
+        builder.start_series()
+        # The graph below may look a bit complicated (and it may be but I've
+        # banged my head enough on it) but the bug requires at least dotted
+        # revnos *and* merged revisions below that.
+        builder.build_snapshot('1', None, [
+            ('add', ('', 'root-id', 'directory', ''))])
+        builder.build_snapshot('2', ['1'], [])
+        builder.build_snapshot('1.1.1', ['1'], [])
+        builder.build_snapshot('2.1.1', ['2'], [])
+        builder.build_snapshot('3', ['2', '1.1.1'], [])
+        builder.build_snapshot('2.1.2', ['2.1.1'], [])
+        builder.build_snapshot('2.2.1', ['2.1.1'], [])
+        builder.build_snapshot('2.1.3', ['2.1.2', '2.2.1'], [])
+        builder.build_snapshot('4', ['3', '2.1.3'], [])
+        builder.build_snapshot('5', ['4', '2.1.2'], [])
+        builder.finish_series()
+        tags = builder.get_branch().tags
+        return builder
+
+    def test_not_an_ancestor(self):
+        builder = self.make_tagged_branch()
+        b = builder.get_branch()
+        b.lock_read()
+        self.addCleanup(b.unlock)
+        self.assertRaises(errors.BzrCommandError,
+                          log._generate_all_revisions,
+                          b, '1.1.1', '2.1.3', 'reverse',
+                          delayed_graph_generation=True)
+
+    def test_wrong_order(self):
+        builder = self.make_tagged_branch()
+        b = builder.get_branch()
+        b.lock_read()
+        self.addCleanup(b.unlock)
+        self.assertRaises(errors.BzrCommandError,
+                          log._generate_all_revisions,
+                          b, '5', '2.1.3', 'reverse',
+                          delayed_graph_generation=True)
+
+
 class TestLogRevSpecsWithPaths(TestLogWithLogCatcher):
 
     def test_log_revno_n_path_wrong_namespace(self):
@@ -210,7 +255,6 @@
         # There is no guarantee that a path exist between two arbitrary
         # revisions.
         self.run_bzr("log -r revno:2:branch1..revno:3:branch2", retcode=3)
-        # But may be it's worth trying though ? -- vila 100115
 
     def test_log_revno_n_path_correct_order(self):
         self.make_linear_branch('branch2')



More information about the bazaar-commits mailing list