Merge ppa-dev-tools:set-command-dependencies into ppa-dev-tools:main

[Bryce Harrington]

This adds support for the --ppa-dependencies option to the create and set commands, to permit adding one or more PPAs for satisfying the given PPA's build dependencies.

I've also added the start of a handy smoketest for the ppa/ppa.py module to run through the basic settings. I plan to expand this to exercise more of the module but for now just tried to establish the basic structure and operation. I've sorted out making it use the 'qastaging' test instance of Launchpad to be able to validate the launchpad operations without actually impacting anything in production. Note that the data from qastaging is quite old and in fact may be missing your user account if you registered within the last few years, so I hope it works but YMMV. If not, you can flip staging off by editing the smoketest thusly:

    lp = Lp('smoketest', staging=False)

There's also a few small cleanup/refactors in separate commits.

Anyway, as usual the smoketest can be run via:

    $ python3 -m ppa.ppa

And all testing run via `pytest-3` or:

    $ make check

[Bryce Harrington (bryce)]

Example output for the smoke test:

$ python3 -m ppa.ppa
##########################
## Ppa class smoke test ##
##########################

setting desc to 'This is a testing PPA and can be deleted'
desc is now 'This is a testing PPA and can be deleted'

name: test-ppa-dxtefi
address: ppa:bryce/test-ppa-dxtefi
str(ppa): bryce/test-ppa-dxtefi
reference: ~bryce/ubuntu/test-ppa-dxtefi
self_link: https://api.qastaging.launchpad.net/devel/~bryce/+archive/ubuntu/test-ppa-dxtefi
web_link: https://qastaging.launchpad.net/~bryce/+archive/ubuntu/test-ppa-dxtefi
description: This is a testing PPA and can be deleted
has_packages: False
architectures: amd64/arm64
dependencies: ~bryce/ubuntu/dependency-ppa-dxtefi
url: https://qastaging.launchpad.net/~bryce/+archive/ubuntu/test-ppa-dxtefi

Ready to cleanup (i.e. delete) temporary PPAs? (y/n) y
...Cleaning up test ppa...

[Lena Voytek (lvoytek)]

Sorry for only getting to this today, got distracted on Friday. Code looks good to me! Added a few comments for cleanup

review: Approve

[Bryce Harrington (bryce)]

Thanks for the review! One comment below, the rest of the suggestions are incorporated, I'll squash and land the branch directly.

[Bryce Harrington (bryce)]

Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
To git+ssh://git.launchpad.net/ppa-dev-tools
   2e6b5d7..d71ef8d main -> main

diff --git a/ppa/lp.py b/ppa/lp.py
index 1762345..17b225a 100644
--- a/ppa/lp.py
+++ b/ppa/lp.py
@@ -39,16 +39,24 @@ class Lp:
 
     _real_instance = None
 
-    def __init__(self, application_name, service=Launchpad):
+    def __init__(self, application_name, service=Launchpad, staging=False):
         """Create a Launchpad service object."""
         self._app_name = application_name
         self._service = service
+        if staging:
+            self._service_root = 'qastaging'
+            self.ROOT_URL = 'https://qastaging.launchpad.net/'
+            self.API_ROOT_URL = 'https://api.qastaging.launchpad.net/devel/'
+            self.BUGS_ROOT_URL = 'https://bugs.qastaging.launchpad.net/'
+            self.CODE_ROOT_URL = 'https://code.qastaging.launchpad.net/'
+        else:
+            self._service_root = 'production'
 
     def _get_instance(self):
         """Authenticate to Launchpad."""
         return self._service.login_with(
             application_name=self._app_name,
-            service_root='production',
+            service_root=self._service_root,
             allow_access_levels=['WRITE_PRIVATE'],
             version='devel',  # Need devel for copyPackage.
         )
diff --git a/ppa/ppa.py b/ppa/ppa.py
index 1536b7d..52894c4 100755
--- a/ppa/ppa.py
+++ b/ppa/ppa.py
@@ -14,7 +14,7 @@ import re
 import sys
 
 from functools import lru_cache
-from lazr.restfulclient.errors import BadRequest, NotFound
+from lazr.restfulclient.errors import BadRequest, NotFound, Unauthorized
 
 
 class PpaDoesNotExist(BaseException):
@@ -141,7 +141,7 @@ class Ppa:
         :rtype: str
         :returns: The url of the PPA.
         """
-        return "https://launchpad.net/~{}/+archive/ubuntu/{}".format(self.team_name, self.ppa_name)
+        return self.archive.web_link
 
     @property
     def description(self):
@@ -209,8 +209,10 @@ class Ppa:
         """
         if not architectures:
             return False
-        uri_base = "https://api.launchpad.net/devel/+processors/{}"
-        procs = [uri_base.format(arch) for arch in architectures]
+        base = self._service.API_ROOT_URL.rstrip('/')
+        procs = []
+        for arch in architectures:
+            procs.append(f'{base}/+processors/{arch}')
         try:
             self.archive.setProcessors(processors=procs)
             return True
@@ -218,6 +220,55 @@ class Ppa:
             sys.stderr.write(e)
             return False
 
+    @property
+    @lru_cache
+    def dependencies(self) -> list[str]:
+        """Returns the additional PPAs configured for building packages in this PPA.
+
+        :rtype: list[str]
+        :returns: List of PPA addresses
+        """
+        ppa_addresses = []
+        for dep in self.archive.dependencies:
+            ppa_dep = dep.dependency
+            ppa_addresses.append(ppa_dep.reference)
+        return ppa_addresses
+
+    def set_dependencies(self, ppa_addresses: list[str]):
+        """Configures the additional PPAs used to build packages in this PPA.
+
+        This removes any existing PPA dependencies and adds the ones
+        in the corresponding list.  If any of these new PPAs cannot be
+        found, this routine bails out without changing the current set.
+
+        :param list[str] ppa_addresses: Additional PPAs to add
+        """
+        base = self._service.API_ROOT_URL.rstrip('/')
+        new_ppa_deps = []
+        for ppa_address in ppa_addresses:
+            team_name, ppa_name = ppa_address_split(ppa_address)
+            new_ppa_dep = f'{base}/~{team_name}/+archive/ubuntu/{ppa_name}'
+            new_ppa_deps.append(new_ppa_dep)
+
+        # TODO: Remove all existing dependencies
+#        for ppa_dep in self.archive.dependencies:
+#            the_ppa.removeArchiveDependency(ppa_dep)
+
+        # TODO: Not sure what to pass here, maybe a string ala 'main'?
+        component = None
+
+        # TODO: Allow setting alternate pockets
+        # TODO: Maybe for convenience it should be same as what's set for main archive?
+        pocket = 'Release'
+
+        for ppa_dep in new_ppa_deps:
+            self.archive.addArchiveDependency(
+                component=component,
+                dependency=ppa_dep,
+                pocket=pocket)
+        # TODO: Error checking
+        #       This can throw ArchiveDependencyError if the ppa_address does not fit the_ppa
+
     def get_binaries(self, distro=None, series=None, arch=None):
         """Retrieves the binary packages available in the PPA.
 
@@ -450,3 +501,70 @@ def get_ppa(lp, config):
         ppa_name=config.get('ppa_name', None),
         team_name=config.get('team_name', None),
         service=lp)
+
+
+if __name__ == "__main__":
+    import pprint
+    import random
+    import string
+    from .lp import Lp
+    from .ppa_group import PpaGroup
+
+    pp = pprint.PrettyPrinter(indent=4)
+
+    print('##########################')
+    print('## Ppa class smoke test ##')
+    print('##########################')
+    print()
+
+    rndstr = str(''.join(random.choices(string.ascii_lowercase, k=6)))
+    dep_name = f'dependency-ppa-{rndstr}'
+    smoketest_ppa_name = f'test-ppa-{rndstr}'
+
+    lp = Lp('smoketest', staging=True)
+    ppa_group = PpaGroup(service=lp, name=lp.me.name)
+
+    dep_ppa = ppa_group.create(dep_name, ppa_description=dep_name)
+    the_ppa = ppa_group.create(smoketest_ppa_name, ppa_description=smoketest_ppa_name)
+    ppa_dependencies = [f'ppa:{lp.me.name}/{dep_name}']
+
+    try:
+        the_ppa.set_publish(True)
+
+        if not the_ppa.exists():
+            print("Error: PPA does not exist")
+            sys.exit(1)
+        the_ppa.set_description("This is a testing PPA and can be deleted")
+        the_ppa.set_publish(False)
+        the_ppa.set_architectures(["amd64", "arm64"])
+        the_ppa.set_dependencies(ppa_dependencies)
+
+        print()
+        print(f"name:          {the_ppa.name}")
+        print(f"address:       {the_ppa.address}")
+        print(f"str(ppa):      {the_ppa}")
+        print(f"reference:     {the_ppa.archive.reference}")
+        print(f"self_link:     {the_ppa.archive.self_link}")
+        print(f"web_link:      {the_ppa.archive.web_link}")
+        print(f"description:   {the_ppa.description}")
+        print(f"has_packages:  {the_ppa.has_packages()}")
+        print(f"architectures: {'/'.join(the_ppa.architectures)}")
+        print(f"dependencies:  {','.join(the_ppa.dependencies)}")
+        print(f"url:           {the_ppa.url}")
+        print()
+
+    except BadRequest as e:
+        print(f"Error: (BadRequest) {str(e.content.decode('utf-8'))}")
+    except Unauthorized as e:
+        print(f"Error: (Unauthorized) {e}")
+
+    answer = 'x'
+    while answer not in ['y', 'n']:
+        answer = input('Ready to cleanup (i.e. delete) temporary test PPAs? (y/n) ')
+        answer = answer[0].lower()
+
+    if answer == 'y':
+        print("  Cleaning up temporary test PPAs...")
+        the_ppa.destroy()
+        dep_ppa.destroy()
+        print("  ...Done")
diff --git a/scripts/ppa b/scripts/ppa
index 9097004..29413ea 100755
--- a/scripts/ppa
+++ b/scripts/ppa
@@ -206,6 +206,13 @@ def add_basic_config_options(parser: argparse.ArgumentParser) -> None:
         help="Do not accept or build packages uploaded to the PPA."
     )
 
+    # Dependencies
+    parser.add_argument(
+        '--ppa-dependencies', '--ppa-depends',
+        dest="ppa_dependencies", action='store',
+        help="The set of other PPAs this PPA should use for satisfying build dependencies."
+    )
+
     parser.add_argument(
         '--publish',
         dest="publish", action='store_true',
@@ -443,11 +450,11 @@ def create_config(lp: Lp, args: argparse.Namespace) -> dict[str, Any]:
 ### Commands ###
 ################
 
-def command_create(lp, config):
+def command_create(lp: Lp, config: dict[str, str]) -> int:
     """Creates a new PPA in Launchpad.
 
     :param Lp lp: The Launchpad wrapper object.
-    :param dict config: Configuration param:value map.
+    :param dict[str, str] config: Configuration param:value map.
     :rtype: int
     :returns: Status code OK (0) on success, non-zero on error.
     """
@@ -480,6 +487,12 @@ def command_create(lp, config):
             if architectures:
                 the_ppa.set_architectures(architectures)
             arch_str = ', '.join(the_ppa.architectures)
+
+            if 'ppa_dependencies' in config:
+                # Split value on comma
+                ppa_addresses = unpack_to_dict(config.get('ppa_dependencies'))
+                the_ppa.set_dependencies(ppa_addresses)
+
         else:
             the_ppa = Ppa(ppa_name, team_name, description)
             arch_str = ', '.join(architectures)
@@ -505,7 +518,7 @@ def command_create(lp, config):
     return 1
 
 
-def command_desc(lp, config):
+def command_desc(lp: Lp, config: dict[str, str]) -> int:
     """Sets the description for a PPA.
 
     :param dict config: Configuration param:value map.
@@ -534,11 +547,11 @@ def command_desc(lp, config):
     return 1
 
 
-def command_destroy(lp, config):
+def command_destroy(lp: Lp, config: dict[str, str]) -> int:
     """Destroys the PPA.
 
     :param Lp lp: The Launchpad wrapper object.
-    :param dict config: Configuration param:value map.
+    :param dict[str, str] config: Configuration param:value map.
     :rtype: int
     :returns: Status code OK (0) on success, non-zero on error.
     """
@@ -554,11 +567,11 @@ def command_destroy(lp, config):
     return 1
 
 
-def command_list(lp, config, filter_func=None):
+def command_list(lp: Lp, config: dict[str, str], filter_func=None) -> int:
     """Lists the PPAs for the user or team.
 
     :param Lp lp: The Launchpad wrapper object.
-    :param dict config: Configuration param:value map.
+    :param dict[str, str] config: Configuration param:value map.
     :rtype: int
     :returns: Status code OK (0) on success, non-zero on error.
     """
@@ -592,11 +605,11 @@ def command_list(lp, config, filter_func=None):
     return 1
 
 
-def command_exists(lp, config):
+def command_exists(lp: Lp, config: dict[str, str]) -> int:
     """Checks if the named PPA exists in Launchpad.
 
     :param Lp lp: The Launchpad wrapper object.
-    :param dict config: Configuration param:value map.
+    :param dict[str, str] config: Configuration param:value map.
     :rtype: int
     :returns: Status code OK (0) on success, non-zero on error.
     """
@@ -609,11 +622,11 @@ def command_exists(lp, config):
     return 1
 
 
-def command_set(lp, config):
+def command_set(lp: Lp, config: dict[str, str]) -> int:
     """Sets one or more properties of PPA in Launchpad.
 
     :param Lp lp: The Launchpad wrapper object.
-    :param dict config: Configuration param:value map.
+    :param dict[str, str] config: Configuration param:value map.
     :rtype: int
     :returns: Status code OK (0) on success, non-zero on error.
     """
@@ -632,6 +645,11 @@ def command_set(lp, config):
         if 'displayname' in config:
             the_ppa.archive.displayname = config['displayname']
 
+        if 'ppa_dependencies' in config:
+            # Split value on comma
+            ppa_addresses = unpack_to_dict(config.get('ppa_dependencies'))
+            the_ppa.set_dependencies(ppa_addresses)
+
         if 'publish' in config:
             the_ppa.archive.publish = config.get('publish')
 
@@ -647,7 +665,7 @@ def command_set(lp, config):
     return 1
 
 
-def command_show(lp, config):
+def command_show(lp: Lp, config: dict[str, str]) -> int:
     """Displays details about the given PPA.
 
     :param Lp lp: The Launchpad wrapper object.
@@ -705,11 +723,11 @@ def command_show(lp, config):
     return 1
 
 
-def command_status(lp, config):
+def command_status(lp: Lp, config: dict[str, str]) -> int:
     """Displays current status of the given ppa.
 
     :param Lp lp: The Launchpad wrapper object.
-    :param dict config: Configuration param:value map.
+    :param dict[str, str] config: Configuration param:value map.
     :rtype: int
     :returns: Status code OK (0) on success, non-zero on error.
     """
@@ -729,11 +747,11 @@ def command_status(lp, config):
     return 1
 
 
-def command_wait(lp, config):
+def command_wait(lp: Lp, config: dict[str, str]) -> int:
     """Polls the PPA build status and block until all builds are finished and published.
 
     :param Lp lp: The Launchpad wrapper object.
-    :param dict config: Configuration param:value map.
+    :param dict[str, str] config: Configuration param:value map.
     :rtype: int
     :returns: Status code OK (0) on success, non-zero on error.
     """
@@ -761,11 +779,11 @@ def command_wait(lp, config):
     return 1
 
 
-def command_tests(lp, config):
+def command_tests(lp: Lp, config: dict[str, str]) -> int:
     """Displays testing status for the PPA.
 
     :param Lp lp: The Launchpad wrapper object.
-    :param dict config: Configuration param:value map.
+    :param dict[str, str] config: Configuration param:value map.
     :rtype: int
     :returns: Status code OK (0) on success, non-zero on error.
     """
diff --git a/tests/helpers.py b/tests/helpers.py
index 2329125..29fa151 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -85,6 +85,11 @@ class LaunchpadMock:
 
 class LpServiceMock:
     """A stand-in for the Lp service object."""
+    ROOT_URL = 'https://mocklaunchpad.net/'
+    API_ROOT_URL = 'https://api.mocklaunchpad.net/devel/'
+    BUGS_ROOT_URL = 'https://bugs.mocklaunchpad.net/'
+    CODE_ROOT_URL = 'https://code.mocklaunchpad.net/'
+
     def __init__(self):
         self.launchpad = LaunchpadMock()
 
diff --git a/tests/test_scripts_ppa.py b/tests/test_scripts_ppa.py
index a107f92..78aaeb3 100644
--- a/tests/test_scripts_ppa.py
+++ b/tests/test_scripts_ppa.py
@@ -235,6 +235,14 @@ def test_create_arg_parser_basic_config(command):
     assert args.set_disabled is True
     args.set_disabled = False
 
+    # Check --ppa-dependencies <PPA[,...]>
+    args = parser.parse_args([command, 'test-ppa', '--ppa-dependencies', 'a,b,c'])
+    assert args.ppa_dependencies == "a,b,c"
+    args.ppa_dependencies = None
+    args = parser.parse_args([command, 'test-ppa', '--ppa-depends', 'a,b,c'])
+    assert args.ppa_dependencies == "a,b,c"
+    args.ppa_dependencies = None
+
     # Check --publish
     args = parser.parse_args([command, 'test-ppa', '--publish'])
     assert args.publish is True
@@ -477,7 +485,8 @@ def test_command_create_with_architectures(monkeypatch, fake_config, architectur
 
 @pytest.mark.xfail(reason="Unimplemented")
 def test_command_desc(fake_config):
-    assert script.command_desc(fake_config) == 0
+    lp = LpServiceMock()
+    assert script.command_desc(lp, fake_config) == 0
     # TODO: Assert that if --dry-run specified, there are no actual
     #   changes requested of launchpad
     # TODO: Verify the description gets set as expected
@@ -485,29 +494,33 @@ def test_command_desc(fake_config):
 
 @pytest.mark.xfail(reason="Unimplemented")
 def test_command_destroy(fake_config):
+    lp = LpServiceMock()
     # TODO: Create a fake ppa to be destroyed
-    assert script.command_destroy(fake_config) == 0
+    assert script.command_destroy(lp, fake_config) == 0
     # TODO: Verify the ppa is requested to be deleted
 
 
 @pytest.mark.xfail(reason="Unimplemented")
 def test_command_list(fake_config):
+    lp = LpServiceMock()
     # TODO: Create a fake ppa with contents to be listed
-    assert script.command_list(fake_config) == 0
+    assert script.command_list(lp, fake_config) == 0
     # TODO: Verify the ppa addresses get listed
 
 
 @pytest.mark.xfail(reason="Unimplemented")
 def test_command_exists(fake_config):
+    lp = LpServiceMock()
     # TODO: Create fake ppa that exists
-    assert script.command_exists(fake_config) == 0
+    assert script.command_exists(lp, fake_config) == 0
     # TODO: Verify this returns true when the ppa does exist
 
 
 @pytest.mark.xfail(reason="Unimplemented")
 def test_command_not_exists(fake_config):
+    lp = LpServiceMock()
     # TODO: Verify this returns true when the ppa does not exist
-    assert script.command_exists(fake_config) == 1
+    assert script.command_exists(lp, fake_config) == 1
 
 
 @pytest.mark.parametrize('params, expected_ppa_config', [
@@ -569,16 +582,19 @@ def test_command_set_architectures(fake_config, architectures, expected_processo
 
 @pytest.mark.xfail(reason="Unimplemented")
 def test_command_show(fake_config):
-    assert script.command_show(fake_config) == 0
+    lp = LpServiceMock()
+    assert script.command_show(lp, fake_config) == 0
 
 
 @pytest.mark.xfail(reason="Unimplemented")
 def test_command_status(fake_config):
-    assert script.command_status(fake_config) == 0
+    lp = LpServiceMock()
+    assert script.command_status(lp, fake_config) == 0
     # TODO: Capture stdout and compare with expected
 
 
 @pytest.mark.xfail(reason="Unimplemented")
 def test_command_wait(fake_config):
+    lp = LpServiceMock()
     # TODO: Set wait period to 1 sec
-    assert script.command_wait(fake_config) == 0
+    assert script.command_wait(lp, fake_config) == 0