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