From 8b2a01f169dc798957bceee752772068aeda3257 Mon Sep 17 00:00:00 2001 From: Dmitry Malykhanov Date: Thu, 23 Jul 2026 12:04:15 +0100 Subject: [PATCH] Proposed extension-generated part names handling --- rockcraft/extensions/_utils.py | 6 ++- rockcraft/extensions/expressjs.py | 12 +++--- rockcraft/extensions/extension.py | 54 +++++++++++++++++++----- tests/unit/extensions/test_expressjs.py | 20 ++++++--- tests/unit/extensions/test_fastapi.py | 8 +++- tests/unit/extensions/test_go.py | 12 +++++- tests/unit/extensions/test_gunicorn.py | 10 ++++- tests/unit/extensions/test_springboot.py | 2 + 8 files changed, 94 insertions(+), 30 deletions(-) diff --git a/rockcraft/extensions/_utils.py b/rockcraft/extensions/_utils.py index c6ddb8943..0e7eed0fb 100644 --- a/rockcraft/extensions/_utils.py +++ b/rockcraft/extensions/_utils.py @@ -41,9 +41,11 @@ def apply_extensions(project_root: Path, yaml_data: dict[str, Any]) -> dict[str, for extension_name in sorted(declared_extensions): extension_class = get_extension_class(extension_name) extension = extension_class( - project_root=project_root, yaml_data=copy.deepcopy(yaml_data) + project_root=project_root, + yaml_data=copy.deepcopy(yaml_data), + extension_name=extension_name, ) - extension.validate(extension_name=extension_name) + extension.validate() _apply_extension(yaml_data, extension) return yaml_data diff --git a/rockcraft/extensions/expressjs.py b/rockcraft/extensions/expressjs.py index c8da65614..5d50cd729 100644 --- a/rockcraft/extensions/expressjs.py +++ b/rockcraft/extensions/expressjs.py @@ -25,7 +25,7 @@ from rockcraft.usernames import SUPPORTED_GLOBAL_USERNAMES from .app_parts import gen_logging_part -from .extension import Extension, _FrameworkFactory +from .extension import Extension, ExtensionPart, _FrameworkFactory USER_UID: int = SUPPORTED_GLOBAL_USERNAMES["_daemon_"]["uid"] @@ -78,20 +78,20 @@ def get_root_snippet(self) -> dict[str, Any]: snippet["services"]["expressjs"]["command"] = "npm start" snippet["parts"] = { - "expressjs-framework/install-app": self._gen_install_app_part(), + self.get_part_name(ExtensionPart.INSTALL_APP): self._gen_install_app_part(), } runtime_part = self._gen_runtime_part() if runtime_part: - snippet["parts"]["expressjs-framework/runtime"] = runtime_part + snippet["parts"][self.get_part_name(ExtensionPart.RUNTIME)] = runtime_part # There is a bug where ca-certificates_data and # expressjs-framework/runtime stage-packages with a transitive # dependency on ca-certificates will both contain # etc/ssl/certs/ca-certificates.crt with different content. - snippet["parts"]["expressjs-framework/runtime"]["stage"] = [ + snippet["parts"][self.get_part_name(ExtensionPart.RUNTIME)]["stage"] = [ "-etc/ssl/certs/ca-certificates.crt" ] - snippet["parts"]["expressjs-framework/logging"] = gen_logging_part() + snippet["parts"][self.get_part_name(ExtensionPart.LOGGING)] = gen_logging_part() return snippet @override @@ -213,7 +213,7 @@ def _gen_runtime_part(self) -> dict[str, Any] | None: def _user_install_app_part(self) -> dict[str, Any]: """Return the user defined install app part.""" return self.yaml_data.get("parts", {}).get( - "expressjs-framework/install-app", {} + self.get_part_name(ExtensionPart.INSTALL_APP), {} ) @property diff --git a/rockcraft/extensions/extension.py b/rockcraft/extensions/extension.py index 707e453fc..8f0604886 100644 --- a/rockcraft/extensions/extension.py +++ b/rockcraft/extensions/extension.py @@ -17,12 +17,14 @@ """Extension base class definition.""" import abc +import enum import os import sys from collections.abc import Sequence from pathlib import Path from typing import Any, cast, final +from craft_application._const import BASES_ALLOW_SLASH_IN_PART_NAME from craft_cli import emit from rockcraft import errors @@ -53,6 +55,14 @@ def get_project_base(yaml_data: dict[str, Any]) -> str | None: return base_str +class ExtensionPart(enum.Enum): + """Common part names used by extensions.""" + + INSTALL_APP = "install-app" + RUNTIME = "runtime" + LOGGING = "logging" + + class Extension(abc.ABC): """Extension is the class from which all extensions inherit. @@ -68,10 +78,12 @@ def __init__( *, project_root: Path, yaml_data: dict[str, Any], + extension_name: str, ) -> None: """Create a new Extension.""" self.project_root = project_root self.yaml_data = yaml_data + self.extension_name = extension_name @staticmethod @abc.abstractmethod @@ -95,11 +107,21 @@ def get_part_snippet(self) -> dict[str, Any]: def get_parts_snippet(self) -> dict[str, Any]: """Return the parts to add to parts.""" + @property + def _extension_name_sep(self) -> str: + """Return the string separating extension part name fragments.""" + base = get_project_base(self.yaml_data) + # Q: is it a good idea to bring in BASES_... here? + return "/" if base in BASES_ALLOW_SLASH_IN_PART_NAME else "." + + def get_part_name(self, part: ExtensionPart) -> str: # Q: type annotation + """Return formatted internal part name.""" + return f"{self.extension_name}{self._extension_name_sep}{part.value}" + @final - def validate(self, extension_name: str) -> None: + def validate(self) -> None: """Validate that the extension can be used with the current project. - :param extension_name: the name of the extension being parsed. :raises errors.ExtensionError: if the extension is incompatible with the project. """ if "base" not in self.yaml_data: @@ -112,7 +134,7 @@ def validate(self, extension_name: str) -> None: "ROCKCRAFT_ENABLE_EXPERIMENTAL_EXTENSIONS" ): raise errors.ExtensionError( - f"Extension is experimental: {extension_name!r}", + f"Extension is experimental: {self.extension_name!r}", doc_slug="/reference/extensions/", resolution="Run with ROCKCRAFT_ENABLE_EXPERIMENTAL_EXTENSIONS=True to enable " "experimental extensions.", @@ -120,24 +142,26 @@ def validate(self, extension_name: str) -> None: if self.is_experimental(base): emit.progress( - f"*EXPERIMENTAL* extension {extension_name!r} enabled", + f"*EXPERIMENTAL* extension {self.extension_name!r} enabled", permanent=True, ) if base not in self.get_supported_bases(): raise errors.ExtensionError( - f"Extension {extension_name!r} does not support base: {base!r}" + f"Extension {self.extension_name!r} does not support base: {base!r}" ) invalid_parts = [ p for p in self.get_parts_snippet() - if not p.startswith(f"{extension_name}/") + if not p.startswith( + f"{self.extension_name}{self._extension_name_sep}" + ) # Q: no test for this path? ] if invalid_parts: raise ValueError( f"Extension has invalid part names: {invalid_parts!r}. " - "Format is /" + f"Format is {self._extension_name_sep}" ) @@ -148,11 +172,21 @@ def __init__(self, v1_cls: type[Extension], v2_cls: type[Extension]) -> None: self._v1_cls = v1_cls self._v2_cls = v2_cls - def __call__(self, *, project_root: Path, yaml_data: dict[str, Any]) -> Extension: + def __call__( + self, *, project_root: Path, yaml_data: dict[str, Any], extension_name: str + ) -> Extension: base = get_project_base(yaml_data) if base in self._v1_cls.get_supported_bases(): - return self._v1_cls(project_root=project_root, yaml_data=yaml_data) - return self._v2_cls(project_root=project_root, yaml_data=yaml_data) + return self._v1_cls( + project_root=project_root, + yaml_data=yaml_data, + extension_name=extension_name, + ) + return self._v2_cls( + project_root=project_root, + yaml_data=yaml_data, + extension_name=extension_name, + ) def get_supported_bases(self) -> tuple[str, ...]: return tuple( diff --git a/tests/unit/extensions/test_expressjs.py b/tests/unit/extensions/test_expressjs.py index fd8e747e6..54396ebea 100644 --- a/tests/unit/extensions/test_expressjs.py +++ b/tests/unit/extensions/test_expressjs.py @@ -401,11 +401,19 @@ def test_expressjs_invalid_package_json_scripts_error( def test_expressjs_factory_dispatch(tmp_path): factory = extensions.ExpressJSFrameworkFactory - v1 = factory(project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@24.04"}) + v1 = factory( + project_root=tmp_path, + yaml_data={"name": "x", "base": "ubuntu@24.04"}, + extension_name="expressjs-framework", + ) assert isinstance(v1, extensions.ExpressJSFramework) assert not isinstance(v1, extensions.ExpressJSFrameworkV2) - v2 = factory(project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@26.04"}) + v2 = factory( + project_root=tmp_path, + yaml_data={"name": "x", "base": "ubuntu@26.04"}, + extension_name="expressjs-framework", + ) assert isinstance(v2, extensions.ExpressJSFrameworkV2) @@ -426,7 +434,7 @@ def test_expressjs_extension_ubuntu2604_default( expressjs_input_yaml["base"] = "ubuntu@26.04" expressjs_input_yaml["build-base"] = "ubuntu@26.04" expressjs_input_yaml["parts"] = { - "expressjs-framework/install-app": { + "expressjs-framework.install-app": { "npm-include-node": False, "npm-node-version": None, } @@ -440,7 +448,7 @@ def test_expressjs_extension_ubuntu2604_default( "platforms": {"amd64": {}}, "run-user": "_daemon_", "parts": { - "expressjs-framework/install-app": { + "expressjs-framework.install-app": { "plugin": "npm", "source": "app/", "npm-include-node": False, @@ -460,12 +468,12 @@ def test_expressjs_extension_ubuntu2604_default( "stage-packages": ["ca-certificates_data", "nodejs_bins"], "build-environment": [{"UV_USE_IO_URING": "0"}], }, - "expressjs-framework/runtime": { + "expressjs-framework.runtime": { "plugin": "nil", "stage-packages": ["npm"], "stage": ["-etc/ssl/certs/ca-certificates.crt"], }, - "expressjs-framework/logging": { + "expressjs-framework.logging": { "plugin": "nil", "override-build": ( "craftctl default\n" diff --git a/tests/unit/extensions/test_fastapi.py b/tests/unit/extensions/test_fastapi.py index 76ab492b0..2455ce5e3 100644 --- a/tests/unit/extensions/test_fastapi.py +++ b/tests/unit/extensions/test_fastapi.py @@ -337,7 +337,9 @@ def test_fastapi_extension_incorrect_prime_prefix_error(tmp_path, fastapi_input_ def test_factory_dispatch_v1(tmp_path): """Factory returns FastAPIFramework (V1) for ubuntu@24.04.""" instance = extensions.FastAPIFrameworkFactory( - project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@24.04"} + project_root=tmp_path, + yaml_data={"name": "x", "base": "ubuntu@24.04"}, + extension_name="fastapi-framework", ) assert isinstance(instance, extensions.FastAPIFramework) assert not isinstance(instance, extensions.FastAPIFrameworkV2) @@ -346,7 +348,9 @@ def test_factory_dispatch_v1(tmp_path): def test_factory_dispatch_v2(tmp_path): """Factory returns FastAPIFrameworkV2 for ubuntu@26.04.""" instance = extensions.FastAPIFrameworkFactory( - project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@26.04"} + project_root=tmp_path, + yaml_data={"name": "x", "base": "ubuntu@26.04"}, + extension_name="fastapi-framework", ) assert isinstance(instance, extensions.FastAPIFrameworkV2) diff --git a/tests/unit/extensions/test_go.py b/tests/unit/extensions/test_go.py index d3cd2683b..4b7d71af8 100644 --- a/tests/unit/extensions/test_go.py +++ b/tests/unit/extensions/test_go.py @@ -281,11 +281,19 @@ def test_go_extension_extra_assets_overridden(tmp_path, go_input_yaml): def test_go_framework_factory_dispatch(tmp_path): factory = extensions.GoFrameworkFactory - v1 = factory(project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@24.04"}) + v1 = factory( + project_root=tmp_path, + yaml_data={"name": "x", "base": "ubuntu@24.04"}, + extension_name="go-framework", + ) assert isinstance(v1, extensions.GoFramework) assert not isinstance(v1, extensions.GoFrameworkV2) - v2 = factory(project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@26.04"}) + v2 = factory( + project_root=tmp_path, + yaml_data={"name": "x", "base": "ubuntu@26.04"}, + extension_name="go-framework", + ) assert isinstance(v2, extensions.GoFrameworkV2) diff --git a/tests/unit/extensions/test_gunicorn.py b/tests/unit/extensions/test_gunicorn.py index e30667ff5..faa8e47c4 100644 --- a/tests/unit/extensions/test_gunicorn.py +++ b/tests/unit/extensions/test_gunicorn.py @@ -797,13 +797,17 @@ def test_flask_framework_factory_dispatch(tmp_path): ) v1 = FlaskFrameworkFactory( - project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@22.04"} + project_root=tmp_path, + yaml_data={"name": "x", "base": "ubuntu@22.04"}, + extension_name="flask-framework", ) assert isinstance(v1, FlaskFramework) assert not isinstance(v1, FlaskFrameworkV2) v2 = FlaskFrameworkFactory( - project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@26.04"} + project_root=tmp_path, + yaml_data={"name": "x", "base": "ubuntu@26.04"}, + extension_name="flask-framework", ) assert isinstance(v2, FlaskFrameworkV2) @@ -1169,6 +1173,7 @@ def test_django_factory_dispatch_v1(tmp_path): instance = factory( project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@22.04"}, + extension_name="django-framework", ) assert isinstance(instance, extensions.DjangoFramework) assert not isinstance(instance, extensions.DjangoFrameworkV2) @@ -1180,6 +1185,7 @@ def test_django_factory_dispatch_v2(tmp_path): instance = factory( project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@26.04"}, + extension_name="django-framework", ) assert isinstance(instance, extensions.DjangoFrameworkV2) diff --git a/tests/unit/extensions/test_springboot.py b/tests/unit/extensions/test_springboot.py index df7c3c56a..333e8a8bb 100644 --- a/tests/unit/extensions/test_springboot.py +++ b/tests/unit/extensions/test_springboot.py @@ -590,6 +590,7 @@ def test_factory_dispatch_v1(tmp_path): instance = SpringBootFrameworkFactory( project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@24.04"}, + extension_name="spring-boot-framework", ) assert isinstance(instance, SpringBootFramework) assert not isinstance(instance, SpringBootFrameworkV2) @@ -605,6 +606,7 @@ def test_factory_dispatch_v2(tmp_path): instance = SpringBootFrameworkFactory( project_root=tmp_path, yaml_data={"name": "x", "base": "ubuntu@26.04"}, + extension_name="spring-boot-framework", ) assert isinstance(instance, SpringBootFrameworkV2)