Merge lp:~jelmer/brz/location-purpose into lp:brz

Proposed by Jelmer Vernooij on 2019-02-09
Status: Merged
Approved by: Jelmer Vernooij on 2019-02-14
Approved revision: 7274
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~jelmer/brz/location-purpose
Merge into: lp:brz
Diff against target: 383 lines (+87/-27)
14 files modified
breezy/builtins.py (+4/-4)
breezy/directory_service.py (+26/-6)
breezy/git/directory.py (+1/-1)
breezy/location.py (+6/-4)
breezy/plugins/launchpad/__init__.py (+0/-1)
breezy/plugins/launchpad/lp_directory.py (+1/-1)
breezy/plugins/launchpad/test_lp_directory.py (+1/-1)
breezy/push.py (+1/-1)
breezy/tests/blackbox/test_pull.py (+1/-1)
breezy/tests/blackbox/test_switch.py (+1/-1)
breezy/tests/test_bundle.py (+1/-1)
breezy/tests/test_directory_service.py (+36/-1)
breezy/tests/test_location.py (+2/-2)
breezy/transport/__init__.py (+6/-2)
To merge this branch: bzr merge lp:~jelmer/brz/location-purpose
Reviewer Review Type Date Requested Status
Martin Packman 2019-02-09 Approve on 2019-02-14
Review via email: mp+362940@code.launchpad.net

Commit message

Add a purpose argument to location dereference functions.

Description of the change

Add a purpose argument to location dereference functions.

To post a comment you must log in.
Martin Packman (gz) wrote :

This branch has the hook change in as well. Not quite sure I understand the reasoning here, but seems basically reasonable? Tests use 'push' as a purpose, but the documented values are 'read' 'write' None?

review: Approve
Jelmer Vernooij (jelmer) wrote :

Change push => write

Jelmer Vernooij (jelmer) wrote :

The idea here is that you can declare whether you're going to read from or write to a branch, allowing callees (such as a directory service) to e.g. give you a https or bzr+ssh URL.

lp:~jelmer/brz/location-purpose updated on 2019-02-14
7274. By Jelmer Vernooij on 2019-02-14

Fix purpose argument to get_transport.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/builtins.py'
2--- breezy/builtins.py 2019-02-04 01:01:24 +0000
3+++ breezy/builtins.py 2019-02-14 22:08:33 +0000
4@@ -1480,7 +1480,7 @@
5 revision_id = br_from.last_revision()
6 if to_location is None:
7 to_location = urlutils.derive_to_location(from_location)
8- to_transport = transport.get_transport(to_location)
9+ to_transport = transport.get_transport(to_location, purpose='write')
10 try:
11 to_transport.mkdir('.')
12 except errors.FileExists:
13@@ -1574,7 +1574,7 @@
14
15 def run(self, location=".", recursive=False):
16 if recursive:
17- t = transport.get_transport(location)
18+ t = transport.get_transport(location, purpose='read')
19 if not t.listable():
20 raise errors.BzrCommandError(
21 "Can't scan this type of location.")
22@@ -2058,7 +2058,7 @@
23 if location is None:
24 location = u'.'
25
26- to_transport = transport.get_transport(location)
27+ to_transport = transport.get_transport(location, purpose='write')
28
29 # The path has to exist to initialize a
30 # branch inside of it.
31@@ -2178,7 +2178,7 @@
32 if location is None:
33 location = '.'
34
35- to_transport = transport.get_transport(location)
36+ to_transport = transport.get_transport(location, purpose='write')
37
38 if format.fixed_components:
39 repo_format_name = None
40
41=== modified file 'breezy/directory_service.py'
42--- breezy/directory_service.py 2018-11-11 04:08:32 +0000
43+++ breezy/directory_service.py 2019-02-14 22:08:33 +0000
44@@ -67,7 +67,7 @@
45 name and URL, and return a URL.
46 """
47
48- def dereference(self, url):
49+ def dereference(self, url, purpose=None):
50 """Dereference a supplied URL if possible.
51
52 URLs that match a registered directory service prefix are looked up in
53@@ -77,19 +77,39 @@
54 requires further dereferencing.
55
56 :param url: The URL to dereference
57+ :param purpose: Purpose of the URL ('read', 'write' or None - if not declared)
58 :return: The dereferenced URL if applicable, the input URL otherwise.
59 """
60 match = self.get_prefix(url)
61 if match is None:
62 return url
63 service, name = match
64- return service().look_up(name, url)
65+ directory = service()
66+ try:
67+ return directory.look_up(name, url, purpose=purpose)
68+ except TypeError:
69+ # Compatibility for plugins written for Breezy < 3.0.0
70+ return directory.look_up(name, url)
71
72
73 directories = DirectoryServiceRegistry()
74
75
76-class AliasDirectory(object):
77+class Directory(object):
78+ """Abstract directory lookup class."""
79+
80+ def look_up(self, name, url, purpose=None):
81+ """Look up an entry in a directory.
82+
83+ :param name: Directory name
84+ :param url: The URL to dereference
85+ :param purpose: Purpose of the URL ('read', 'write' or None - if not declared)
86+ :return: The dereferenced URL if applicable, the input URL otherwise.
87+ """
88+ raise NotImplementedError(self.look_up)
89+
90+
91+class AliasDirectory(Directory):
92 """Directory lookup for locations associated with a branch.
93
94 :parent, :submit, :public, :push, :this, and :bound are currently
95@@ -110,7 +130,7 @@
96 branch_aliases.register('this', lambda b: b.base,
97 help="This branch.")
98
99- def look_up(self, name, url):
100+ def look_up(self, name, url, purpose=None):
101 branch = _mod_branch.Branch.open_containing('.')[0]
102 parts = url.split('/', 1)
103 if len(parts) == 2:
104@@ -156,14 +176,14 @@
105 'Easy access to remembered branch locations')
106
107
108-class ColocatedDirectory(object):
109+class ColocatedDirectory(Directory):
110 """Directory lookup for colocated branches.
111
112 co:somename will resolve to the colocated branch with "somename" in
113 the current directory.
114 """
115
116- def look_up(self, name, url):
117+ def look_up(self, name, url, purpose=None):
118 dir = _mod_controldir.ControlDir.open_containing('.')[0]
119 return urlutils.join_segment_parameters(dir.user_url,
120 {"branch": urlutils.escape(name)})
121
122=== modified file 'breezy/git/directory.py'
123--- breezy/git/directory.py 2018-11-11 04:08:32 +0000
124+++ breezy/git/directory.py 2019-02-14 22:08:33 +0000
125@@ -26,6 +26,6 @@
126
127 class GitHubDirectory(object):
128
129- def look_up(self, name, url):
130+ def look_up(self, name, url, purpose=None):
131 """See DirectoryService.look_up"""
132 return "git+ssh://git@github.com/" + name
133
134=== modified file 'breezy/location.py'
135--- breezy/location.py 2019-02-09 02:03:11 +0000
136+++ breezy/location.py 2019-02-14 22:08:33 +0000
137@@ -37,26 +37,28 @@
138 Hooks.__init__(self, "breezy.location", "hooks")
139 self.add_hook(
140 'rewrite_url',
141- "Called with a URL to rewrite.", (3, 0))
142+ "Possibly rewrite a URL. Called with a URL to rewrite and the "
143+ "purpose of the URL.", (3, 0))
144
145
146 hooks = LocationHooks()
147
148
149-def location_to_url(location):
150+def location_to_url(location, purpose=None):
151 """Determine a fully qualified URL from a location string.
152
153 This will try to interpret location as both a URL and a directory path. It
154 will also lookup the location in directories.
155
156 :param location: Unicode or byte string object with a location
157+ :param purpose: Intended method of access (None, 'read' or 'write')
158 :raise InvalidURL: If the location is already a URL, but not valid.
159 :return: Byte string with resulting URL
160 """
161 if not isinstance(location, string_types):
162 raise AssertionError("location not a byte or unicode string")
163 from .directory_service import directories
164- location = directories.dereference(location)
165+ location = directories.dereference(location, purpose)
166
167 # Catch any URLs which are passing Unicode rather than ASCII
168 try:
169@@ -77,6 +79,6 @@
170 return urlutils.local_path_to_url(location)
171
172 for hook in hooks['rewrite_url']:
173- location = hook(location)
174+ location = hook(location, purpose=purpose)
175
176 return location
177
178=== modified file 'breezy/plugins/launchpad/__init__.py'
179--- breezy/plugins/launchpad/__init__.py 2019-02-02 22:23:40 +0000
180+++ breezy/plugins/launchpad/__init__.py 2019-02-14 22:08:33 +0000
181@@ -29,7 +29,6 @@
182
183 launchpad-login: Show or set the Launchpad user ID
184 launchpad-open: Open a Launchpad branch page in your web browser
185- launchpad-mirror: Ask Launchpad to mirror a branch now
186
187 As well as the following deprecated command:
188
189
190=== modified file 'breezy/plugins/launchpad/lp_directory.py'
191--- breezy/plugins/launchpad/lp_directory.py 2019-02-02 22:23:40 +0000
192+++ breezy/plugins/launchpad/lp_directory.py 2019-02-14 22:08:33 +0000
193@@ -117,7 +117,7 @@
194
195 class LaunchpadDirectory(object):
196
197- def look_up(self, name, url):
198+ def look_up(self, name, url, purpose=None):
199 """See DirectoryService.look_up"""
200 return self._resolve(url)
201
202
203=== modified file 'breezy/plugins/launchpad/test_lp_directory.py'
204--- breezy/plugins/launchpad/test_lp_directory.py 2019-01-17 01:04:56 +0000
205+++ breezy/plugins/launchpad/test_lp_directory.py 2019-02-14 22:08:33 +0000
206@@ -397,7 +397,7 @@
207 class FooService(object):
208 """A directory service that maps the name to a FILE url"""
209
210- def look_up(self, name, url):
211+ def look_up(self, name, url, purpose=None):
212 if 'lp:///apt' == url:
213 return target_branch.base.rstrip('/')
214 return '!unexpected look_up value!'
215
216=== modified file 'breezy/push.py'
217--- breezy/push.py 2018-11-17 16:53:10 +0000
218+++ breezy/push.py 2019-02-14 22:08:33 +0000
219@@ -81,7 +81,7 @@
220 directory exists without a current control directory in it
221 :param lossy: Allow lossy push
222 """
223- to_transport = transport.get_transport(location)
224+ to_transport = transport.get_transport(location, purpose='write')
225 try:
226 dir_to = controldir.ControlDir.open_from_transport(to_transport)
227 except errors.NotBranchError:
228
229=== modified file 'breezy/tests/blackbox/test_pull.py'
230--- breezy/tests/blackbox/test_pull.py 2018-11-29 23:42:41 +0000
231+++ breezy/tests/blackbox/test_pull.py 2019-02-14 22:08:33 +0000
232@@ -353,7 +353,7 @@
233 class FooService(object):
234 """A directory service that always returns source"""
235
236- def look_up(self, name, url):
237+ def look_up(self, name, url, purpose=None):
238 return 'source'
239 directories.register('foo:', FooService, 'Testing directory service')
240 self.addCleanup(directories.remove, 'foo:')
241
242=== modified file 'breezy/tests/blackbox/test_switch.py'
243--- breezy/tests/blackbox/test_switch.py 2019-01-04 21:21:12 +0000
244+++ breezy/tests/blackbox/test_switch.py 2019-02-14 22:08:33 +0000
245@@ -347,7 +347,7 @@
246 tree = branch.create_checkout('tree', lightweight=True)
247
248 class FooLookup(object):
249- def look_up(self, name, url):
250+ def look_up(self, name, url, purpose=None):
251 return 'foo-' + name
252 directories.register('foo:', FooLookup, 'Create branches named foo-')
253 self.addCleanup(directories.remove, 'foo:')
254
255=== modified file 'breezy/tests/test_bundle.py'
256--- breezy/tests/test_bundle.py 2019-01-01 21:23:40 +0000
257+++ breezy/tests/test_bundle.py 2019-02-14 22:08:33 +0000
258@@ -1848,7 +1848,7 @@
259 class FooService(object):
260 """A directory service that always returns source"""
261
262- def look_up(self, name, url):
263+ def look_up(self, name, url, purpose=None):
264 return 'source'
265 directories.register('foo:', FooService, 'Testing directory service')
266 self.addCleanup(directories.remove, 'foo:')
267
268=== modified file 'breezy/tests/test_directory_service.py'
269--- breezy/tests/test_directory_service.py 2018-11-11 04:08:32 +0000
270+++ breezy/tests/test_directory_service.py 2019-02-14 22:08:33 +0000
271@@ -36,7 +36,7 @@
272 # eg 'file:///foo' on Unix, or 'file:///C:/foo' on Windows
273 base = urlutils.local_path_to_url('/foo')
274
275- def look_up(self, name, url):
276+ def look_up(self, name, url, purpose=None):
277 return self.base + name
278
279
280@@ -55,7 +55,12 @@
281 def test_dereference(self):
282 self.assertEqual(FooService.base + 'bar',
283 self.registry.dereference('foo:bar'))
284+ self.assertEqual(FooService.base + 'bar',
285+ self.registry.dereference('foo:bar', purpose='write'))
286 self.assertEqual('baz:qux', self.registry.dereference('baz:qux'))
287+ self.assertEqual(
288+ 'baz:qux',
289+ self.registry.dereference('baz:qux', purpose='write'))
290
291 def test_get_transport(self):
292 directories.register('foo:', FooService, 'Map foo URLs to http urls')
293@@ -64,6 +69,36 @@
294 transport.get_transport('foo:bar').base)
295
296
297+class OldService(object):
298+ """A directory service that maps the name to a FILE url"""
299+
300+ # eg 'file:///foo' on Unix, or 'file:///C:/foo' on Windows
301+ base = urlutils.local_path_to_url('/foo')
302+
303+ def look_up(self, name, url):
304+ return self.base + name
305+
306+
307+class TestOldDirectoryLookup(TestCase):
308+ """Test compatibility with older implementations of Directory
309+ that don't support the purpose argument."""
310+
311+ def setUp(self):
312+ super(TestOldDirectoryLookup, self).setUp()
313+ self.registry = DirectoryServiceRegistry()
314+ self.registry.register('old:', OldService, 'Map foo URLs to http urls')
315+
316+ def test_dereference(self):
317+ self.assertEqual(OldService.base + 'bar',
318+ self.registry.dereference('old:bar'))
319+ self.assertEqual(OldService.base + 'bar',
320+ self.registry.dereference('old:bar', purpose='write'))
321+ self.assertEqual('baz:qux', self.registry.dereference('baz:qux'))
322+ self.assertEqual(
323+ 'baz:qux',
324+ self.registry.dereference('baz:qux', purpose='write'))
325+
326+
327 class TestAliasDirectory(TestCaseWithTransport):
328
329 def setUp(self):
330
331=== modified file 'breezy/tests/test_location.py'
332--- breezy/tests/test_location.py 2019-02-09 02:03:11 +0000
333+++ breezy/tests/test_location.py 2019-02-14 22:08:33 +0000
334@@ -30,7 +30,7 @@
335
336 class SomeDirectory(object):
337
338- def look_up(self, name, url):
339+ def look_up(self, name, url, purpose=None):
340 return "http://bar"
341
342
343@@ -78,7 +78,7 @@
344 def test_rewrite_hook(self):
345 self.assertEqual(
346 'http://foo.example.com/blah', location_to_url('http://foo.example.com/blah'))
347- def rewrite_url(url):
348+ def rewrite_url(url, purpose=None):
349 return url.replace('foo', 'bar')
350 self.addCleanup(location_hooks.uninstall_named_hook, 'rewrite_url', 'test')
351 location_hooks.install_named_hook('rewrite_url', rewrite_url, 'test')
352
353=== modified file 'breezy/transport/__init__.py'
354--- breezy/transport/__init__.py 2018-12-10 05:22:17 +0000
355+++ breezy/transport/__init__.py 2019-02-14 22:08:33 +0000
356@@ -1553,7 +1553,7 @@
357 raise errors.UnsupportedProtocol(url, last_err)
358
359
360-def get_transport(base, possible_transports=None):
361+def get_transport(base, possible_transports=None, purpose=None):
362 """Open a transport to access a URL or directory.
363
364 :param base: either a URL or a directory name.
365@@ -1561,13 +1561,17 @@
366 :param transports: optional reusable transports list. If not None, created
367 transports will be added to the list.
368
369+ :param purpose: Purpose for which the transport will be used
370+ (e.g. 'read', 'write' or None)
371+
372 :return: A new transport optionally sharing its connection with one of
373 possible_transports.
374 """
375 if base is None:
376 base = '.'
377 return get_transport_from_url(
378- _mod_location.location_to_url(base), possible_transports)
379+ _mod_location.location_to_url(base, purpose=purpose),
380+ possible_transports)
381
382
383 def _try_transport_factories(base, factory_list):

Subscribers

People subscribed via source and target branches