Merge lp:~garyvdm/bzr/PublicStackedParentsProvider into lp:~bzr/bzr/trunk-old
- PublicStackedParentsProvider
- Merge into trunk-old
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 |
Related bugs: |
This proposal supersedes a proposal from 2009-05-27.
This proposal has been superseded by a proposal from 2009-05-29.
Commit message
Description of the change
Gary van der Merwe (garyvdm) wrote : Posted in a previous version of this proposal | # |
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 StackedParentsP
>
def __repr__(self):
- - return "_StackedParent
+ return "StackedParents
^- we generally try to do:
return "%s(%r)" % (self._
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 _StackedParents
return StackedParentsP
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://
iEYEARECAAYFAko
5BgAn2Jk/
=12aX
-----END PGP SIGNATURE-----
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_
+ # rev2 is availible in both providers.
+ # 1
+ # |
+ # 2
+ parents1 = _mod_graph.
+ parents2 = _mod_graph.
+ stacked = self.callDeprec
+ _mod_graph.
...
+ self.assertEqua
+ stacked.
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://
iEYEARECAAYFAko
trAAn3aZTmfxX/
=HegE
-----END PGP SIGNATURE-----
Preview Diff
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 | 2699 | del ie.text_id | 2699 | del ie.text_id |
6 | 2700 | 2700 | ||
7 | 2701 | def get_parent_map(self, revision_ids): | 2701 | def get_parent_map(self, revision_ids): |
9 | 2702 | """See graph._StackedParentsProvider.get_parent_map""" | 2702 | """See graph.StackedParentsProvider.get_parent_map""" |
10 | 2703 | return dict((revision_id, self.revisions[revision_id]) | 2703 | return dict((revision_id, self.revisions[revision_id]) |
11 | 2704 | for revision_id in revision_ids | 2704 | for revision_id in revision_ids |
12 | 2705 | if revision_id in self.revisions) | 2705 | if revision_id in self.revisions) |
13 | 2706 | 2706 | ||
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 | 20 | debug, | 20 | debug, |
19 | 21 | errors, | 21 | errors, |
20 | 22 | revision, | 22 | revision, |
21 | 23 | symbol_versioning, | ||
22 | 24 | trace, | 23 | trace, |
23 | 25 | tsort, | 24 | tsort, |
24 | 26 | ) | 25 | ) |
25 | 26 | from bzrlib.symbol_versioning import deprecated_function, deprecated_in | ||
26 | 27 | 27 | ||
27 | 28 | STEP_UNIQUE_SEARCHER_EVERY = 5 | 28 | STEP_UNIQUE_SEARCHER_EVERY = 5 |
28 | 29 | 29 | ||
29 | @@ -60,18 +60,25 @@ | |||
30 | 60 | return 'DictParentsProvider(%r)' % self.ancestry | 60 | return 'DictParentsProvider(%r)' % self.ancestry |
31 | 61 | 61 | ||
32 | 62 | def get_parent_map(self, keys): | 62 | def get_parent_map(self, keys): |
34 | 63 | """See _StackedParentsProvider.get_parent_map""" | 63 | """See StackedParentsProvider.get_parent_map""" |
35 | 64 | ancestry = self.ancestry | 64 | ancestry = self.ancestry |
36 | 65 | return dict((k, ancestry[k]) for k in keys if k in ancestry) | 65 | return dict((k, ancestry[k]) for k in keys if k in ancestry) |
37 | 66 | 66 | ||
41 | 67 | 67 | @deprecated_function(deprecated_in((0, 1, 16))) | |
42 | 68 | class _StackedParentsProvider(object): | 68 | def _StackedParentsProvider(*args, **kwargs): |
43 | 69 | 69 | return StackedParentsProvider(*args, **kwargs) | |
44 | 70 | |||
45 | 71 | class StackedParentsProvider(object): | ||
46 | 72 | """A parents provider which stacks (or unions) multiple providers. | ||
47 | 73 | |||
48 | 74 | The providers are queries in the order of the provided parent_providers. | ||
49 | 75 | """ | ||
50 | 76 | |||
51 | 70 | def __init__(self, parent_providers): | 77 | def __init__(self, parent_providers): |
52 | 71 | self._parent_providers = parent_providers | 78 | self._parent_providers = parent_providers |
53 | 72 | 79 | ||
54 | 73 | def __repr__(self): | 80 | def __repr__(self): |
56 | 74 | return "_StackedParentsProvider(%r)" % self._parent_providers | 81 | return "%s(%r)" % (self.__class__.__name__, self._parent_providers) |
57 | 75 | 82 | ||
58 | 76 | def get_parent_map(self, keys): | 83 | def get_parent_map(self, keys): |
59 | 77 | """Get a mapping of keys => parents | 84 | """Get a mapping of keys => parents |
60 | @@ -148,7 +155,7 @@ | |||
61 | 148 | return dict(self._cache) | 155 | return dict(self._cache) |
62 | 149 | 156 | ||
63 | 150 | def get_parent_map(self, keys): | 157 | def get_parent_map(self, keys): |
65 | 151 | """See _StackedParentsProvider.get_parent_map.""" | 158 | """See StackedParentsProvider.get_parent_map.""" |
66 | 152 | cache = self._cache | 159 | cache = self._cache |
67 | 153 | if cache is None: | 160 | if cache is None: |
68 | 154 | cache = self._get_parent_map(keys) | 161 | cache = self._get_parent_map(keys) |
69 | 155 | 162 | ||
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 | 1192 | ', '.join(map(repr, self._indices))) | 1192 | ', '.join(map(repr, self._indices))) |
75 | 1193 | 1193 | ||
76 | 1194 | def get_parent_map(self, keys): | 1194 | def get_parent_map(self, keys): |
78 | 1195 | """See graph._StackedParentsProvider.get_parent_map""" | 1195 | """See graph.StackedParentsProvider.get_parent_map""" |
79 | 1196 | search_keys = set(keys) | 1196 | search_keys = set(keys) |
80 | 1197 | if NULL_REVISION in search_keys: | 1197 | if NULL_REVISION in search_keys: |
81 | 1198 | search_keys.discard(NULL_REVISION) | 1198 | search_keys.discard(NULL_REVISION) |
82 | 1199 | 1199 | ||
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 | 1566 | providers.insert(0, other) | 1566 | providers.insert(0, other) |
88 | 1567 | providers.extend(r._make_parents_provider() for r in | 1567 | providers.extend(r._make_parents_provider() for r in |
89 | 1568 | self._fallback_repositories) | 1568 | self._fallback_repositories) |
91 | 1569 | return graph._StackedParentsProvider(providers) | 1569 | return graph.StackedParentsProvider(providers) |
92 | 1570 | 1570 | ||
93 | 1571 | def _serialise_search_recipe(self, recipe): | 1571 | def _serialise_search_recipe(self, recipe): |
94 | 1572 | """Serialise a graph search recipe. | 1572 | """Serialise a graph search recipe. |
95 | 1573 | 1573 | ||
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 | 54 | return 'KnitParentsProvider(%r)' % self._knit | 54 | return 'KnitParentsProvider(%r)' % self._knit |
101 | 55 | 55 | ||
102 | 56 | def get_parent_map(self, keys): | 56 | def get_parent_map(self, keys): |
104 | 57 | """See graph._StackedParentsProvider.get_parent_map""" | 57 | """See graph.StackedParentsProvider.get_parent_map""" |
105 | 58 | parent_map = {} | 58 | parent_map = {} |
106 | 59 | for revision_id in keys: | 59 | for revision_id in keys: |
107 | 60 | if revision_id is None: | 60 | if revision_id is None: |
108 | @@ -85,7 +85,7 @@ | |||
109 | 85 | return 'KnitsParentsProvider(%r)' % self._knit | 85 | return 'KnitsParentsProvider(%r)' % self._knit |
110 | 86 | 86 | ||
111 | 87 | def get_parent_map(self, keys): | 87 | def get_parent_map(self, keys): |
113 | 88 | """See graph._StackedParentsProvider.get_parent_map""" | 88 | """See graph.StackedParentsProvider.get_parent_map""" |
114 | 89 | parent_map = self._knit.get_parent_map( | 89 | parent_map = self._knit.get_parent_map( |
115 | 90 | [self._prefix + (key,) for key in keys]) | 90 | [self._prefix + (key,) for key in keys]) |
116 | 91 | result = {} | 91 | result = {} |
117 | 92 | 92 | ||
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 | 2379 | return self.control_files.get_transaction() | 2379 | return self.control_files.get_transaction() |
123 | 2380 | 2380 | ||
124 | 2381 | def get_parent_map(self, revision_ids): | 2381 | def get_parent_map(self, revision_ids): |
126 | 2382 | """See graph._StackedParentsProvider.get_parent_map""" | 2382 | """See graph.StackedParentsProvider.get_parent_map""" |
127 | 2383 | # revisions index works in keys; this just works in revisions | 2383 | # revisions index works in keys; this just works in revisions |
128 | 2384 | # therefore wrap and unwrap | 2384 | # therefore wrap and unwrap |
129 | 2385 | query_keys = [] | 2385 | query_keys = [] |
130 | @@ -2408,7 +2408,7 @@ | |||
131 | 2408 | parents_provider = self._make_parents_provider() | 2408 | parents_provider = self._make_parents_provider() |
132 | 2409 | if (other_repository is not None and | 2409 | if (other_repository is not None and |
133 | 2410 | not self.has_same_location(other_repository)): | 2410 | not self.has_same_location(other_repository)): |
135 | 2411 | parents_provider = graph._StackedParentsProvider( | 2411 | parents_provider = graph.StackedParentsProvider( |
136 | 2412 | [parents_provider, other_repository._make_parents_provider()]) | 2412 | [parents_provider, other_repository._make_parents_provider()]) |
137 | 2413 | return graph.Graph(parents_provider) | 2413 | return graph.Graph(parents_provider) |
138 | 2414 | 2414 | ||
139 | 2415 | 2415 | ||
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 | 661 | self.assertEqual((set(['e']), set(['f', 'g'])), | 661 | self.assertEqual((set(['e']), set(['f', 'g'])), |
145 | 662 | graph.find_difference('e', 'f')) | 662 | graph.find_difference('e', 'f')) |
146 | 663 | 663 | ||
147 | 664 | |||
148 | 664 | def test_stacked_parents_provider(self): | 665 | def test_stacked_parents_provider(self): |
149 | 665 | parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev3']}) | 666 | parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev3']}) |
150 | 666 | parents2 = _mod_graph.DictParentsProvider({'rev1': ['rev4']}) | 667 | parents2 = _mod_graph.DictParentsProvider({'rev1': ['rev4']}) |
152 | 667 | stacked = _mod_graph._StackedParentsProvider([parents1, parents2]) | 668 | stacked = _mod_graph.StackedParentsProvider([parents1, parents2]) |
153 | 668 | self.assertEqual({'rev1':['rev4'], 'rev2':['rev3']}, | 669 | self.assertEqual({'rev1':['rev4'], 'rev2':['rev3']}, |
154 | 669 | stacked.get_parent_map(['rev1', 'rev2'])) | 670 | stacked.get_parent_map(['rev1', 'rev2'])) |
155 | 670 | self.assertEqual({'rev2':['rev3'], 'rev1':['rev4']}, | 671 | self.assertEqual({'rev2':['rev3'], 'rev1':['rev4']}, |
156 | @@ -673,6 +674,17 @@ | |||
157 | 673 | stacked.get_parent_map(['rev2', 'rev2'])) | 674 | stacked.get_parent_map(['rev2', 'rev2'])) |
158 | 674 | self.assertEqual({'rev1':['rev4']}, | 675 | self.assertEqual({'rev1':['rev4']}, |
159 | 675 | stacked.get_parent_map(['rev1', 'rev1'])) | 676 | stacked.get_parent_map(['rev1', 'rev1'])) |
160 | 677 | |||
161 | 678 | def test_stacked_parents_provider_overlapping(self): | ||
162 | 679 | # rev2 is availible in both providers. | ||
163 | 680 | # 1 | ||
164 | 681 | # | | ||
165 | 682 | # 2 | ||
166 | 683 | parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev1']}) | ||
167 | 684 | parents2 = _mod_graph.DictParentsProvider({'rev2': ['rev1']}) | ||
168 | 685 | stacked = _mod_graph.StackedParentsProvider([parents1, parents2]) | ||
169 | 686 | self.assertEqual({'rev2': ['rev1']}, | ||
170 | 687 | stacked.get_parent_map(['rev2'])) | ||
171 | 676 | 688 | ||
172 | 677 | def test_iter_topo_order(self): | 689 | def test_iter_topo_order(self): |
173 | 678 | graph = self.make_graph(ancestry_1) | 690 | graph = self.make_graph(ancestry_1) |
174 | 679 | 691 | ||
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 | 40 | revision, | 40 | revision, |
180 | 41 | ui, | 41 | ui, |
181 | 42 | ) | 42 | ) |
183 | 43 | from bzrlib.graph import DictParentsProvider, Graph, _StackedParentsProvider | 43 | from bzrlib.graph import DictParentsProvider, Graph, StackedParentsProvider |
184 | 44 | from bzrlib.transport.memory import MemoryTransport | 44 | from bzrlib.transport.memory import MemoryTransport |
185 | 45 | """) | 45 | """) |
186 | 46 | from bzrlib.inter import InterObject | 46 | from bzrlib.inter import InterObject |
187 | @@ -1333,7 +1333,7 @@ | |||
188 | 1333 | result[revision.NULL_REVISION] = () | 1333 | result[revision.NULL_REVISION] = () |
189 | 1334 | self._providers = self._providers[:1] + self.fallback_versionedfiles | 1334 | self._providers = self._providers[:1] + self.fallback_versionedfiles |
190 | 1335 | result.update( | 1335 | result.update( |
192 | 1336 | _StackedParentsProvider(self._providers).get_parent_map(keys)) | 1336 | StackedParentsProvider(self._providers).get_parent_map(keys)) |
193 | 1337 | for key, parents in result.iteritems(): | 1337 | for key, parents in result.iteritems(): |
194 | 1338 | if parents == (): | 1338 | if parents == (): |
195 | 1339 | result[key] = (revision.NULL_REVISION,) | 1339 | result[key] = (revision.NULL_REVISION,) |
The patch make StackedParentsP rovider a public api. It also adds a test to check that overlaping revisions from the different providers are handled correctly.