← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~lgp171188/lpcraft:easier-way-to-add-a-ppa into lpcraft:main

 

Guruprasad has proposed merging ~lgp171188/lpcraft:easier-way-to-add-a-ppa into lpcraft:main.

Commit message:
Allow specifying PPAs using the shortform notation

LP: #1982950


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1982950 in lpcraft: "Adding PPAs could be easier"
  https://bugs.launchpad.net/lpcraft/+bug/1982950

For more details, see:
https://code.launchpad.net/~lgp171188/lpcraft/+git/lpcraft/+merge/433493
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/lpcraft:easier-way-to-add-a-ppa into lpcraft:main.
diff --git a/NEWS.rst b/NEWS.rst
index 8403acb..a63549c 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -7,6 +7,8 @@ Version history
 
 - Sanitize the project name before cleaning.
 
+- Allow specifying PPAs using the shortform notation.
+
 0.0.35 (2022-10-27)
 ===================
 
diff --git a/docs/configuration.rst b/docs/configuration.rst
index 258d869..d80888a 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -184,26 +184,34 @@ More properties can be implemented on demand.
 
 ``type`` (required)
     Specifies the type of package-repository.
-    Currently only `apt` is supported.
+    Currently only ``apt`` is supported.
 
 ``formats`` (required)
     Specifies the format of the package-repository.
     Supported values: ``deb`` and ``deb-src``.
 
-``components`` (required)
-    Specifies the component of the package-repository,
+``components`` (optional)
+    Specifies the component of the package-repository.
     One or several of ``main``, ``restricted``, ``universe``, ``multiverse``.
+    This property is not allowed when the ``ppa`` key is specified. Otherwise,
+    it is required.
 
 ``suites`` (required)
     Specifies the suite of the package-repository.
     One or several of ``bionic``, ``focal``, ``jammy``.
 
-``url`` (required)
+``url`` (optional)
     Specifies the URL of the package-repository,
     e.g. ``http://ppa.launchpad.net/snappy-dev/snapcraft-daily/ubuntu``.
     The URL is rendered using `Jinja2 <https://pypi.org/project/Jinja2/>`_.
     This can be used to supply authentication details via the *secrets*
-    command line option.
+    command line option. This property is not allowed when the ``ppa`` key
+    is specified. Otherwise, it is required.
+
+``ppa`` (optional)
+    Specifies the PPA to be used as the package repository in the short form,
+    e.g. ``launchpad/ppa``. This is automatically expanded to an appropriate
+    ``url`` value.
 
 ``trusted`` (optional)
     Set this to ``true`` to override APT's security checks, ie accept sources
diff --git a/lpcraft/config.py b/lpcraft/config.py
index d8d2045..f60cbc2 100644
--- a/lpcraft/config.py
+++ b/lpcraft/config.py
@@ -8,13 +8,15 @@ from pathlib import Path
 from typing import Any, Dict, Iterator, List, Optional, Type, Union
 
 import pydantic
-from pydantic import AnyHttpUrl, StrictStr, validator
+from pydantic import AnyHttpUrl, StrictStr, root_validator, validator
 
 from lpcraft.errors import ConfigurationError
 from lpcraft.plugins import PLUGINS
 from lpcraft.plugins.plugins import BaseConfig, BasePlugin
 from lpcraft.utils import load_yaml
 
+LAUNCHPAD_PPA_BASE_URL = "https://ppa.launchpadcontent.net";
+
 
 class _Identifier(pydantic.ConstrainedStr):
     """A string with constrained syntax used as a short identifier.
@@ -119,6 +121,13 @@ class PackageSuite(str, Enum):
     jammy = "jammy"  # 22.04
 
 
+class PPAShortFormURL(pydantic.ConstrainedStr):
+    """A string with a constrained syntax to match a PPA short form URL."""
+
+    strict = True
+    regex = re.compile(r"^[a-z0-9][a-z0-9\+\._\-]+/[a-z0-9][a-z0-9\+\._\-]+$")
+
+
 class PackageRepository(ModelConfigDefaults):
     """A representation of a package repository.
 
@@ -126,12 +135,57 @@ class PackageRepository(ModelConfigDefaults):
     """
 
     type: PackageType  # e.g. `apt``
+    ppa: Optional[PPAShortFormURL]  # e.g. `ppa:launchpad/ppa`
     formats: List[PackageFormat]  # e.g. `[deb, deb-src]`
-    components: List[PackageComponent]  # e.g. `[main, universe]`
+    components: Optional[List[PackageComponent]]  # e.g. `[main, universe]`
     suites: List[PackageSuite]  # e.g. `[bionic, focal]`
-    url: AnyHttpUrl
+    url: Optional[AnyHttpUrl]
     trusted: Optional[bool]
 
+    @root_validator(pre=True)
+    def validate_multiple_fields(
+        cls, values: Dict[str, Any]
+    ) -> Dict[str, Any]:
+        if "url" not in values and "ppa" not in values:
+            raise ValueError(
+                "One of the following keys is required with an appropriate"
+                " value: 'url', 'ppa'."
+            )
+        elif "url" in values and "ppa" in values:
+            raise ValueError(
+                "Only one of the following keys can be specified:"
+                " 'url', 'ppa'."
+            )
+        elif "ppa" not in values and "components" not in values:
+            raise ValueError(
+                "One of the following keys is required with an appropriate"
+                " value: 'components', 'ppa'."
+            )
+        elif "ppa" in values and "components" in values:
+            raise ValueError(
+                "The 'components' key is not allowed when the 'ppa' key is"
+                " specified. PPAs only support the 'main' component."
+            )
+        return values
+
+    @validator("components", pre=True, always=True)
+    def infer_components_if_ppa_is_set(
+        cls, v: List[PackageComponent], values: Dict[str, Any]
+    ) -> List[PackageComponent]:
+        if v is None and values["ppa"]:
+            return ["main"]
+        return v
+
+    @validator("url", pre=True, always=True)
+    def infer_url_if_ppa_is_set(
+        cls, v: AnyHttpUrl, values: Dict[str, Any]
+    ) -> AnyHttpUrl:
+        if v is None and values["ppa"]:
+            return "{}/{}/ubuntu".format(
+                LAUNCHPAD_PPA_BASE_URL, values["ppa"].replace("ppa:", "")
+            )
+        return v
+
     @validator("trusted")
     def convert_trusted(cls, v: bool) -> str:
         # trusted is True or False, but we need `yes` or `no`
@@ -144,6 +198,7 @@ class PackageRepository(ModelConfigDefaults):
         """  # noqa: E501
         for format in self.formats:
             for suite in self.suites:
+                assert self.components is not None
                 if self.trusted:
                     yield f"{format} [trusted={self.trusted}] {self.url!s} {suite} {' '.join(self.components)}"  # noqa: E501
                 else:
diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py
index ea57875..9387f98 100644
--- a/lpcraft/tests/test_config.py
+++ b/lpcraft/tests/test_config.py
@@ -16,7 +16,12 @@ from testtools.matchers import (
     MatchesStructure,
 )
 
-from lpcraft.config import Config, OutputDistributeEnum, PackageRepository
+from lpcraft.config import (
+    LAUNCHPAD_PPA_BASE_URL,
+    Config,
+    OutputDistributeEnum,
+    PackageRepository,
+)
 from lpcraft.errors import CommandError
 
 
@@ -562,6 +567,146 @@ class TestConfig(TestCase):
             list(repositories[2].sources_list_lines()),
         )
 
+    def test_missing_ppa_and_url(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                        packages: [foo]
+                        package-repositories:
+                            - type: apt
+                              formats: [deb, deb-src]
+                              components: [main]
+                              suites: [focal]
+                """
+            )
+        )
+        self.assertRaisesRegex(
+            ValidationError,
+            r"One of the following keys is required with an appropriate"
+            r" value: 'url', 'ppa'",
+            Config.load,
+            path,
+        )
+
+    def test_both_ppa_and_url_provided(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                        packages: [foo]
+                        package-repositories:
+                            - type: apt
+                              formats: [deb, deb-src]
+                              suites: [focal]
+                              ppa: launchpad/ppa
+                              url: https://canonical.example.com
+                """
+            )
+        )
+        self.assertRaisesRegex(
+            ValidationError,
+            r"Only one of the following keys can be specified:"
+            r" 'url', 'ppa'",
+            Config.load,
+            path,
+        )
+
+    def test_missing_ppa_and_components(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                        packages: [foo]
+                        package-repositories:
+                            - type: apt
+                              formats: [deb, deb-src]
+                              suites: [focal]
+                              url: https://canonical.example.com
+                """
+            )
+        )
+        self.assertRaisesRegex(
+            ValidationError,
+            r"One of the following keys is required with an appropriate"
+            r" value: 'components', 'ppa'.",
+            Config.load,
+            path,
+        )
+
+    def test_both_ppa_and_components_specified(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                        packages: [foo]
+                        package-repositories:
+                            - type: apt
+                              formats: [deb, deb-src]
+                              components: [main]
+                              ppa: launchpad/ppa
+                """
+            )
+        )
+        self.assertRaisesRegex(
+            ValidationError,
+            r"The 'components' key is not allowed when the 'ppa' key is"
+            r" specified. PPAs only support the 'main' component.",
+            Config.load,
+            path,
+        )
+
+    def test_ppa_shortform_url_and_components_automatically_inferred(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                        packages: [foo]
+                        package-repositories:
+                            - type: apt
+                              formats: [deb, deb-src]
+                              suites: [focal]
+                              ppa: launchpad/ppa
+                """
+            )
+        )
+        config = Config.load(path)
+        package_repository = config.jobs["test"][0].package_repositories[0]
+
+        self.assertEqual("launchpad/ppa", package_repository.ppa)
+        self.assertEqual(
+            "{}/launchpad/ppa/ubuntu".format(
+                LAUNCHPAD_PPA_BASE_URL,
+            ),
+            str(package_repository.url),
+        )
+        self.assertEqual(["main"], package_repository.components)
+
     def test_specify_license_via_spdx(self):
         path = self.create_config(
             dedent(

Follow ups