Merge lp:~cjwatson/bzr/remove-register-branch into lp:bzr

[Colin Watson]

Remove `bzr register-branch`, since it has not worked for a long time.

The `bzr register-branch` command has been broken for a long time with no bug reports:

 * It relies on password authentication to Launchpad, which was deprecated in favour of SSO many years ago and disabled entirely in January 2012.
 * Its entire purpose is to create mirrored branches, which are generally deprecated in favour of code imports, and the mirroring of those branches no longer works because the relevant systems haven't had firewall-level access to the outside world for quite some time.

There's really not much point trying to resurrect this command and convert it to create code imports with modern authentication. It should just be removed.

Unmerged revisions

6623. By Colin Watson on 2017-05-23

Remove `bzr register-branch`, since it has not worked for a long time.

=== modified file 'bzrlib/plugins/launchpad/__init__.py'
--- bzrlib/plugins/launchpad/__init__.py	2012-03-10 19:11:06 +0000
+++ bzrlib/plugins/launchpad/__init__.py	2017-05-23 12:36:28 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006-2011 Canonical Ltd
+# Copyright (C) 2006-2017 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -30,7 +30,6 @@
     launchpad-login: Show or set the Launchpad user ID
     launchpad-open: Open a Launchpad branch page in your web browser
     lp-propose-merge: Propose merging a branch on Launchpad
-    register-branch: Register a branch with launchpad.net
     launchpad-mirror: Ask Launchpad to mirror a branch now
 
 """
@@ -57,7 +56,6 @@
 from bzrlib.help_topics import topic_registry
 
 for klsname, aliases in [
-    ("cmd_register_branch", []),
     ("cmd_launchpad_open", ["lp-open"]),
     ("cmd_launchpad_login", ["lp-login"]),
     ("cmd_launchpad_mirror", ["lp-mirror"]),
@@ -177,11 +175,6 @@
       Launchpad bug 12345. When you push that branch to Launchpad it will
       automatically be linked to the bug report.
 
-    * The register-branch command tells Launchpad about the url of a
-      public branch.  Launchpad will then mirror the branch, display
-      its contents and allow it to be attached to bugs and other
-      objects.
-
 For more information see http://help.launchpad.net/
 """
 topic_registry.register('launchpad',

=== modified file 'bzrlib/plugins/launchpad/cmds.py'
--- bzrlib/plugins/launchpad/cmds.py	2012-10-22 15:51:31 +0000
+++ bzrlib/plugins/launchpad/cmds.py	2017-05-23 12:36:28 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006-2012 Canonical Ltd
+# Copyright (C) 2006-2017 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -29,7 +29,6 @@
 from bzrlib.errors import (
     BzrCommandError,
     InvalidURL,
-    NoPublicBranch,
     NotBranchError,
     )
 from bzrlib.i18n import gettext
@@ -39,114 +38,6 @@
     )
 
 
-class cmd_register_branch(Command):
-    __doc__ = """Register a branch with launchpad.net.
-
-    This command lists a bzr branch in the directory of branches on
-    launchpad.net.  Registration allows the branch to be associated with
-    bugs or specifications.
-
-    Before using this command you must register the project to which the
-    branch belongs, and create an account for yourself on launchpad.net.
-
-    arguments:
-        public_url: The publicly visible url for the branch to register.
-                    This must be an http or https url (which Launchpad can read
-                    from to access the branch). Local file urls, SFTP urls, and
-                    bzr+ssh urls will not work.
-                    If no public_url is provided, bzr will use the configured
-                    public_url if there is one for the current branch, and
-                    otherwise error.
-
-    example:
-        bzr register-branch http://foo.com/bzr/fooproject.mine \\
-                --project fooproject
-    """
-    takes_args = ['public_url?']
-    takes_options = [
-         Option('project',
-                'Launchpad project short name to associate with the branch.',
-                unicode),
-         Option('product',
-                'Launchpad product short name to associate with the branch.',
-                unicode,
-                hidden=True),
-         Option('branch-name',
-                'Short name for the branch; '
-                'by default taken from the last component of the url.',
-                unicode),
-         Option('branch-title',
-                'One-sentence description of the branch.',
-                unicode),
-         Option('branch-description',
-                'Longer description of the purpose or contents of the branch.',
-                unicode),
-         Option('author',
-                "Branch author's email address, if not yourself.",
-                unicode),
-         Option('link-bug',
-                'The bug this branch fixes.',
-                int),
-         Option('dry-run',
-                'Prepare the request but don\'t actually send it.')
-        ]
-
-
-    def run(self,
-            public_url=None,
-            project='',
-            product=None,
-            branch_name='',
-            branch_title='',
-            branch_description='',
-            author='',
-            link_bug=None,
-            dry_run=False):
-        from bzrlib.plugins.launchpad.lp_registration import (
-            BranchRegistrationRequest, BranchBugLinkRequest,
-            DryRunLaunchpadService, LaunchpadService)
-        if public_url is None:
-            try:
-                b = _mod_branch.Branch.open_containing('.')[0]
-            except NotBranchError:
-                raise BzrCommandError(gettext(
-                            'register-branch requires a public '
-                            'branch url - see bzr help register-branch.'))
-            public_url = b.get_public_branch()
-            if public_url is None:
-                raise NoPublicBranch(b)
-        if product is not None:
-            project = product
-            trace.note(gettext(
-                '--product is deprecated; please use --project.'))
-
-
-        rego = BranchRegistrationRequest(branch_url=public_url,
-                                         branch_name=branch_name,
-                                         branch_title=branch_title,
-                                         branch_description=branch_description,
-                                         product_name=project,
-                                         author_email=author,
-                                         )
-        linko = BranchBugLinkRequest(branch_url=public_url,
-                                     bug_id=link_bug)
-        if not dry_run:
-            service = LaunchpadService()
-            # This gives back the xmlrpc url that can be used for future
-            # operations on the branch.  It's not so useful to print to the
-            # user since they can't do anything with it from a web browser; it
-            # might be nice for the server to tell us about an html url as
-            # well.
-        else:
-            # Run on service entirely in memory
-            service = DryRunLaunchpadService()
-        service.gather_user_credentials()
-        rego.submit(service)
-        if link_bug:
-            linko.submit(service)
-        self.outf.write('Branch registered.\n')
-
-
 class cmd_launchpad_open(Command):
     __doc__ = """Open a Launchpad branch page in your web browser."""
 

=== modified file 'bzrlib/plugins/launchpad/lp_registration.py'
--- bzrlib/plugins/launchpad/lp_registration.py	2012-01-20 13:07:10 +0000
+++ bzrlib/plugins/launchpad/lp_registration.py	2017-05-23 12:36:28 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006-2011 Canonical Ltd
+# Copyright (C) 2006-2017 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -144,51 +144,13 @@
             raise errors.InvalidURL(path=url)
         return cls(lp_instance=lp_instance, **kwargs)
 
-    def get_proxy(self, authenticated):
+    def get_proxy(self):
         """Return the proxy for XMLRPC requests."""
-        if authenticated:
-            # auth info must be in url
-            # TODO: if there's no registrant email perhaps we should
-            # just connect anonymously?
-            scheme, hostinfo, path = urlsplit(self.service_url)[:3]
-            if '@' in hostinfo:
-                raise AssertionError(hostinfo)
-            if self.registrant_email is None:
-                raise AssertionError()
-            if self.registrant_password is None:
-                raise AssertionError()
-            # TODO: perhaps fully quote the password to make it very slightly
-            # obscured
-            # TODO: can we perhaps add extra Authorization headers
-            # directly to the request, rather than putting this into
-            # the url?  perhaps a bit more secure against accidentally
-            # revealing it.  std66 s3.2.1 discourages putting the
-            # password in the url.
-            hostinfo = '%s:%s@%s' % (urlutils.quote(self.registrant_email),
-                                     urlutils.quote(self.registrant_password),
-                                     hostinfo)
-            url = urlunsplit((scheme, hostinfo, path, '', ''))
-        else:
-            url = self.service_url
+        url = self.service_url
         return xmlrpclib.ServerProxy(url, transport=self.transport)
 
-    def gather_user_credentials(self):
-        """Get the password from the user."""
-        the_config = config.GlobalConfig()
-        self.registrant_email = the_config.user_email()
-        if self.registrant_password is None:
-            auth = config.AuthenticationConfig()
-            scheme, hostinfo = urlsplit(self.service_url)[:2]
-            prompt = 'launchpad.net password for %s: ' % \
-                    self.registrant_email
-            # We will reuse http[s] credentials if we can, prompt user
-            # otherwise
-            self.registrant_password = auth.get_password(scheme, hostinfo,
-                                                         self.registrant_email,
-                                                         prompt=prompt)
-
-    def send_request(self, method_name, method_params, authenticated):
-        proxy = self.get_proxy(authenticated)
+    def send_request(self, method_name, method_params):
+        proxy = self.get_proxy()
         method = getattr(proxy, method_name)
         try:
             result = method(*method_params)
@@ -255,7 +217,6 @@
 
     # Set this to the XMLRPC method name.
     _methodname = None
-    _authenticated = True
 
     def _request_params(self):
         """Return the arguments to pass to the method"""
@@ -264,88 +225,22 @@
     def submit(self, service):
         """Submit request to Launchpad XMLRPC server.
 
-        :param service: LaunchpadService indicating where to send
-            the request and the authentication credentials.
+        :param service: LaunchpadService indicating where to send the request.
         """
-        return service.send_request(self._methodname, self._request_params(),
-                                    self._authenticated)
+        return service.send_request(self._methodname, self._request_params())
 
 
 class DryRunLaunchpadService(LaunchpadService):
-    """Service that just absorbs requests without sending to server.
-
-    The dummy service does not need authentication.
-    """
-
-    def send_request(self, method_name, method_params, authenticated):
-        pass
-
-    def gather_user_credentials(self):
-        pass
-
-
-class BranchRegistrationRequest(BaseRequest):
-    """Request to tell Launchpad about a bzr branch."""
-
-    _methodname = 'register_branch'
-
-    def __init__(self, branch_url,
-                 branch_name='',
-                 branch_title='',
-                 branch_description='',
-                 author_email='',
-                 product_name='',
-                 ):
-        if not branch_url:
-            raise errors.InvalidURL(branch_url, "You need to specify a non-empty branch URL.")
-        self.branch_url = branch_url
-        if branch_name:
-            self.branch_name = branch_name
-        else:
-            self.branch_name = self._find_default_branch_name(self.branch_url)
-        self.branch_title = branch_title
-        self.branch_description = branch_description
-        self.author_email = author_email
-        self.product_name = product_name
-
-    def _request_params(self):
-        """Return xmlrpc request parameters"""
-        # This must match the parameter tuple expected by Launchpad for this
-        # method
-        return (self.branch_url,
-                self.branch_name,
-                self.branch_title,
-                self.branch_description,
-                self.author_email,
-                self.product_name,
-               )
-
-    def _find_default_branch_name(self, branch_url):
-        i = branch_url.rfind('/')
-        return branch_url[i+1:]
-
-
-class BranchBugLinkRequest(BaseRequest):
-    """Request to link a bzr branch in Launchpad to a bug."""
-
-    _methodname = 'link_branch_to_bug'
-
-    def __init__(self, branch_url, bug_id):
-        self.bug_id = bug_id
-        self.branch_url = branch_url
-
-    def _request_params(self):
-        """Return xmlrpc request parameters"""
-        # This must match the parameter tuple expected by Launchpad for this
-        # method
-        return (self.branch_url, self.bug_id, '')
+    """Service that just absorbs requests without sending to server."""
+
+    def send_request(self, method_name, method_params):
+        pass
 
 
 class ResolveLaunchpadPathRequest(BaseRequest):
     """Request to resolve the path component of an lp: URL."""
 
     _methodname = 'resolve_lp_path'
-    _authenticated = False
 
     def __init__(self, path):
         if not path:

=== modified file 'bzrlib/plugins/launchpad/test_register.py'
--- bzrlib/plugins/launchpad/test_register.py	2016-02-01 18:06:32 +0000
+++ bzrlib/plugins/launchpad/test_register.py	2017-05-23 12:36:28 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006-2012, 2016 Canonical Ltd
+# Copyright (C) 2006-2012, 2016-2017 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -16,21 +16,13 @@
 
 import base64
 from StringIO import StringIO
-import urlparse
 import xmlrpclib
 
-from bzrlib import (
-    config,
-    tests,
-    ui,
-    )
 from bzrlib.tests import TestCaseWithTransport
 
 # local import
 from bzrlib.plugins.launchpad.lp_registration import (
         BaseRequest,
-        BranchBugLinkRequest,
-        BranchRegistrationRequest,
         ResolveLaunchpadPathRequest,
         LaunchpadService,
         )
@@ -41,11 +33,6 @@
 # the results passed in.  Not sure how to get the transport object back out to
 # validate that its OK - may not be necessary.
 
-# TODO: Add test for (and implement) other command-line options to set
-# project, author_email, description.
-
-# TODO: project_id is not properly handled -- must be passed in rpc or path.
-
 class InstrumentedXMLRPCConnection(object):
     """Stands in place of an http connection for the purposes of testing"""
 
@@ -102,25 +89,15 @@
     # Python 2.5's xmlrpclib looks for this.
     _use_datetime = False
 
-    def __init__(self, testcase, expect_auth):
+    def __init__(self, testcase):
         self.testcase = testcase
-        self.expect_auth = expect_auth
         self._connection = (None, None)
 
     def make_connection(self, host):
         host, http_headers, x509 = self.get_host_info(host)
         test = self.testcase
         self.connected_host = host
-        if self.expect_auth:
-            auth_hdrs = [v for k,v in http_headers if k == 'Authorization']
-            if len(auth_hdrs) != 1:
-                raise AssertionError("multiple auth headers: %r"
-                    % (auth_hdrs,))
-            authinfo = auth_hdrs[0]
-            expected_auth = '[e-mail]:testpassword'
-            test.assertEqual(authinfo,
-                    'Basic ' + base64.encodestring(expected_auth).strip())
-        elif http_headers:
+        if http_headers:
             raise AssertionError()
         return InstrumentedXMLRPCConnection(test)
 
@@ -146,80 +123,22 @@
 
 class MockLaunchpadService(LaunchpadService):
 
-    def send_request(self, method_name, method_params, authenticated):
+    def send_request(self, method_name, method_params):
         """Stash away the method details rather than sending them to a real server"""
         self.called_method_name = method_name
         self.called_method_params = method_params
-        self.called_authenticated = authenticated
-
-
-class TestBranchRegistration(TestCaseWithTransport):
+
+
+class TestResolveLaunchpadPathRequest(TestCaseWithTransport):
 
     def setUp(self):
-        super(TestBranchRegistration, self).setUp()
+        super(TestResolveLaunchpadPathRequest, self).setUp()
         # make sure we have a reproducible standard environment
         self.overrideEnv('BZR_LP_XMLRPC_URL', None)
 
-    def test_register_help(self):
-        """register-branch accepts --help"""
-        out, err = self.run_bzr(['register-branch', '--help'])
-        self.assertContainsRe(out, r'Register a branch')
-
-    def test_register_no_url_no_branch(self):
-        """register-branch command requires parameters"""
-        self.make_repository('.')
-        self.run_bzr_error(
-            ['register-branch requires a public branch url - '
-             'see bzr help register-branch'],
-            'register-branch')
-
-    def test_register_no_url_in_published_branch_no_error(self):
-        b = self.make_branch('.')
-        b.set_public_branch('http://test-server.com/bzr/branch')
-        out, err = self.run_bzr(['register-branch', '--dry-run'])
-        self.assertEqual('Branch registered.\n', out)
-        self.assertEqual('', err)
-
-    def test_register_no_url_in_unpublished_branch_errors(self):
-        b = self.make_branch('.')
-        out, err = self.run_bzr_error(['no public branch'],
-            ['register-branch', '--dry-run'])
-        self.assertEqual('', out)
-
-    def test_register_dry_run(self):
-        out, err = self.run_bzr(['register-branch',
-                                'http://test-server.com/bzr/branch',
-                                '--dry-run'])
-        self.assertEqual(out, 'Branch registered.\n')
-
     def test_onto_transport(self):
-        """How the request is sent by transmitting across a mock Transport"""
-        # use a real transport, but intercept at the http/xml layer
-        transport = InstrumentedXMLRPCTransport(self, expect_auth=True)
-        service = LaunchpadService(transport)
-        service.registrant_email = '[e-mail]'
-        service.registrant_password = 'testpassword'
-        rego = BranchRegistrationRequest('http://test-server.com/bzr/branch',
-                'branch-id',
-                'my test branch',
-                'description',
-                '[e-mail]',
-                'product')
-        rego.submit(service)
-        self.assertEqual(transport.connected_host, 'xmlrpc.launchpad.net')
-        self.assertEqual(len(transport.sent_params), 6)
-        self.assertEqual(transport.sent_params,
-                ('http://test-server.com/bzr/branch',  # branch_url
-                 'branch-id',                          # branch_name
-                 'my test branch',                     # branch_title
-                 'description',
-                 '[e-mail]',
-                 'product'))
-        self.assertTrue(transport.got_request)
-
-    def test_onto_transport_unauthenticated(self):
-        """An unauthenticated request is transmitted across a mock Transport"""
-        transport = InstrumentedXMLRPCTransport(self, expect_auth=False)
+        """A request is transmitted across a mock Transport"""
+        transport = InstrumentedXMLRPCTransport(self)
         service = LaunchpadService(transport)
         resolve = ResolveLaunchpadPathRequest('bzr')
         resolve.submit(service)
@@ -243,59 +162,12 @@
         self.assertEqual(service.called_method_name, 'dummy_request')
         self.assertEqual(service.called_method_params, (42,))
 
-    def test_mock_server_registration(self):
-        """Send registration to mock server"""
-        test_case = self
-        class MockRegistrationService(MockLaunchpadService):
-            def send_request(self, method_name, method_params, authenticated):
-                test_case.assertEqual(method_name, "register_branch")
-                test_case.assertEqual(list(method_params),
-                        ['url', 'name', 'title', 'description', 'email', 'name'])
-                test_case.assertEqual(authenticated, True)
-                return 'result'
-        service = MockRegistrationService()
-        rego = BranchRegistrationRequest('url', 'name', 'title',
-                        'description', 'email', 'name')
-        result = rego.submit(service)
-        self.assertEqual(result, 'result')
-
-    def test_mock_server_registration_with_defaults(self):
-        """Send registration to mock server"""
-        test_case = self
-        class MockRegistrationService(MockLaunchpadService):
-            def send_request(self, method_name, method_params, authenticated):
-                test_case.assertEqual(method_name, "register_branch")
-                test_case.assertEqual(list(method_params),
-                        ['http://server/branch', 'branch', '', '', '', ''])
-                test_case.assertEqual(authenticated, True)
-                return 'result'
-        service = MockRegistrationService()
-        rego = BranchRegistrationRequest('http://server/branch')
-        result = rego.submit(service)
-        self.assertEqual(result, 'result')
-
-    def test_mock_bug_branch_link(self):
-        """Send bug-branch link to mock server"""
-        test_case = self
-        class MockService(MockLaunchpadService):
-            def send_request(self, method_name, method_params, authenticated):
-                test_case.assertEqual(method_name, "link_branch_to_bug")
-                test_case.assertEqual(list(method_params),
-                        ['http://server/branch', 1234, ''])
-                test_case.assertEqual(authenticated, True)
-                return 'http://launchpad.net/bug/1234'
-        service = MockService()
-        rego = BranchBugLinkRequest('http://server/branch', 1234)
-        result = rego.submit(service)
-        self.assertEqual(result, 'http://launchpad.net/bug/1234')
-
     def test_mock_resolve_lp_url(self):
         test_case = self
         class MockService(MockLaunchpadService):
-            def send_request(self, method_name, method_params, authenticated):
+            def send_request(self, method_name, method_params):
                 test_case.assertEqual(method_name, "resolve_lp_path")
                 test_case.assertEqual(list(method_params), ['bzr'])
-                test_case.assertEqual(authenticated, False)
                 return dict(urls=[
                         'bzr+ssh://bazaar.launchpad.net~bzr/bzr/trunk',
                         'sftp://bazaar.launchpad.net~bzr/bzr/trunk',
@@ -310,61 +182,3 @@
                 'sftp://bazaar.launchpad.net~bzr/bzr/trunk',
                 'bzr+http://bazaar.launchpad.net~bzr/bzr/trunk',
                 'http://bazaar.launchpad.net~bzr/bzr/trunk'])
-
-
-class TestGatherUserCredentials(tests.TestCaseInTempDir):
-
-    def setUp(self):
-        super(TestGatherUserCredentials, self).setUp()
-        # make sure we have a reproducible standard environment
-        self.overrideEnv('BZR_LP_XMLRPC_URL', None)
-
-    def test_gather_user_credentials_has_password(self):
-        service = LaunchpadService()
-        service.registrant_password = 'mypassword'
-        # This should be a basic no-op, since we already have the password
-        service.gather_user_credentials()
-        self.assertEqual('mypassword', service.registrant_password)
-
-    def test_gather_user_credentials_from_auth_conf(self):
-        auth_path = config.authentication_config_filename()
-        service = LaunchpadService()
-        g_conf = config.GlobalStack()
-        g_conf.set('email', 'Test User <[e-mail]>')
-        g_conf.store.save()
-        # FIXME: auth_path base dir exists only because bazaar.conf has just
-        # been saved, brittle... -- vila 20120731
-        f = open(auth_path, 'wb')
-        try:
-            scheme, hostinfo = urlparse.urlsplit(service.service_url)[:2]
-            f.write('[section]\n'
-                    'scheme=%s\n'
-                    'host=%s\n'
-                    'user=[e-mail]\n'
-                    'password=testpass\n'
-                    % (scheme, hostinfo))
-        finally:
-            f.close()
-        self.assertIs(None, service.registrant_password)
-        service.gather_user_credentials()
-        self.assertEqual('[e-mail]', service.registrant_email)
-        self.assertEqual('testpass', service.registrant_password)
-
-    def test_gather_user_credentials_prompts(self):
-        service = LaunchpadService()
-        self.assertIs(None, service.registrant_password)
-        g_conf = config.GlobalStack()
-        g_conf.set('email', 'Test User <[e-mail]>')
-        g_conf.store.save()
-        stdout = tests.StringIOWrapper()
-        stderr = tests.StringIOWrapper()
-        ui.ui_factory = tests.TestUIFactory(stdin='userpass\n',
-                                            stdout=stdout, stderr=stderr)
-        self.assertIs(None, service.registrant_password)
-        service.gather_user_credentials()
-        self.assertEqual('[e-mail]', service.registrant_email)
-        self.assertEqual('userpass', service.registrant_password)
-        self.assertEqual('', stdout.getvalue())
-        self.assertContainsRe(stderr.getvalue(),
-                             'launchpad.net password for test@user\\.com')
-

=== modified file 'doc/en/release-notes/bzr-2.8.txt'
--- doc/en/release-notes/bzr-2.8.txt	2017-03-17 10:39:02 +0000
+++ doc/en/release-notes/bzr-2.8.txt	2017-05-23 12:36:28 +0000
@@ -15,6 +15,15 @@
 
 .. These may require users to change the way they use Bazaar.
 
+ * The ``bzr register-branch`` command from the Launchpad plugin has been
+   removed, because it has not worked for at least five years: it relies on
+   password authentication rather than SSO, the relevant systems no longer
+   have firewall-level access to the outside world, and in general the
+   Mirrored branch type is deprecated.  Either just push the branch to
+   Launchpad or use code imports instead
+   (https://help.launchpad.net/VcsImports).
+   (Colin Watson, #254567, #483689)
+
 New Features
 ************