Merge lp:~garyvdm/bzr/PublicStackedParentsProvider into lp:~bzr/bzr/trunk-old
- PublicStackedParentsProvider
- Merge into trunk-old
Status: | Merged |
---|---|
Approved by: | John A Meinel |
Approved revision: | no longer in the source branch. |
Merged at revision: | not available |
Proposed branch: | lp:~garyvdm/bzr/PublicStackedParentsProvider |
Merge into: | lp:~bzr/bzr/trunk-old |
Diff against target: | 224 lines |
To merge this branch: | bzr merge lp:~garyvdm/bzr/PublicStackedParentsProvider |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andrew Bennetts | Approve | ||
John A Meinel | Approve | ||
Review via email: mp+6881@code.launchpad.net |
This proposal supersedes 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 : 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.
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-----
John A Meinel (jameinel) : | # |
Preview Diff
1 | === modified file 'NEWS' | |||
2 | --- NEWS 2009-06-05 23:21:51 +0000 | |||
3 | +++ NEWS 2009-06-06 02:36:15 +0000 | |||
4 | @@ -102,6 +102,9 @@ | |||
5 | 102 | *********** | 102 | *********** |
6 | 103 | 103 | ||
7 | 104 | * Added osutils.parent_directories(). (Ian Clatworthy) | 104 | * Added osutils.parent_directories(). (Ian Clatworthy) |
8 | 105 | * ``graph.StackedParentsProvider`` is now a public API, replacing | ||
9 | 106 | ``graph._StackedParentsProvider``. The api is now considered stable and ready | ||
10 | 107 | for external users. (Gary van der Merwe) | ||
11 | 105 | 108 | ||
12 | 106 | * TreeTransformBase no longer assumes that limbo is provided via disk. | 109 | * TreeTransformBase no longer assumes that limbo is provided via disk. |
13 | 107 | DiskTreeTransform now provides disk functionality. (Aaron Bentley) | 110 | DiskTreeTransform now provides disk functionality. (Aaron Bentley) |
14 | 108 | 111 | ||
15 | === modified file 'bzrlib/bzrdir.py' | |||
16 | --- bzrlib/bzrdir.py 2009-06-04 21:25:46 +0000 | |||
17 | +++ bzrlib/bzrdir.py 2009-06-06 02:36:15 +0000 | |||
18 | @@ -2768,7 +2768,7 @@ | |||
19 | 2768 | del ie.text_id | 2768 | del ie.text_id |
20 | 2769 | 2769 | ||
21 | 2770 | def get_parent_map(self, revision_ids): | 2770 | def get_parent_map(self, revision_ids): |
23 | 2771 | """See graph._StackedParentsProvider.get_parent_map""" | 2771 | """See graph.StackedParentsProvider.get_parent_map""" |
24 | 2772 | return dict((revision_id, self.revisions[revision_id]) | 2772 | return dict((revision_id, self.revisions[revision_id]) |
25 | 2773 | for revision_id in revision_ids | 2773 | for revision_id in revision_ids |
26 | 2774 | if revision_id in self.revisions) | 2774 | if revision_id in self.revisions) |
27 | 2775 | 2775 | ||
28 | === modified file 'bzrlib/graph.py' | |||
29 | --- bzrlib/graph.py 2009-05-27 09:40:33 +0000 | |||
30 | +++ bzrlib/graph.py 2009-06-06 02:36:15 +0000 | |||
31 | @@ -20,10 +20,10 @@ | |||
32 | 20 | debug, | 20 | debug, |
33 | 21 | errors, | 21 | errors, |
34 | 22 | revision, | 22 | revision, |
35 | 23 | symbol_versioning, | ||
36 | 24 | trace, | 23 | trace, |
37 | 25 | tsort, | 24 | tsort, |
38 | 26 | ) | 25 | ) |
39 | 26 | from bzrlib.symbol_versioning import deprecated_function, deprecated_in | ||
40 | 27 | 27 | ||
41 | 28 | STEP_UNIQUE_SEARCHER_EVERY = 5 | 28 | STEP_UNIQUE_SEARCHER_EVERY = 5 |
42 | 29 | 29 | ||
43 | @@ -60,18 +60,25 @@ | |||
44 | 60 | return 'DictParentsProvider(%r)' % self.ancestry | 60 | return 'DictParentsProvider(%r)' % self.ancestry |
45 | 61 | 61 | ||
46 | 62 | def get_parent_map(self, keys): | 62 | def get_parent_map(self, keys): |
48 | 63 | """See _StackedParentsProvider.get_parent_map""" | 63 | """See StackedParentsProvider.get_parent_map""" |
49 | 64 | ancestry = self.ancestry | 64 | ancestry = self.ancestry |
50 | 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) |
51 | 66 | 66 | ||
55 | 67 | 67 | @deprecated_function(deprecated_in((1, 16, 0))) | |
56 | 68 | class _StackedParentsProvider(object): | 68 | def _StackedParentsProvider(*args, **kwargs): |
57 | 69 | 69 | return StackedParentsProvider(*args, **kwargs) | |
58 | 70 | |||
59 | 71 | class StackedParentsProvider(object): | ||
60 | 72 | """A parents provider which stacks (or unions) multiple providers. | ||
61 | 73 | |||
62 | 74 | The providers are queries in the order of the provided parent_providers. | ||
63 | 75 | """ | ||
64 | 76 | |||
65 | 70 | def __init__(self, parent_providers): | 77 | def __init__(self, parent_providers): |
66 | 71 | self._parent_providers = parent_providers | 78 | self._parent_providers = parent_providers |
67 | 72 | 79 | ||
68 | 73 | def __repr__(self): | 80 | def __repr__(self): |
70 | 74 | return "_StackedParentsProvider(%r)" % self._parent_providers | 81 | return "%s(%r)" % (self.__class__.__name__, self._parent_providers) |
71 | 75 | 82 | ||
72 | 76 | def get_parent_map(self, keys): | 83 | def get_parent_map(self, keys): |
73 | 77 | """Get a mapping of keys => parents | 84 | """Get a mapping of keys => parents |
74 | @@ -148,7 +155,7 @@ | |||
75 | 148 | return dict(self._cache) | 155 | return dict(self._cache) |
76 | 149 | 156 | ||
77 | 150 | def get_parent_map(self, keys): | 157 | def get_parent_map(self, keys): |
79 | 151 | """See _StackedParentsProvider.get_parent_map.""" | 158 | """See StackedParentsProvider.get_parent_map.""" |
80 | 152 | cache = self._cache | 159 | cache = self._cache |
81 | 153 | if cache is None: | 160 | if cache is None: |
82 | 154 | cache = self._get_parent_map(keys) | 161 | cache = self._get_parent_map(keys) |
83 | 155 | 162 | ||
84 | === modified file 'bzrlib/index.py' | |||
85 | --- bzrlib/index.py 2009-03-24 01:53:42 +0000 | |||
86 | +++ bzrlib/index.py 2009-06-06 02:36:15 +0000 | |||
87 | @@ -1192,7 +1192,7 @@ | |||
88 | 1192 | ', '.join(map(repr, self._indices))) | 1192 | ', '.join(map(repr, self._indices))) |
89 | 1193 | 1193 | ||
90 | 1194 | def get_parent_map(self, keys): | 1194 | def get_parent_map(self, keys): |
92 | 1195 | """See graph._StackedParentsProvider.get_parent_map""" | 1195 | """See graph.StackedParentsProvider.get_parent_map""" |
93 | 1196 | search_keys = set(keys) | 1196 | search_keys = set(keys) |
94 | 1197 | if NULL_REVISION in search_keys: | 1197 | if NULL_REVISION in search_keys: |
95 | 1198 | search_keys.discard(NULL_REVISION) | 1198 | search_keys.discard(NULL_REVISION) |
96 | 1199 | 1199 | ||
97 | === modified file 'bzrlib/remote.py' | |||
98 | --- bzrlib/remote.py 2009-06-03 01:37:27 +0000 | |||
99 | +++ bzrlib/remote.py 2009-06-06 02:36:15 +0000 | |||
100 | @@ -1576,7 +1576,7 @@ | |||
101 | 1576 | providers.insert(0, other) | 1576 | providers.insert(0, other) |
102 | 1577 | providers.extend(r._make_parents_provider() for r in | 1577 | providers.extend(r._make_parents_provider() for r in |
103 | 1578 | self._fallback_repositories) | 1578 | self._fallback_repositories) |
105 | 1579 | return graph._StackedParentsProvider(providers) | 1579 | return graph.StackedParentsProvider(providers) |
106 | 1580 | 1580 | ||
107 | 1581 | def _serialise_search_recipe(self, recipe): | 1581 | def _serialise_search_recipe(self, recipe): |
108 | 1582 | """Serialise a graph search recipe. | 1582 | """Serialise a graph search recipe. |
109 | 1583 | 1583 | ||
110 | === modified file 'bzrlib/repofmt/knitrepo.py' | |||
111 | --- bzrlib/repofmt/knitrepo.py 2009-04-09 20:23:07 +0000 | |||
112 | +++ bzrlib/repofmt/knitrepo.py 2009-06-06 02:36:15 +0000 | |||
113 | @@ -54,7 +54,7 @@ | |||
114 | 54 | return 'KnitParentsProvider(%r)' % self._knit | 54 | return 'KnitParentsProvider(%r)' % self._knit |
115 | 55 | 55 | ||
116 | 56 | def get_parent_map(self, keys): | 56 | def get_parent_map(self, keys): |
118 | 57 | """See graph._StackedParentsProvider.get_parent_map""" | 57 | """See graph.StackedParentsProvider.get_parent_map""" |
119 | 58 | parent_map = {} | 58 | parent_map = {} |
120 | 59 | for revision_id in keys: | 59 | for revision_id in keys: |
121 | 60 | if revision_id is None: | 60 | if revision_id is None: |
122 | @@ -85,7 +85,7 @@ | |||
123 | 85 | return 'KnitsParentsProvider(%r)' % self._knit | 85 | return 'KnitsParentsProvider(%r)' % self._knit |
124 | 86 | 86 | ||
125 | 87 | def get_parent_map(self, keys): | 87 | def get_parent_map(self, keys): |
127 | 88 | """See graph._StackedParentsProvider.get_parent_map""" | 88 | """See graph.StackedParentsProvider.get_parent_map""" |
128 | 89 | parent_map = self._knit.get_parent_map( | 89 | parent_map = self._knit.get_parent_map( |
129 | 90 | [self._prefix + (key,) for key in keys]) | 90 | [self._prefix + (key,) for key in keys]) |
130 | 91 | result = {} | 91 | result = {} |
131 | 92 | 92 | ||
132 | === modified file 'bzrlib/repository.py' | |||
133 | --- bzrlib/repository.py 2009-06-04 21:25:46 +0000 | |||
134 | +++ bzrlib/repository.py 2009-06-06 02:36:15 +0000 | |||
135 | @@ -2384,7 +2384,7 @@ | |||
136 | 2384 | return self.control_files.get_transaction() | 2384 | return self.control_files.get_transaction() |
137 | 2385 | 2385 | ||
138 | 2386 | def get_parent_map(self, revision_ids): | 2386 | def get_parent_map(self, revision_ids): |
140 | 2387 | """See graph._StackedParentsProvider.get_parent_map""" | 2387 | """See graph.StackedParentsProvider.get_parent_map""" |
141 | 2388 | # revisions index works in keys; this just works in revisions | 2388 | # revisions index works in keys; this just works in revisions |
142 | 2389 | # therefore wrap and unwrap | 2389 | # therefore wrap and unwrap |
143 | 2390 | query_keys = [] | 2390 | query_keys = [] |
144 | @@ -2413,7 +2413,7 @@ | |||
145 | 2413 | parents_provider = self._make_parents_provider() | 2413 | parents_provider = self._make_parents_provider() |
146 | 2414 | if (other_repository is not None and | 2414 | if (other_repository is not None and |
147 | 2415 | not self.has_same_location(other_repository)): | 2415 | not self.has_same_location(other_repository)): |
149 | 2416 | parents_provider = graph._StackedParentsProvider( | 2416 | parents_provider = graph.StackedParentsProvider( |
150 | 2417 | [parents_provider, other_repository._make_parents_provider()]) | 2417 | [parents_provider, other_repository._make_parents_provider()]) |
151 | 2418 | return graph.Graph(parents_provider) | 2418 | return graph.Graph(parents_provider) |
152 | 2419 | 2419 | ||
153 | 2420 | 2420 | ||
154 | === modified file 'bzrlib/tests/test_graph.py' | |||
155 | --- bzrlib/tests/test_graph.py 2009-05-12 02:24:54 +0000 | |||
156 | +++ bzrlib/tests/test_graph.py 2009-06-06 02:36:15 +0000 | |||
157 | @@ -22,6 +22,7 @@ | |||
158 | 22 | ) | 22 | ) |
159 | 23 | from bzrlib.revision import NULL_REVISION | 23 | from bzrlib.revision import NULL_REVISION |
160 | 24 | from bzrlib.tests import TestCaseWithMemoryTransport | 24 | from bzrlib.tests import TestCaseWithMemoryTransport |
161 | 25 | from bzrlib.symbol_versioning import deprecated_in | ||
162 | 25 | 26 | ||
163 | 26 | 27 | ||
164 | 27 | # Ancestry 1: | 28 | # Ancestry 1: |
165 | @@ -661,10 +662,36 @@ | |||
166 | 661 | self.assertEqual((set(['e']), set(['f', 'g'])), | 662 | self.assertEqual((set(['e']), set(['f', 'g'])), |
167 | 662 | graph.find_difference('e', 'f')) | 663 | graph.find_difference('e', 'f')) |
168 | 663 | 664 | ||
169 | 665 | |||
170 | 664 | def test_stacked_parents_provider(self): | 666 | def test_stacked_parents_provider(self): |
171 | 665 | parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev3']}) | 667 | parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev3']}) |
172 | 666 | parents2 = _mod_graph.DictParentsProvider({'rev1': ['rev4']}) | 668 | parents2 = _mod_graph.DictParentsProvider({'rev1': ['rev4']}) |
174 | 667 | stacked = _mod_graph._StackedParentsProvider([parents1, parents2]) | 669 | stacked = _mod_graph.StackedParentsProvider([parents1, parents2]) |
175 | 670 | self.assertEqual({'rev1':['rev4'], 'rev2':['rev3']}, | ||
176 | 671 | stacked.get_parent_map(['rev1', 'rev2'])) | ||
177 | 672 | self.assertEqual({'rev2':['rev3'], 'rev1':['rev4']}, | ||
178 | 673 | stacked.get_parent_map(['rev2', 'rev1'])) | ||
179 | 674 | self.assertEqual({'rev2':['rev3']}, | ||
180 | 675 | stacked.get_parent_map(['rev2', 'rev2'])) | ||
181 | 676 | self.assertEqual({'rev1':['rev4']}, | ||
182 | 677 | stacked.get_parent_map(['rev1', 'rev1'])) | ||
183 | 678 | |||
184 | 679 | def test_stacked_parents_provider_overlapping(self): | ||
185 | 680 | # rev2 is availible in both providers. | ||
186 | 681 | # 1 | ||
187 | 682 | # | | ||
188 | 683 | # 2 | ||
189 | 684 | parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev1']}) | ||
190 | 685 | parents2 = _mod_graph.DictParentsProvider({'rev2': ['rev1']}) | ||
191 | 686 | stacked = _mod_graph.StackedParentsProvider([parents1, parents2]) | ||
192 | 687 | self.assertEqual({'rev2': ['rev1']}, | ||
193 | 688 | stacked.get_parent_map(['rev2'])) | ||
194 | 689 | |||
195 | 690 | def test__stacked_parents_provider_deprecated(self): | ||
196 | 691 | parents1 = _mod_graph.DictParentsProvider({'rev2': ['rev3']}) | ||
197 | 692 | parents2 = _mod_graph.DictParentsProvider({'rev1': ['rev4']}) | ||
198 | 693 | stacked = self.applyDeprecated(deprecated_in((1, 16, 0)), | ||
199 | 694 | _mod_graph._StackedParentsProvider, [parents1, parents2]) | ||
200 | 668 | self.assertEqual({'rev1':['rev4'], 'rev2':['rev3']}, | 695 | self.assertEqual({'rev1':['rev4'], 'rev2':['rev3']}, |
201 | 669 | stacked.get_parent_map(['rev1', 'rev2'])) | 696 | stacked.get_parent_map(['rev1', 'rev2'])) |
202 | 670 | self.assertEqual({'rev2':['rev3'], 'rev1':['rev4']}, | 697 | self.assertEqual({'rev2':['rev3'], 'rev1':['rev4']}, |
203 | 671 | 698 | ||
204 | === modified file 'bzrlib/versionedfile.py' | |||
205 | --- bzrlib/versionedfile.py 2009-05-26 09:20:17 +0000 | |||
206 | +++ bzrlib/versionedfile.py 2009-06-06 02:36:15 +0000 | |||
207 | @@ -40,7 +40,7 @@ | |||
208 | 40 | revision, | 40 | revision, |
209 | 41 | ui, | 41 | ui, |
210 | 42 | ) | 42 | ) |
212 | 43 | from bzrlib.graph import DictParentsProvider, Graph, _StackedParentsProvider | 43 | from bzrlib.graph import DictParentsProvider, Graph, StackedParentsProvider |
213 | 44 | from bzrlib.transport.memory import MemoryTransport | 44 | from bzrlib.transport.memory import MemoryTransport |
214 | 45 | """) | 45 | """) |
215 | 46 | from bzrlib.inter import InterObject | 46 | from bzrlib.inter import InterObject |
216 | @@ -1333,7 +1333,7 @@ | |||
217 | 1333 | result[revision.NULL_REVISION] = () | 1333 | result[revision.NULL_REVISION] = () |
218 | 1334 | self._providers = self._providers[:1] + self.fallback_versionedfiles | 1334 | self._providers = self._providers[:1] + self.fallback_versionedfiles |
219 | 1335 | result.update( | 1335 | result.update( |
221 | 1336 | _StackedParentsProvider(self._providers).get_parent_map(keys)) | 1336 | StackedParentsProvider(self._providers).get_parent_map(keys)) |
222 | 1337 | for key, parents in result.iteritems(): | 1337 | for key, parents in result.iteritems(): |
223 | 1338 | if parents == (): | 1338 | if parents == (): |
224 | 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.