Merge lp:~vila/bzr/575631-exclude-ancestry into lp:bzr

[Vincent Ladeuil]

``bzr log --exclude-common-ancestry`` works for linear ancestries.

log has too many code paths, some were forgotten when --exclude-ancestry was implemented.

This proposal fixes the cases where simple revnos are used.

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/575631-exclude-ancestry into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #575631 "bzr log --exclude-common-ancestry -rX..Y" inconsistent
> https://bugs.launchpad.net/bugs/575631
>
>
> log has too many code paths, some were forgotten when --exclude-ancestry was implemented.
>
> This proposal fixes the cases where simple revnos are used.
>

 merge: approve

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwQ3KQACgkQJdeBCYSNAAPWvQCfbP/TRBO5oR2B5DJdg1NEZVf8
TZ8AoLuU7aoXU8Pavrs9abBrEBOVF1X6
=IB3i
-----END PGP SIGNATURE-----

review: Approve

=== modified file 'NEWS'
--- NEWS	2010-06-07 02:38:04 +0000
+++ NEWS	2010-06-10 07:12:29 +0000
@@ -24,10 +24,17 @@
 Bug Fixes
 *********
 
+<<<<<<< TREE
 * ``bzr init`` does not recursively scan directory contents anymore
   leading to faster init for directories with existing content.
   (Martin [gz], Parth Malwankar, #501307)
 
+=======
+* ``bzr log --exclude-common-ancestry`` is now taken into account for
+  linear ancetries.
+  (Vincent Ladeuil, #575631)
+
+>>>>>>> MERGE-SOURCE
 * Final fix for 'no help for command' issue. We now show a clean message
   when a command has no help, document how to set help more clearly, and
   test that all commands available to the test suite have help.

=== modified file 'bzrlib/log.py'
--- bzrlib/log.py	2010-05-11 08:44:59 +0000
+++ bzrlib/log.py	2010-06-10 07:12:29 +0000
@@ -522,7 +522,7 @@
     elif not generate_merge_revisions:
         # If we only want to see linear revisions, we can iterate ...
         iter_revs = _generate_flat_revisions(branch, start_rev_id, end_rev_id,
-                                             direction)
+                                             direction, exclude_common_ancestry)
         if direction == 'forward':
             iter_revs = reversed(iter_revs)
     else:
@@ -544,8 +544,11 @@
         return [(rev_id, revno_str, 0)]
 
 
-def _generate_flat_revisions(branch, start_rev_id, end_rev_id, direction):
-    result = _linear_view_revisions(branch, start_rev_id, end_rev_id)
+def _generate_flat_revisions(branch, start_rev_id, end_rev_id, direction,
+                             exclude_common_ancestry=False):
+    result = _linear_view_revisions(
+        branch, start_rev_id, end_rev_id,
+        exclude_common_ancestry=exclude_common_ancestry)
     # If a start limit was given and it's not obviously an
     # ancestor of the end limit, check it before outputting anything
     if direction == 'forward' or (start_rev_id
@@ -572,7 +575,7 @@
     if delayed_graph_generation:
         try:
             for rev_id, revno, depth in  _linear_view_revisions(
-                branch, start_rev_id, end_rev_id):
+                branch, start_rev_id, end_rev_id, exclude_common_ancestry):
                 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
@@ -643,14 +646,17 @@
     return True
 
 
-def _linear_view_revisions(branch, start_rev_id, end_rev_id):
+def _linear_view_revisions(branch, start_rev_id, end_rev_id,
+                           exclude_common_ancestry=False):
     """Calculate a sequence of revisions to view, newest to oldest.
 
     :param start_rev_id: the lower revision-id
     :param end_rev_id: the upper revision-id
+    :param exclude_common_ancestry: Whether the start_rev_id should be part of
+        the iterated revisions.
     :return: An iterator of (revision_id, dotted_revno, merge_depth) tuples.
     :raises _StartNotLinearAncestor: if a start_rev_id is specified but
-      is not found walking the left-hand history
+        is not found walking the left-hand history
     """
     br_revno, br_rev_id = branch.last_revision_info()
     repo = branch.repository
@@ -667,7 +673,8 @@
             revno = branch.revision_id_to_dotted_revno(revision_id)
             revno_str = '.'.join(str(n) for n in revno)
             if not found_start and revision_id == start_rev_id:
-                yield revision_id, revno_str, 0
+                if not exclude_common_ancestry:
+                    yield revision_id, revno_str, 0
                 found_start = True
                 break
             else:

=== modified file 'bzrlib/tests/blackbox/test_log.py'
--- bzrlib/tests/blackbox/test_log.py	2010-04-14 12:30:17 +0000
+++ bzrlib/tests/blackbox/test_log.py	2010-06-10 07:12:29 +0000
@@ -159,6 +159,14 @@
         self.assertLogRevnos(['-c1'], ['1'])
 
 
+class TestLogExcludeCommonAncestry(TestLogWithLogCatcher):
+
+    def test_exclude_common_ancestry_simple_revnos(self):
+        self.make_linear_branch()
+        self.assertLogRevnos(['-r1..3', '--exclude-common-ancestry'],
+                             ['3', '2'])
+
+
 class TestLogMergedLinearAncestry(TestLogWithLogCatcher):
 
     def setUp(self):
@@ -167,6 +175,18 @@
         # stop calling run_bzr, there is no point) --vila 100118.
         builder = branchbuilder.BranchBuilder(self.get_transport())
         builder.start_series()
+        # 1
+        # | \
+        # 2  1.1.1
+        # | / |
+        # 3  1.1.2
+        # |   |
+        # |  1.1.3
+        # | / |
+        # 4  1.1.4
+        # | /
+        # 5
+
         # mainline
         builder.build_snapshot('1', None, [
             ('add', ('', 'root-id', 'directory', ''))])

=== modified file 'bzrlib/tests/test_log.py'
--- bzrlib/tests/test_log.py	2010-05-05 08:18:32 +0000
+++ bzrlib/tests/test_log.py	2010-06-10 07:12:29 +0000
@@ -1724,13 +1724,13 @@
         return br
 
     def assertLogRevnos(self, expected_revnos, b, start, end,
-                        exclude_common_ancestry):
+                        exclude_common_ancestry, generate_merge_revisions=True):
         # FIXME: the layering in log makes it hard to test intermediate levels,
         # I wish adding filters with their parameters were easier...
         # -- vila 20100413
         iter_revs = log._calc_view_revisions(
             b, start, end, direction='reverse',
-            generate_merge_revisions=True,
+            generate_merge_revisions=generate_merge_revisions,
             exclude_common_ancestry=exclude_common_ancestry)
         self.assertEqual(expected_revnos,
                          [revid for revid, revno, depth in iter_revs])
@@ -1738,11 +1738,19 @@
     def test_merge_sorted_exclude_ancestry(self):
         b = self.make_branch_with_alternate_ancestries()
         self.assertLogRevnos(['3', '1.1.2', '1.2.1', '1.1.1', '2', '1'],
-                             b, '1', '3', False)
+                             b, '1', '3', exclude_common_ancestry=False)
         # '2' is part of the '3' ancestry but not part of '1.1.1' ancestry so
         # it should be mentioned even if merge_sort order will make it appear
         # after 1.1.1
         self.assertLogRevnos(['3', '1.1.2', '1.2.1', '2'],
-                             b, '1.1.1', '3', True)
+                             b, '1.1.1', '3', exclude_common_ancestry=True)
 
+    def test_merge_sorted_simple_revnos_exclude_ancestry(self):
+        b = self.make_branch_with_alternate_ancestries()
+        self.assertLogRevnos(['3', '2'],
+                             b, '1', '3', exclude_common_ancestry=True,
+                             generate_merge_revisions=False)
+        self.assertLogRevnos(['3', '1.1.2', '1.2.1', '1.1.1', '2'],
+                             b, '1', '3', exclude_common_ancestry=True,
+                             generate_merge_revisions=True)