Merge lp:~jelmer/bzr/get-known-graph-ancestry into lp:bzr

[Jelmer Vernooij]

This patch adds Repository.get_known_graph_ancestry().

This method is necessary because not all repositories have VersionedFiles implementations; several things that already rely on VF.get_known_ancestry_graph() (such as qdiff) break with bzr-svn at the moment.

[Vincent Ladeuil (vila)]

Sounds good to me, the tests are light and appropriate as smoke ones which is all that is needed to start.

We may want the same thing for file revisions.

review: Approve

[Robert Collins (lifeless)]

On Sat, 2010-04-10 at 01:26 +0000, Jelmer Vernooij wrote:
> Jelmer Vernooij has proposed merging lp:~jelmer/bzr/get-known-graph-ancestry into lp:bzr.

Just to say, I've got reservations about this - we want to shrink
Repository IMO, not expand it. Why can't bzr-svn just expose a VF object
- even if its slow if used for untuned things, it would work, wouldn't
it? And avoid the duplication that this adds.

 review: disapprove

review: Disapprove

[Jelmer Vernooij (jelmer)]

On Sun, 2010-04-11 at 02:48 +0000, Robert Collins wrote:
> Review: Disapprove
> On Sat, 2010-04-10 at 01:26 +0000, Jelmer Vernooij wrote:
> > Jelmer Vernooij has proposed merging lp:~jelmer/bzr/get-known-graph-ancestry into lp:bzr.
>
> Just to say, I've got reservations about this - we want to shrink
> Repository IMO, not expand it. Why can't bzr-svn just expose a VF object
> - even if its slow if used for untuned things, it would work, wouldn't
> it? And avoid the duplication that this adds.
I agree that we should reduce the number of methods on Repository in
general, and as you might have seen I have helped remove some methods
earlier. However, I think VersionedFiles is the wrong interface here and
putting repository methods on it only because Repository already has a
lot of methods is wrong imho.

get_known_graph_ancestry() also returns a helper object itself that can
be used for further extension.

Cheers,

Jelmer

[Martin Pool (mbp)]

On 12 April 2010 00:54, Jelmer Vernooij <email address hidden> wrote:
>> Just to say, I've got reservations about this - we want to shrink
>> Repository IMO, not expand it. Why can't bzr-svn just expose a VF object
>> - even if its slow if used for untuned things, it would work, wouldn't
>> it? And avoid the duplication that this adds.
> I agree that we should reduce the number of methods on Repository in
> general, and as you might have seen I have helped remove some methods
> earlier. However, I think VersionedFiles is the wrong interface here and
> putting repository methods on it only because Repository already has a
> lot of methods is wrong imho.

I agree.

--
Martin <http://launchpad.net/~mbp/>

=== modified file 'NEWS'
--- NEWS	2010-04-08 08:48:59 +0000
+++ NEWS	2010-04-10 01:26:17 +0000
@@ -284,6 +284,9 @@
 * New method ``BzrDir.list_branches()`` that returns a sequence of branches 
   present in a control directory. (Jelmer Vernooij)
 
+* New method ``Repository.get_known_graph_ancestry()``. 
+  (Jelmer Vernooij, #495502)
+
 * New transport methods ``readlink``, ``symlink`` and ``hardlink``.
   (Neil Santos)
 

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2010-04-02 15:06:55 +0000
+++ bzrlib/branch.py	2010-04-10 01:26:17 +0000
@@ -447,11 +447,10 @@
         # start_revision_id.
         if self._merge_sorted_revisions_cache is None:
             last_revision = self.last_revision()
-            last_key = (last_revision,)
-            known_graph = self.repository.revisions.get_known_graph_ancestry(
-                [last_key])
+            known_graph = self.repository.get_known_graph_ancestry(
+                [last_revision])
             self._merge_sorted_revisions_cache = known_graph.merge_sort(
-                last_key)
+                last_revision)
         filtered = self._filter_merge_sorted_revisions(
             self._merge_sorted_revisions_cache, start_revision_id,
             stop_revision_id, stop_rule)

=== modified file 'bzrlib/fetch.py'
--- bzrlib/fetch.py	2010-02-23 07:43:11 +0000
+++ bzrlib/fetch.py	2010-04-10 01:26:17 +0000
@@ -28,8 +28,6 @@
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
 from bzrlib import (
-    graph as _mod_graph,
-    static_tuple,
     tsort,
     versionedfile,
     )
@@ -249,7 +247,7 @@
         if len(revs) > 100:
             # XXX: not covered by tests, should have a flag to always run
             # this. -- mbp 20100129
-            graph = _get_rich_root_heads_graph(self.source, revs)
+            graph = self.source_repo.get_known_graph_ancestry(revs)
         new_roots_stream = _new_root_data_stream(
             root_id_order, rev_id_to_root_id, parent_map, self.source, graph)
         return [('texts', new_roots_stream)]
@@ -257,11 +255,7 @@
 
 def _get_rich_root_heads_graph(source_repo, revision_ids):
     """Get a Graph object suitable for asking heads() for new rich roots."""
-    st = static_tuple.StaticTuple
-    revision_keys = [st(r_id).intern() for r_id in revision_ids]
-    known_graph = source_repo.revisions.get_known_graph_ancestry(
-                    revision_keys)
-    return _mod_graph.GraphThunkIdsToKeys(known_graph)
+    return 
 
 
 def _new_root_data_stream(

=== modified file 'bzrlib/graph.py'
--- bzrlib/graph.py	2010-02-17 17:11:16 +0000
+++ bzrlib/graph.py	2010-04-10 01:26:17 +0000
@@ -1685,12 +1685,18 @@
     def __init__(self, graph):
         self._graph = graph
 
+    def topo_sort(self):
+        return [r for (r,) in self._graph.topo_sort()]
+
     def heads(self, ids):
         """See Graph.heads()"""
         as_keys = [(i,) for i in ids]
         head_keys = self._graph.heads(as_keys)
         return set([h[0] for h in head_keys])
 
+    def merge_sort(self, tip_revision):
+        return self._graph.merge_sort((tip_revision,))
+
 
 _counters = [0,0,0,0,0,0,0]
 try:

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2010-03-11 18:00:23 +0000
+++ bzrlib/remote.py	2010-04-10 01:26:17 +0000
@@ -29,6 +29,7 @@
     repository,
     revision,
     revision as _mod_revision,
+    static_tuple,
     symbol_versioning,
 )
 from bzrlib.branch import BranchReferenceFormat
@@ -903,6 +904,15 @@
         parents_provider = self._make_parents_provider(other_repository)
         return graph.Graph(parents_provider)
 
+    @needs_read_lock
+    def get_known_graph_ancestry(self, revision_ids):
+        """Return the known graph for a set of revision ids and their ancestors.
+        """
+        st = static_tuple.StaticTuple
+        revision_keys = [st(r_id).intern() for r_id in revision_ids]
+        known_graph = self.revisions.get_known_graph_ancestry(revision_keys)
+        return graph.GraphThunkIdsToKeys(known_graph)
+
     def gather_stats(self, revid=None, committers=None):
         """See Repository.gather_stats()."""
         path = self.bzrdir._path_for_remote_call(self._client)

=== modified file 'bzrlib/repository.py'
--- bzrlib/repository.py	2010-03-26 10:25:47 +0000
+++ bzrlib/repository.py	2010-04-10 01:26:17 +0000
@@ -40,6 +40,7 @@
     lru_cache,
     osutils,
     revision as _mod_revision,
+    static_tuple,
     symbol_versioning,
     trace,
     tsort,
@@ -2626,6 +2627,15 @@
     def _make_parents_provider(self):
         return self
 
+    @needs_read_lock
+    def get_known_graph_ancestry(self, revision_ids):
+        """Return the known graph for a set of revision ids and their ancestors.
+        """
+        st = static_tuple.StaticTuple
+        revision_keys = [st(r_id).intern() for r_id in revision_ids]
+        known_graph = self.revisions.get_known_graph_ancestry(revision_keys)
+        return graph.GraphThunkIdsToKeys(known_graph)
+
     def get_graph(self, other_repository=None):
         """Return the graph walker for this repository format"""
         parents_provider = self._make_parents_provider()

=== modified file 'bzrlib/tests/per_repository/test_repository.py'
--- bzrlib/tests/per_repository/test_repository.py	2010-02-23 07:43:11 +0000
+++ bzrlib/tests/per_repository/test_repository.py	2010-04-10 01:26:17 +0000
@@ -724,6 +724,25 @@
         self.assertTrue('ghost' not in parents)
         self.assertEqual(parents['rev2'], ('rev1', 'ghost'))
 
+    def test_get_known_graph_ancestry(self):
+        tree = self.make_branch_and_tree('here')
+        tree.lock_write()
+        self.addCleanup(tree.unlock)
+        # A
+        # |\
+        # | B
+        # |/
+        # C
+        tree.commit('initial commit', rev_id='A')
+        tree_other = tree.bzrdir.sprout('there').open_workingtree()
+        tree_other.commit('another', rev_id='B')
+        tree.merge_from_branch(tree_other.branch)
+        tree.commit('another', rev_id='C')
+        kg = tree.branch.repository.get_known_graph_ancestry(
+            ['C'])
+        self.assertEqual(['C'], list(kg.heads(['A', 'B', 'C'])))
+        self.assertEqual(['A', 'B', 'C'], list(kg.topo_sort()))
+
     def test_parent_map_type(self):
         tree = self.make_branch_and_tree('here')
         tree.lock_write()