Bazaar

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

  1. PublicStackedParentsProvider
  2. Merge into trunk-old
Proposed by Gary van der Merwe
Status: Superseded
Proposed branch: lp:~garyvdm/bzr/PublicStackedParentsProvider
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 195 lines
To merge this branch: bzr merge lp:~garyvdm/bzr/PublicStackedParentsProvider

This proposal supersedes a proposal from 2009-05-27.

This proposal has been superseded by a proposal from 2009-05-29.

To post a comment you must log in.
Revision history for this message
Gary van der Merwe (garyvdm) wrote : Posted in a previous version of this proposal

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

Revision history for this message
John A Meinel (jameinel) wrote : Posted in a previous version of this proposal

-----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-----

Revision history for this message
John A Meinel (jameinel) wrote :

-----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-----

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/bzrdir.py'
2--- bzrlib/bzrdir.py 2009-05-23 04:55:52 +0000
3+++ bzrlib/bzrdir.py 2009-05-29 08:35:09 +0000
4@@ -2699,7 +2699,7 @@
5 del ie.text_id
6
7 def get_parent_map(self, revision_ids):
8- """See graph._StackedParentsProvider.get_parent_map"""
9+ """See graph.StackedParentsProvider.get_parent_map"""
10 return dict((revision_id, self.revisions[revision_id])
11 for revision_id in revision_ids
12 if revision_id in self.revisions)
13
14=== modified file 'bzrlib/graph.py'
15--- bzrlib/graph.py 2009-05-23 04:55:52 +0000
16+++ bzrlib/graph.py 2009-05-29 08:35:09 +0000
17@@ -20,10 +20,10 @@
18 debug,
19 errors,
20 revision,
21- symbol_versioning,
22 trace,
23 tsort,
24 )
25+from bzrlib.symbol_versioning import deprecated_function, deprecated_in
26
27 STEP_UNIQUE_SEARCHER_EVERY = 5
28
29@@ -60,18 +60,25 @@
30 return 'DictParentsProvider(%r)' % self.ancestry
31
32 def get_parent_map(self, keys):
33- """See _StackedParentsProvider.get_parent_map"""
34+ """See StackedParentsProvider.get_parent_map"""
35 ancestry = self.ancestry
36 return dict((k, ancestry[k]) for k in keys if k in ancestry)
37
38-
39-class _StackedParentsProvider(object):
40-
41+@deprecated_function(deprecated_in((0, 1, 16)))
42+def _StackedParentsProvider(*args, **kwargs):
43+ return StackedParentsProvider(*args, **kwargs)
44+
45+class StackedParentsProvider(object):
46+ """A parents provider which stacks (or unions) multiple providers.
47+
48+ The providers are queries in the order of the provided parent_providers.
49+ """
50+
51 def __init__(self, parent_providers):
52 self._parent_providers = parent_providers
53
54 def __repr__(self):
55- return "_StackedParentsProvider(%r)" % self._parent_providers
56+ return "%s(%r)" % (self.__class__.__name__, self._parent_providers)
57
58 def get_parent_map(self, keys):
59 """Get a mapping of keys => parents
60@@ -148,7 +155,7 @@
61 return dict(self._cache)
62
63 def get_parent_map(self, keys):
64- """See _StackedParentsProvider.get_parent_map."""
65+ """See StackedParentsProvider.get_parent_map."""
66 cache = self._cache
67 if cache is None:
68 cache = self._get_parent_map(keys)
69
70=== modified file 'bzrlib/index.py'
71--- bzrlib/index.py 2009-03-24 01:53:42 +0000
72+++ bzrlib/index.py 2009-05-29 08:35:09 +0000
73@@ -1192,7 +1192,7 @@
74 ', '.join(map(repr, self._indices)))
75
76 def get_parent_map(self, keys):
77- """See graph._StackedParentsProvider.get_parent_map"""
78+ """See graph.StackedParentsProvider.get_parent_map"""
79 search_keys = set(keys)
80 if NULL_REVISION in search_keys:
81 search_keys.discard(NULL_REVISION)
82
83=== modified file 'bzrlib/remote.py'
84--- bzrlib/remote.py 2009-05-10 23:45:33 +0000
85+++ bzrlib/remote.py 2009-05-29 08:35:09 +0000
86@@ -1566,7 +1566,7 @@
87 providers.insert(0, other)
88 providers.extend(r._make_parents_provider() for r in
89 self._fallback_repositories)
90- return graph._StackedParentsProvider(providers)
91+ return graph.StackedParentsProvider(providers)
92
93 def _serialise_search_recipe(self, recipe):
94 """Serialise a graph search recipe.
95
96=== modified file 'bzrlib/repofmt/knitrepo.py'
97--- bzrlib/repofmt/knitrepo.py 2009-04-09 20:23:07 +0000
98+++ bzrlib/repofmt/knitrepo.py 2009-05-29 08:35:09 +0000
99@@ -54,7 +54,7 @@
100 return 'KnitParentsProvider(%r)' % self._knit
101
102 def get_parent_map(self, keys):
103- """See graph._StackedParentsProvider.get_parent_map"""
104+ """See graph.StackedParentsProvider.get_parent_map"""
105 parent_map = {}
106 for revision_id in keys:
107 if revision_id is None:
108@@ -85,7 +85,7 @@
109 return 'KnitsParentsProvider(%r)' % self._knit
110
111 def get_parent_map(self, keys):
112- """See graph._StackedParentsProvider.get_parent_map"""
113+ """See graph.StackedParentsProvider.get_parent_map"""
114 parent_map = self._knit.get_parent_map(
115 [self._prefix + (key,) for key in keys])
116 result = {}
117
118=== modified file 'bzrlib/repository.py'
119--- bzrlib/repository.py 2009-05-12 04:54:04 +0000
120+++ bzrlib/repository.py 2009-05-29 08:35:09 +0000
121@@ -2379,7 +2379,7 @@
122 return self.control_files.get_transaction()
123
124 def get_parent_map(self, revision_ids):
125- """See graph._StackedParentsProvider.get_parent_map"""
126+ """See graph.StackedParentsProvider.get_parent_map"""
127 # revisions index works in keys; this just works in revisions
128 # therefore wrap and unwrap
129 query_keys = []
130@@ -2408,7 +2408,7 @@
131 parents_provider = self._make_parents_provider()
132 if (other_repository is not None and
133 not self.has_same_location(other_repository)):
134- parents_provider = graph._StackedParentsProvider(
135+ parents_provider = graph.StackedParentsProvider(
136 [parents_provider, other_repository._make_parents_provider()])
137 return graph.Graph(parents_provider)
138
139
140=== modified file 'bzrlib/tests/test_graph.py'
141--- bzrlib/tests/test_graph.py 2009-03-24 23:19:12 +0000
142+++ bzrlib/tests/test_graph.py 2009-05-29 08:35:09 +0000
143@@ -661,10 +661,11 @@
144 self.assertEqual((set(['e']), set(['f', 'g'])),
145 graph.find_difference('e', 'f'))
146
147+
148 def test_stacked_parents_provider(self):
149 parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev3']})
150 parents2 = _mod_graph.DictParentsProvider({'rev1': ['rev4']})
151- stacked = _mod_graph._StackedParentsProvider([parents1, parents2])
152+ stacked = _mod_graph.StackedParentsProvider([parents1, parents2])
153 self.assertEqual({'rev1':['rev4'], 'rev2':['rev3']},
154 stacked.get_parent_map(['rev1', 'rev2']))
155 self.assertEqual({'rev2':['rev3'], 'rev1':['rev4']},
156@@ -673,6 +674,17 @@
157 stacked.get_parent_map(['rev2', 'rev2']))
158 self.assertEqual({'rev1':['rev4']},
159 stacked.get_parent_map(['rev1', 'rev1']))
160+
161+ def test_stacked_parents_provider_overlapping(self):
162+ # rev2 is availible in both providers.
163+ # 1
164+ # |
165+ # 2
166+ parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev1']})
167+ parents2 = _mod_graph.DictParentsProvider({'rev2': ['rev1']})
168+ stacked = _mod_graph.StackedParentsProvider([parents1, parents2])
169+ self.assertEqual({'rev2': ['rev1']},
170+ stacked.get_parent_map(['rev2']))
171
172 def test_iter_topo_order(self):
173 graph = self.make_graph(ancestry_1)
174
175=== modified file 'bzrlib/versionedfile.py'
176--- bzrlib/versionedfile.py 2009-04-29 17:02:36 +0000
177+++ bzrlib/versionedfile.py 2009-05-29 08:35:09 +0000
178@@ -40,7 +40,7 @@
179 revision,
180 ui,
181 )
182-from bzrlib.graph import DictParentsProvider, Graph, _StackedParentsProvider
183+from bzrlib.graph import DictParentsProvider, Graph, StackedParentsProvider
184 from bzrlib.transport.memory import MemoryTransport
185 """)
186 from bzrlib.inter import InterObject
187@@ -1333,7 +1333,7 @@
188 result[revision.NULL_REVISION] = ()
189 self._providers = self._providers[:1] + self.fallback_versionedfiles
190 result.update(
191- _StackedParentsProvider(self._providers).get_parent_map(keys))
192+ StackedParentsProvider(self._providers).get_parent_map(keys))
193 for key, parents in result.iteritems():
194 if parents == ():
195 result[key] = (revision.NULL_REVISION,)