Merge lp:~garyvdm/bzr/PublicStackedParentsProvider into lp:~bzr/bzr/trunk-old

[Gary van der Merwe]

[Gary van der Merwe (garyvdm)]

The patch make StackedParentsProvider a public api. It also adds a test to check that overlaping revisions from the different providers are handled correctly.

[John A Meinel (jameinel)]

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

Gary van der Merwe wrote:
> Gary van der Merwe has proposed merging lp:~garyvdm/bzr/PublicStackedParentsProvider into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> The patch make StackedParentsProvider a public api. It also adds a test to check that overlaping revisions from the different providers are handled correctly.
>
     def __repr__(self):
- - return "_StackedParentsProvider(%r)" % self._parent_providers
+ return "StackedParentsProvider(%r)" % self._parent_providers

^- we generally try to do:

return "%s(%r)" % (self.__class__.__name__, self._parent_providers)

so that you don't have to update these things, and so that child classes
automatically get named the right thing, too.

We might want to add:

@deprecated((0,1, 16)
def _StackedParentsProvider(*args, **kwargs):
  return StackedParentsProvider(*args, **kwargs)

I realize we don't *need* to by our contract rules. But where we can, it
is a lot nicer to start giving deprecation warnings, rather than causing
existing code to just start breaking.

John
=:->

 Review: approve

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

iEYEARECAAYFAkoeZFMACgkQJdeBCYSNAANTBQCcCGnzpFlAqI74sq5veUieoEOP
5BgAn2Jk/pRDdA+Vh/2ZUDS35bRo8kiC
=12aX
-----END PGP SIGNATURE-----

[John A Meinel (jameinel)]

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

Gary van der Merwe wrote:
> Gary van der Merwe has proposed merging lp:~garyvdm/bzr/PublicStackedParentsProvider into lp:bzr.

The only thing I would mention is that we should add a single test that
exercises the deprecated function. So that we

1) Know that the deprecation warning is emitted
2) Know that the api still works.

This would be something like:
+
+ def test_stacked_parents_provider_overlapping(self):
+ # rev2 is availible in both providers.
+ # 1
+ # |
+ # 2
+ parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev1']})
+ parents2 = _mod_graph.DictParentsProvider({'rev2': ['rev1']})
+ stacked = self.callDeprecated(deprecated_in(1, 16, 0),
+ _mod_graph._StackedParentsProvider(
...
+ self.assertEqual({'rev2': ['rev1']},
+ stacked.get_parent_map(['rev2']))

  Review: approve

Sorry I forgot to mention that before, but with that change, I think we
should land it.

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

iEYEARECAAYFAkofnqwACgkQJdeBCYSNAAN7ggCgllDFqwDus82fkVKnXUsXHzMI
trAAn3aZTmfxX/LF0AoWOt8Ps6h6gKBu
=HegE
-----END PGP SIGNATURE-----

[John A Meinel (jameinel)]

review: Approve

[Andrew Bennetts (spiv)]

I'll merge.

review: Approve

=== modified file 'NEWS'
--- NEWS	2009-06-05 23:21:51 +0000
+++ NEWS	2009-06-06 02:36:15 +0000
@@ -102,6 +102,9 @@
 ***********
 
 * Added osutils.parent_directories(). (Ian Clatworthy)
+* ``graph.StackedParentsProvider`` is now a public API, replacing
+  ``graph._StackedParentsProvider``. The api is now considered stable and ready
+  for external users. (Gary van der Merwe)
 
 * TreeTransformBase no longer assumes that limbo is provided via disk.
   DiskTreeTransform now provides disk functionality.  (Aaron Bentley)

=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py	2009-06-04 21:25:46 +0000
+++ bzrlib/bzrdir.py	2009-06-06 02:36:15 +0000
@@ -2768,7 +2768,7 @@
         del ie.text_id
 
     def get_parent_map(self, revision_ids):
-        """See graph._StackedParentsProvider.get_parent_map"""
+        """See graph.StackedParentsProvider.get_parent_map"""
         return dict((revision_id, self.revisions[revision_id])
                     for revision_id in revision_ids
                      if revision_id in self.revisions)

=== modified file 'bzrlib/graph.py'
--- bzrlib/graph.py	2009-05-27 09:40:33 +0000
+++ bzrlib/graph.py	2009-06-06 02:36:15 +0000
@@ -20,10 +20,10 @@
     debug,
     errors,
     revision,
-    symbol_versioning,
     trace,
     tsort,
     )
+from bzrlib.symbol_versioning import deprecated_function, deprecated_in
 
 STEP_UNIQUE_SEARCHER_EVERY = 5
 
@@ -60,18 +60,25 @@
         return 'DictParentsProvider(%r)' % self.ancestry
 
     def get_parent_map(self, keys):
-        """See _StackedParentsProvider.get_parent_map"""
+        """See StackedParentsProvider.get_parent_map"""
         ancestry = self.ancestry
         return dict((k, ancestry[k]) for k in keys if k in ancestry)
 
-
-class _StackedParentsProvider(object):
-
+@deprecated_function(deprecated_in((1, 16, 0)))
+def _StackedParentsProvider(*args, **kwargs):
+    return StackedParentsProvider(*args, **kwargs)
+
+class StackedParentsProvider(object):
+    """A parents provider which stacks (or unions) multiple providers.
+    
+    The providers are queries in the order of the provided parent_providers.
+    """
+    
     def __init__(self, parent_providers):
         self._parent_providers = parent_providers
 
     def __repr__(self):
-        return "_StackedParentsProvider(%r)" % self._parent_providers
+        return "%s(%r)" % (self.__class__.__name__, self._parent_providers)
 
     def get_parent_map(self, keys):
         """Get a mapping of keys => parents
@@ -148,7 +155,7 @@
         return dict(self._cache)
 
     def get_parent_map(self, keys):
-        """See _StackedParentsProvider.get_parent_map."""
+        """See StackedParentsProvider.get_parent_map."""
         cache = self._cache
         if cache is None:
             cache = self._get_parent_map(keys)

=== modified file 'bzrlib/index.py'
--- bzrlib/index.py	2009-03-24 01:53:42 +0000
+++ bzrlib/index.py	2009-06-06 02:36:15 +0000
@@ -1192,7 +1192,7 @@
                 ', '.join(map(repr, self._indices)))
 
     def get_parent_map(self, keys):
-        """See graph._StackedParentsProvider.get_parent_map"""
+        """See graph.StackedParentsProvider.get_parent_map"""
         search_keys = set(keys)
         if NULL_REVISION in search_keys:
             search_keys.discard(NULL_REVISION)

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2009-06-03 01:37:27 +0000
+++ bzrlib/remote.py	2009-06-06 02:36:15 +0000
@@ -1576,7 +1576,7 @@
             providers.insert(0, other)
         providers.extend(r._make_parents_provider() for r in
                          self._fallback_repositories)
-        return graph._StackedParentsProvider(providers)
+        return graph.StackedParentsProvider(providers)
 
     def _serialise_search_recipe(self, recipe):
         """Serialise a graph search recipe.

=== modified file 'bzrlib/repofmt/knitrepo.py'
--- bzrlib/repofmt/knitrepo.py	2009-04-09 20:23:07 +0000
+++ bzrlib/repofmt/knitrepo.py	2009-06-06 02:36:15 +0000
@@ -54,7 +54,7 @@
         return 'KnitParentsProvider(%r)' % self._knit
 
     def get_parent_map(self, keys):
-        """See graph._StackedParentsProvider.get_parent_map"""
+        """See graph.StackedParentsProvider.get_parent_map"""
         parent_map = {}
         for revision_id in keys:
             if revision_id is None:
@@ -85,7 +85,7 @@
         return 'KnitsParentsProvider(%r)' % self._knit
 
     def get_parent_map(self, keys):
-        """See graph._StackedParentsProvider.get_parent_map"""
+        """See graph.StackedParentsProvider.get_parent_map"""
         parent_map = self._knit.get_parent_map(
             [self._prefix + (key,) for key in keys])
         result = {}

=== modified file 'bzrlib/repository.py'
--- bzrlib/repository.py	2009-06-04 21:25:46 +0000
+++ bzrlib/repository.py	2009-06-06 02:36:15 +0000
@@ -2384,7 +2384,7 @@
         return self.control_files.get_transaction()
 
     def get_parent_map(self, revision_ids):
-        """See graph._StackedParentsProvider.get_parent_map"""
+        """See graph.StackedParentsProvider.get_parent_map"""
         # revisions index works in keys; this just works in revisions
         # therefore wrap and unwrap
         query_keys = []
@@ -2413,7 +2413,7 @@
         parents_provider = self._make_parents_provider()
         if (other_repository is not None and
             not self.has_same_location(other_repository)):
-            parents_provider = graph._StackedParentsProvider(
+            parents_provider = graph.StackedParentsProvider(
                 [parents_provider, other_repository._make_parents_provider()])
         return graph.Graph(parents_provider)
 

=== modified file 'bzrlib/tests/test_graph.py'
--- bzrlib/tests/test_graph.py	2009-05-12 02:24:54 +0000
+++ bzrlib/tests/test_graph.py	2009-06-06 02:36:15 +0000
@@ -22,6 +22,7 @@
     )
 from bzrlib.revision import NULL_REVISION
 from bzrlib.tests import TestCaseWithMemoryTransport
+from bzrlib.symbol_versioning import deprecated_in 
 
 
 # Ancestry 1:
@@ -661,10 +662,36 @@
         self.assertEqual((set(['e']), set(['f', 'g'])),
                          graph.find_difference('e', 'f'))
 
+
     def test_stacked_parents_provider(self):
         parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev3']})
         parents2 = _mod_graph.DictParentsProvider({'rev1': ['rev4']})
-        stacked = _mod_graph._StackedParentsProvider([parents1, parents2])
+        stacked = _mod_graph.StackedParentsProvider([parents1, parents2])
+        self.assertEqual({'rev1':['rev4'], 'rev2':['rev3']},
+                         stacked.get_parent_map(['rev1', 'rev2']))
+        self.assertEqual({'rev2':['rev3'], 'rev1':['rev4']},
+                         stacked.get_parent_map(['rev2', 'rev1']))
+        self.assertEqual({'rev2':['rev3']},
+                         stacked.get_parent_map(['rev2', 'rev2']))
+        self.assertEqual({'rev1':['rev4']},
+                         stacked.get_parent_map(['rev1', 'rev1']))
+    
+    def test_stacked_parents_provider_overlapping(self):
+        # rev2 is availible in both providers.
+        # 1
+        # |
+        # 2
+        parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev1']})
+        parents2 = _mod_graph.DictParentsProvider({'rev2': ['rev1']})
+        stacked = _mod_graph.StackedParentsProvider([parents1, parents2])
+        self.assertEqual({'rev2': ['rev1']},
+                         stacked.get_parent_map(['rev2']))
+
+    def test__stacked_parents_provider_deprecated(self):
+        parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev3']})
+        parents2 = _mod_graph.DictParentsProvider({'rev1': ['rev4']})
+        stacked = self.applyDeprecated(deprecated_in((1, 16, 0)),
+                    _mod_graph._StackedParentsProvider, [parents1, parents2])
         self.assertEqual({'rev1':['rev4'], 'rev2':['rev3']},
                          stacked.get_parent_map(['rev1', 'rev2']))
         self.assertEqual({'rev2':['rev3'], 'rev1':['rev4']},

=== modified file 'bzrlib/versionedfile.py'
--- bzrlib/versionedfile.py	2009-05-26 09:20:17 +0000
+++ bzrlib/versionedfile.py	2009-06-06 02:36:15 +0000
@@ -40,7 +40,7 @@
     revision,
     ui,
     )
-from bzrlib.graph import DictParentsProvider, Graph, _StackedParentsProvider
+from bzrlib.graph import DictParentsProvider, Graph, StackedParentsProvider
 from bzrlib.transport.memory import MemoryTransport
 """)
 from bzrlib.inter import InterObject
@@ -1333,7 +1333,7 @@
             result[revision.NULL_REVISION] = ()
         self._providers = self._providers[:1] + self.fallback_versionedfiles
         result.update(
-            _StackedParentsProvider(self._providers).get_parent_map(keys))
+            StackedParentsProvider(self._providers).get_parent_map(keys))
         for key, parents in result.iteritems():
             if parents == ():
                 result[key] = (revision.NULL_REVISION,)