launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29513
[Merge] ~lgp171188/lpcraft:set-sensible-defaults-some-package-repository-fields into lpcraft:main
Guruprasad has proposed merging ~lgp171188/lpcraft:set-sensible-defaults-some-package-repository-fields into lpcraft:main.
Commit message:
Set sensible default values for some package repository fields
Also, simplify the package repository cross-field validation.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lgp171188/lpcraft/+git/lpcraft/+merge/435356
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/lpcraft:set-sensible-defaults-some-package-repository-fields into lpcraft:main.
diff --git a/NEWS.rst b/NEWS.rst
index 4defd5a..f351e3f 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -5,8 +5,9 @@ Version history
0.0.40 (unreleased)
===================
-- Fix the leakage of package repositories from a job to the next
+- Fix the leakage of package repositories from a job to the next.
- Add support for Python 3.11.
+- Set sensible default values for some package repository fields.
0.0.39 (2023-01-06)
===================
diff --git a/docs/configuration.rst b/docs/configuration.rst
index 180734c..d25c96d 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -190,13 +190,15 @@ Adding a PPA
Specifies the type of package-repository.
Currently only ``apt`` is supported.
-``formats`` (required)
+``formats`` (optional)
Specifies the format of the package-repository.
- Supported values: ``deb`` and ``deb-src``.
+ Supported values: ``deb`` and ``deb-src``. If unspecified,
+ the format is assumed to be ``deb`` , i.e. ``[deb]``
-``suites`` (required)
+``suites`` (optional)
Specifies the suite of the package-repository.
- One or several of ``bionic``, ``focal``, ``jammy``.
+ One or several of ``bionic``, ``focal``, ``jammy``. If unspecified,
+ the suite is assumed to be the corresponding job's ``series`` value.
``ppa`` (required)
Specifies the PPA to be used as the package repository in the short form,
@@ -228,11 +230,13 @@ Adding a deb repository
``formats`` (required)
Specifies the format of the package-repository.
- Supported values: ``deb`` and ``deb-src``.
+ Supported values: ``deb`` and ``deb-src``. If unspecified,
+ the format is assumed to be ``deb``, i.e. ``[deb]``.
``suites`` (required)
Specifies the suite of the package-repository.
- One or several of ``bionic``, ``focal``, ``jammy``.
+ One or several of ``bionic``, ``focal``, ``jammy``. If unspecified,
+ the suite is assumed to be the corresponding job's ``series`` value.
``components`` (required)
Specifies the component of the package-repository,
diff --git a/lpcraft/config.py b/lpcraft/config.py
index 7c36713..d228270 100644
--- a/lpcraft/config.py
+++ b/lpcraft/config.py
@@ -152,9 +152,9 @@ class PackageRepository(ModelConfigDefaults):
type: PackageType # e.g. `apt``
ppa: Optional[PPAShortFormURL] # e.g. `launchpad/ubuntu/ppa`
- formats: List[PackageFormat] # e.g. `[deb, deb-src]`
+ formats: Optional[List[PackageFormat]] # e.g. `[deb, deb-src]`
components: Optional[List[PackageComponent]] # e.g. `[main, universe]`
- suites: List[PackageSuite] # e.g. `[bionic, focal]`
+ suites: Optional[List[PackageSuite]] # e.g. `[bionic, focal]`
url: Optional[AnyHttpUrl]
trusted: Optional[bool]
@@ -162,26 +162,29 @@ class PackageRepository(ModelConfigDefaults):
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."
- )
+ if "url" in values:
+ if "ppa" in values:
+ raise ValueError(
+ "Only one of the following keys can be specified:"
+ " 'url', 'ppa'."
+ )
+ if "components" not in values:
+ raise ValueError(
+ "The 'components' key is required when the 'url' key"
+ " is specified."
+ )
+ else:
+ if "ppa" not in values:
+ raise ValueError(
+ "One of the following keys is required with an appropriate"
+ " value: 'url', 'ppa'."
+ )
+ if "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)
@@ -206,6 +209,14 @@ class PackageRepository(ModelConfigDefaults):
)
return v
+ @validator("formats", pre=True, always=True)
+ def set_formats_default_value(
+ cls, v: List[PackageFormat]
+ ) -> List[PackageFormat]:
+ if not v:
+ v = [PackageFormat.deb]
+ return v
+
@validator("trusted")
def convert_trusted(cls, v: bool) -> str:
# trusted is True or False, but we need `yes` or `no`
@@ -216,6 +227,8 @@ class PackageRepository(ModelConfigDefaults):
e.g. 'deb https://canonical.example.org/artifactory/jammy-golang-backport focal main'
""" # noqa: E501
+ assert self.formats is not None
+ assert self.suites is not None
for format in self.formats:
for suite in self.suites:
assert self.components is not None
@@ -254,6 +267,18 @@ class Job(ModelConfigDefaults):
v = [v]
return v
+ @pydantic.validator("package_repositories")
+ def validate_package_repositories(
+ cls, v: List[PackageRepository], values: Dict[StrictStr, Any]
+ ) -> List[PackageRepository]:
+ package_repositories = None
+ for index, package_repository in enumerate(v):
+ if not package_repository.suites:
+ if not package_repositories:
+ package_repositories = v.copy()
+ package_repositories[index].suites = [values["series"]]
+ return package_repositories or v
+
@pydantic.root_validator(pre=True)
def move_plugin_config_settings(
cls, values: Dict[StrictStr, Any]
diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py
index b68e20b..6c69eab 100644
--- a/lpcraft/tests/test_config.py
+++ b/lpcraft/tests/test_config.py
@@ -579,6 +579,32 @@ class TestConfig(TestCase):
get_ppa_url_parts(PPAShortFormURL("example/debian/bar")),
)
+ def test_default_values_for_package_repository_suites_and_formats(self):
+ path = self.create_config(
+ dedent(
+ """
+ pipeline:
+ - test
+ jobs:
+ test:
+ series: focal
+ architectures: amd64
+ packages: [foo]
+ package-repositories:
+ - type: apt
+ ppa: launchpad/ubuntu/ppa
+ - type: apt
+ url: https://canonical.example.org/ubuntu
+ components: [main]
+ """
+ )
+ )
+ config = Config.load(path)
+
+ for package_repository in config.jobs["test"][0].package_repositories:
+ self.assertEqual(["deb"], package_repository.formats)
+ self.assertEqual(["focal"], package_repository.suites)
+
def test_missing_ppa_and_url(self):
path = self.create_config(
dedent(
@@ -634,7 +660,7 @@ class TestConfig(TestCase):
path,
)
- def test_missing_ppa_and_components(self):
+ def test_missing_components_when_url_is_specified(self):
path = self.create_config(
dedent(
"""
@@ -655,8 +681,8 @@ class TestConfig(TestCase):
)
self.assertRaisesRegex(
ValidationError,
- r"One of the following keys is required with an appropriate"
- r" value: 'components', 'ppa'.",
+ r"The 'components' key is required when the 'url' key"
+ r" is specified.",
Config.load,
path,
)
