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

[Jelmer Vernooij]

Add a purpose argument to location dereference functions.

Add a purpose argument to location dereference functions.

[Martin Packman (gz)]

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

Change push => write

[Jelmer Vernooij (jelmer)]

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.

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/245/

=== modified file 'breezy/builtins.py'
--- breezy/builtins.py	2019-02-04 01:01:24 +0000
+++ breezy/builtins.py	2019-02-14 22:08:33 +0000
@@ -1480,7 +1480,7 @@
             revision_id = br_from.last_revision()
         if to_location is None:
             to_location = urlutils.derive_to_location(from_location)
-        to_transport = transport.get_transport(to_location)
+        to_transport = transport.get_transport(to_location, purpose='write')
         try:
             to_transport.mkdir('.')
         except errors.FileExists:
@@ -1574,7 +1574,7 @@
 
     def run(self, location=".", recursive=False):
         if recursive:
-            t = transport.get_transport(location)
+            t = transport.get_transport(location, purpose='read')
             if not t.listable():
                 raise errors.BzrCommandError(
                     "Can't scan this type of location.")
@@ -2058,7 +2058,7 @@
         if location is None:
             location = u'.'
 
-        to_transport = transport.get_transport(location)
+        to_transport = transport.get_transport(location, purpose='write')
 
         # The path has to exist to initialize a
         # branch inside of it.
@@ -2178,7 +2178,7 @@
         if location is None:
             location = '.'
 
-        to_transport = transport.get_transport(location)
+        to_transport = transport.get_transport(location, purpose='write')
 
         if format.fixed_components:
             repo_format_name = None
 
=== modified file 'breezy/directory_service.py'
--- breezy/directory_service.py	2018-11-11 04:08:32 +0000
+++ breezy/directory_service.py	2019-02-14 22:08:33 +0000
@@ -67,7 +67,7 @@
     name and URL, and return a URL.
     """
 
-    def dereference(self, url):
+    def dereference(self, url, purpose=None):
         """Dereference a supplied URL if possible.
 
         URLs that match a registered directory service prefix are looked up in
@@ -77,19 +77,39 @@
         requires further dereferencing.
 
         :param url: The URL to dereference
+        :param purpose: Purpose of the URL ('read', 'write' or None - if not declared)
         :return: The dereferenced URL if applicable, the input URL otherwise.
         """
         match = self.get_prefix(url)
         if match is None:
             return url
         service, name = match
-        return service().look_up(name, url)
+        directory = service()
+        try:
+            return directory.look_up(name, url, purpose=purpose)
+        except TypeError:
+            # Compatibility for plugins written for Breezy < 3.0.0
+            return directory.look_up(name, url)
 
 
 directories = DirectoryServiceRegistry()
 
 
-class AliasDirectory(object):
+class Directory(object):
+    """Abstract directory lookup class."""
+
+    def look_up(self, name, url, purpose=None):
+        """Look up an entry in a directory.
+
+        :param name: Directory name
+        :param url: The URL to dereference
+        :param purpose: Purpose of the URL ('read', 'write' or None - if not declared)
+        :return: The dereferenced URL if applicable, the input URL otherwise.
+        """
+        raise NotImplementedError(self.look_up)
+
+
+class AliasDirectory(Directory):
     """Directory lookup for locations associated with a branch.
 
     :parent, :submit, :public, :push, :this, and :bound are currently
@@ -110,7 +130,7 @@
     branch_aliases.register('this', lambda b: b.base,
                             help="This branch.")
 
-    def look_up(self, name, url):
+    def look_up(self, name, url, purpose=None):
         branch = _mod_branch.Branch.open_containing('.')[0]
         parts = url.split('/', 1)
         if len(parts) == 2:
@@ -156,14 +176,14 @@
                      'Easy access to remembered branch locations')
 
 
-class ColocatedDirectory(object):
+class ColocatedDirectory(Directory):
     """Directory lookup for colocated branches.
 
     co:somename will resolve to the colocated branch with "somename" in
     the current directory.
     """
 
-    def look_up(self, name, url):
+    def look_up(self, name, url, purpose=None):
         dir = _mod_controldir.ControlDir.open_containing('.')[0]
         return urlutils.join_segment_parameters(dir.user_url,
                                                 {"branch": urlutils.escape(name)})
 
=== modified file 'breezy/git/directory.py'
--- breezy/git/directory.py	2018-11-11 04:08:32 +0000
+++ breezy/git/directory.py	2019-02-14 22:08:33 +0000
@@ -26,6 +26,6 @@
 
 class GitHubDirectory(object):
 
-    def look_up(self, name, url):
+    def look_up(self, name, url, purpose=None):
         """See DirectoryService.look_up"""
         return "git+ssh://[e-mail]/" + name
 
=== modified file 'breezy/location.py'
--- breezy/location.py	2019-02-09 02:03:11 +0000
+++ breezy/location.py	2019-02-14 22:08:33 +0000
@@ -37,26 +37,28 @@
         Hooks.__init__(self, "breezy.location", "hooks")
         self.add_hook(
             'rewrite_url',
-            "Called with a URL to rewrite.", (3, 0))
+            "Possibly rewrite a URL. Called with a URL to rewrite and the "
+            "purpose of the URL.", (3, 0))
 
 
 hooks = LocationHooks()
 
 
-def location_to_url(location):
+def location_to_url(location, purpose=None):
     """Determine a fully qualified URL from a location string.
 
     This will try to interpret location as both a URL and a directory path. It
     will also lookup the location in directories.
 
     :param location: Unicode or byte string object with a location
+    :param purpose: Intended method of access (None, 'read' or 'write')
     :raise InvalidURL: If the location is already a URL, but not valid.
     :return: Byte string with resulting URL
     """
     if not isinstance(location, string_types):
         raise AssertionError("location not a byte or unicode string")
     from .directory_service import directories
-    location = directories.dereference(location)
+    location = directories.dereference(location, purpose)
 
     # Catch any URLs which are passing Unicode rather than ASCII
     try:
@@ -77,6 +79,6 @@
         return urlutils.local_path_to_url(location)
 
     for hook in hooks['rewrite_url']:
-        location = hook(location)
+        location = hook(location, purpose=purpose)
 
     return location
 
=== modified file 'breezy/plugins/launchpad/__init__.py'
--- breezy/plugins/launchpad/__init__.py	2019-02-02 22:23:40 +0000
+++ breezy/plugins/launchpad/__init__.py	2019-02-14 22:08:33 +0000
@@ -29,7 +29,6 @@
 
     launchpad-login: Show or set the Launchpad user ID
     launchpad-open: Open a Launchpad branch page in your web browser
-    launchpad-mirror: Ask Launchpad to mirror a branch now
 
 As well as the following deprecated command:
 
 
=== modified file 'breezy/plugins/launchpad/lp_directory.py'
--- breezy/plugins/launchpad/lp_directory.py	2019-02-02 22:23:40 +0000
+++ breezy/plugins/launchpad/lp_directory.py	2019-02-14 22:08:33 +0000
@@ -117,7 +117,7 @@
 
 class LaunchpadDirectory(object):
 
-    def look_up(self, name, url):
+    def look_up(self, name, url, purpose=None):
         """See DirectoryService.look_up"""
         return self._resolve(url)
 
 
=== modified file 'breezy/plugins/launchpad/test_lp_directory.py'
--- breezy/plugins/launchpad/test_lp_directory.py	2019-01-17 01:04:56 +0000
+++ breezy/plugins/launchpad/test_lp_directory.py	2019-02-14 22:08:33 +0000
@@ -397,7 +397,7 @@
         class FooService(object):
             """A directory service that maps the name to a FILE url"""
 
-            def look_up(self, name, url):
+            def look_up(self, name, url, purpose=None):
                 if 'lp:///apt' == url:
                     return target_branch.base.rstrip('/')
                 return '!unexpected look_up value!'
 
=== modified file 'breezy/push.py'
--- breezy/push.py	2018-11-17 16:53:10 +0000
+++ breezy/push.py	2019-02-14 22:08:33 +0000
@@ -81,7 +81,7 @@
         directory exists without a current control directory in it
     :param lossy: Allow lossy push
     """
-    to_transport = transport.get_transport(location)
+    to_transport = transport.get_transport(location, purpose='write')
     try:
         dir_to = controldir.ControlDir.open_from_transport(to_transport)
     except errors.NotBranchError:
 
=== modified file 'breezy/tests/blackbox/test_pull.py'
--- breezy/tests/blackbox/test_pull.py	2018-11-29 23:42:41 +0000
+++ breezy/tests/blackbox/test_pull.py	2019-02-14 22:08:33 +0000
@@ -353,7 +353,7 @@
         class FooService(object):
             """A directory service that always returns source"""
 
-            def look_up(self, name, url):
+            def look_up(self, name, url, purpose=None):
                 return 'source'
         directories.register('foo:', FooService, 'Testing directory service')
         self.addCleanup(directories.remove, 'foo:')
 
=== modified file 'breezy/tests/blackbox/test_switch.py'
--- breezy/tests/blackbox/test_switch.py	2019-01-04 21:21:12 +0000
+++ breezy/tests/blackbox/test_switch.py	2019-02-14 22:08:33 +0000
@@ -347,7 +347,7 @@
         tree = branch.create_checkout('tree', lightweight=True)
 
         class FooLookup(object):
-            def look_up(self, name, url):
+            def look_up(self, name, url, purpose=None):
                 return 'foo-' + name
         directories.register('foo:', FooLookup, 'Create branches named foo-')
         self.addCleanup(directories.remove, 'foo:')
 
=== modified file 'breezy/tests/test_bundle.py'
--- breezy/tests/test_bundle.py	2019-01-01 21:23:40 +0000
+++ breezy/tests/test_bundle.py	2019-02-14 22:08:33 +0000
@@ -1848,7 +1848,7 @@
         class FooService(object):
             """A directory service that always returns source"""
 
-            def look_up(self, name, url):
+            def look_up(self, name, url, purpose=None):
                 return 'source'
         directories.register('foo:', FooService, 'Testing directory service')
         self.addCleanup(directories.remove, 'foo:')
 
=== modified file 'breezy/tests/test_directory_service.py'
--- breezy/tests/test_directory_service.py	2018-11-11 04:08:32 +0000
+++ breezy/tests/test_directory_service.py	2019-02-14 22:08:33 +0000
@@ -36,7 +36,7 @@
     # eg 'file:///foo' on Unix, or 'file:///C:/foo' on Windows
     base = urlutils.local_path_to_url('/foo')
 
-    def look_up(self, name, url):
+    def look_up(self, name, url, purpose=None):
         return self.base + name
 
 
@@ -55,7 +55,12 @@
     def test_dereference(self):
         self.assertEqual(FooService.base + 'bar',
                          self.registry.dereference('foo:bar'))
+        self.assertEqual(FooService.base + 'bar',
+                         self.registry.dereference('foo:bar', purpose='write'))
         self.assertEqual('baz:qux', self.registry.dereference('baz:qux'))
+        self.assertEqual(
+            'baz:qux',
+            self.registry.dereference('baz:qux', purpose='write'))
 
     def test_get_transport(self):
         directories.register('foo:', FooService, 'Map foo URLs to http urls')
@@ -64,6 +69,36 @@
                          transport.get_transport('foo:bar').base)
 
 
+class OldService(object):
+    """A directory service that maps the name to a FILE url"""
+
+    # eg 'file:///foo' on Unix, or 'file:///C:/foo' on Windows
+    base = urlutils.local_path_to_url('/foo')
+
+    def look_up(self, name, url):
+        return self.base + name
+
+
+class TestOldDirectoryLookup(TestCase):
+    """Test compatibility with older implementations of Directory
+    that don't support the purpose argument."""
+
+    def setUp(self):
+        super(TestOldDirectoryLookup, self).setUp()
+        self.registry = DirectoryServiceRegistry()
+        self.registry.register('old:', OldService, 'Map foo URLs to http urls')
+
+    def test_dereference(self):
+        self.assertEqual(OldService.base + 'bar',
+                         self.registry.dereference('old:bar'))
+        self.assertEqual(OldService.base + 'bar',
+                         self.registry.dereference('old:bar', purpose='write'))
+        self.assertEqual('baz:qux', self.registry.dereference('baz:qux'))
+        self.assertEqual(
+            'baz:qux',
+            self.registry.dereference('baz:qux', purpose='write'))
+
+
 class TestAliasDirectory(TestCaseWithTransport):
 
     def setUp(self):
 
=== modified file 'breezy/tests/test_location.py'
--- breezy/tests/test_location.py	2019-02-09 02:03:11 +0000
+++ breezy/tests/test_location.py	2019-02-14 22:08:33 +0000
@@ -30,7 +30,7 @@
 
 class SomeDirectory(object):
 
-    def look_up(self, name, url):
+    def look_up(self, name, url, purpose=None):
         return "http://bar"
 
 
@@ -78,7 +78,7 @@
     def test_rewrite_hook(self):
         self.assertEqual(
             'http://foo.example.com/blah', location_to_url('http://foo.example.com/blah'))
-        def rewrite_url(url):
+        def rewrite_url(url, purpose=None):
             return url.replace('foo', 'bar')
         self.addCleanup(location_hooks.uninstall_named_hook, 'rewrite_url', 'test')
         location_hooks.install_named_hook('rewrite_url', rewrite_url, 'test')
 
=== modified file 'breezy/transport/__init__.py'
--- breezy/transport/__init__.py	2018-12-10 05:22:17 +0000
+++ breezy/transport/__init__.py	2019-02-14 22:08:33 +0000
@@ -1553,7 +1553,7 @@
     raise errors.UnsupportedProtocol(url, last_err)
 
 
-def get_transport(base, possible_transports=None):
+def get_transport(base, possible_transports=None, purpose=None):
     """Open a transport to access a URL or directory.
 
     :param base: either a URL or a directory name.
@@ -1561,13 +1561,17 @@
     :param transports: optional reusable transports list. If not None, created
         transports will be added to the list.
 
+    :param purpose: Purpose for which the transport will be used
+        (e.g. 'read', 'write' or None)
+
     :return: A new transport optionally sharing its connection with one of
         possible_transports.
     """
     if base is None:
         base = '.'
     return get_transport_from_url(
-        _mod_location.location_to_url(base), possible_transports)
+        _mod_location.location_to_url(base, purpose=purpose),
+        possible_transports)
 
 
 def _try_transport_factories(base, factory_list):