From 9f2693ead5405aa653fce7f8451428666f6b958c Mon Sep 17 00:00:00 2001 From: Javier Peletier Date: Wed, 10 Dec 2025 00:59:58 +0100 Subject: [PATCH] [core] Packages refactor and conditional package inclusion (package refactor part 1) (#11605) Co-authored-by: J. Nick Koston --- esphome/components/packages/__init__.py | 151 ++++++++--- esphome/config.py | 10 +- .../component_tests/packages/test_packages.py | 238 ++++++++++++++++-- .../06-package_merging.approved.yaml | 43 ++++ .../06-package_merging.input.yaml | 61 +++++ tests/unit_tests/test_substitutions.py | 6 +- 6 files changed, 446 insertions(+), 63 deletions(-) create mode 100644 tests/unit_tests/fixtures/substitutions/06-package_merging.approved.yaml create mode 100644 tests/unit_tests/fixtures/substitutions/06-package_merging.input.yaml diff --git a/esphome/components/packages/__init__.py b/esphome/components/packages/__init__.py index 15ab11d6b0..6d353ccf11 100644 --- a/esphome/components/packages/__init__.py +++ b/esphome/components/packages/__init__.py @@ -1,5 +1,9 @@ +from collections import UserDict +from collections.abc import Callable +from functools import reduce import logging from pathlib import Path +from typing import Any from esphome import git, yaml_util from esphome.components.substitutions.jinja import has_jinja @@ -15,6 +19,7 @@ from esphome.const import ( CONF_PATH, CONF_REF, CONF_REFRESH, + CONF_SUBSTITUTIONS, CONF_URL, CONF_USERNAME, CONF_VARS, @@ -27,32 +32,43 @@ _LOGGER = logging.getLogger(__name__) DOMAIN = CONF_PACKAGES -def valid_package_contents(package_config: dict): - """Validates that a package_config that will be merged looks as much as possible to a valid config - to fail early on obvious mistakes.""" - if isinstance(package_config, dict): - if CONF_URL in package_config: - # If a URL key is found, then make sure the config conforms to a remote package schema: - return REMOTE_PACKAGE_SCHEMA(package_config) +def validate_has_jinja(value: Any): + if not isinstance(value, str) or not has_jinja(value): + raise cv.Invalid("string does not contain Jinja syntax") + return value - # Validate manually since Voluptuous would regenerate dicts and lose metadata - # such as ESPHomeDataBase - for k, v in package_config.items(): - if not isinstance(k, str): - raise cv.Invalid("Package content keys must be strings") - if isinstance(v, (dict, list, Remove)): - continue # e.g. script: [], psram: !remove, logger: {level: debug} - if v is None: - continue # e.g. web_server: - if isinstance(v, str) and has_jinja(v): - # e.g: remote package shorthand: - # package_name: github://esphome/repo/file.yaml@${ branch } - continue - raise cv.Invalid("Invalid component content in package definition") - return package_config +def valid_package_contents(allow_jinja: bool = True) -> Callable[[Any], dict]: + """Returns a validator that checks if a package_config that will be merged looks as + much as possible to a valid config to fail early on obvious mistakes.""" - raise cv.Invalid("Package contents must be a dict") + def validator(package_config: dict) -> dict: + if isinstance(package_config, dict): + if CONF_URL in package_config: + # If a URL key is found, then make sure the config conforms to a remote package schema: + return REMOTE_PACKAGE_SCHEMA(package_config) + + # Validate manually since Voluptuous would regenerate dicts and lose metadata + # such as ESPHomeDataBase + for k, v in package_config.items(): + if not isinstance(k, str): + raise cv.Invalid("Package content keys must be strings") + if isinstance(v, (dict, list, Remove)): + continue # e.g. script: [], psram: !remove, logger: {level: debug} + if v is None: + continue # e.g. web_server: + if allow_jinja and isinstance(v, str) and has_jinja(v): + # e.g: remote package shorthand: + # package_name: github://esphome/repo/file.yaml@${ branch }, or: + # switch: ${ expression that evals to a switch } + continue + + raise cv.Invalid("Invalid component content in package definition") + return package_config + + raise cv.Invalid("Package contents must be a dict") + + return validator def expand_file_to_files(config: dict): @@ -142,7 +158,10 @@ REMOTE_PACKAGE_SCHEMA = cv.All( PACKAGE_SCHEMA = cv.Any( # A package definition is either: validate_source_shorthand, # A git URL shorthand string that expands to a remote package schema, or REMOTE_PACKAGE_SCHEMA, # a valid remote package schema, or - valid_package_contents, # Something that at least looks like an actual package, e.g. {wifi:{ssid: xxx}} + validate_has_jinja, # a Jinja string that may resolve to a package, or + valid_package_contents( + allow_jinja=True + ), # Something that at least looks like an actual package, e.g. {wifi:{ssid: xxx}} # which will have to be fully validated later as per each component's schema. ) @@ -235,32 +254,84 @@ def _process_remote_package(config: dict, skip_update: bool = False) -> dict: return {"packages": packages} -def _process_package(package_config, config, skip_update: bool = False): - recursive_package = package_config - if CONF_URL in package_config: - package_config = _process_remote_package(package_config, skip_update) - if isinstance(package_config, dict): - recursive_package = do_packages_pass(package_config, skip_update) - return merge_config(recursive_package, config) - - -def do_packages_pass(config: dict, skip_update: bool = False): +def _walk_packages( + config: dict, callback: Callable[[dict], dict], validate_deprecated: bool = True +) -> dict: if CONF_PACKAGES not in config: return config packages = config[CONF_PACKAGES] - with cv.prepend_path(CONF_PACKAGES): + + # The following block and `validate_deprecated` parameter can be safely removed + # once single-package deprecation is effective + if validate_deprecated: packages = CONFIG_SCHEMA(packages) + + with cv.prepend_path(CONF_PACKAGES): if isinstance(packages, dict): for package_name, package_config in reversed(packages.items()): with cv.prepend_path(package_name): - config = _process_package(package_config, config, skip_update) + package_config = callback(package_config) + packages[package_name] = _walk_packages(package_config, callback) elif isinstance(packages, list): - for package_config in reversed(packages): - config = _process_package(package_config, config, skip_update) + for idx in reversed(range(len(packages))): + with cv.prepend_path(idx): + package_config = callback(packages[idx]) + packages[idx] = _walk_packages(package_config, callback) else: raise cv.Invalid( f"Packages must be a key to value mapping or list, got {type(packages)} instead" ) - - del config[CONF_PACKAGES] + config[CONF_PACKAGES] = packages + return config + + +def do_packages_pass(config: dict, skip_update: bool = False) -> dict: + """Processes, downloads and validates all packages in the config. + Also extracts and merges all substitutions found in packages into the main config substitutions. + """ + if CONF_PACKAGES not in config: + return config + + substitutions = UserDict(config.pop(CONF_SUBSTITUTIONS, {})) + + def process_package_callback(package_config: dict) -> dict: + """This will be called for each package found in the config.""" + package_config = PACKAGE_SCHEMA(package_config) + if isinstance(package_config, str): + return package_config # Jinja string, skip processing + if CONF_URL in package_config: + package_config = _process_remote_package(package_config, skip_update) + # Extract substitutions from the package and merge them into the main substitutions: + substitutions.data = merge_config( + package_config.pop(CONF_SUBSTITUTIONS, {}), substitutions.data + ) + return package_config + + _walk_packages(config, process_package_callback) + + if substitutions: + config[CONF_SUBSTITUTIONS] = substitutions.data + + return config + + +def merge_packages(config: dict) -> dict: + """Merges all packages into the main config and removes the `packages:` key.""" + if CONF_PACKAGES not in config: + return config + + # Build flat list of all package configs to merge in priority order: + merge_list: list[dict] = [] + + validate_package = valid_package_contents(allow_jinja=False) + + def process_package_callback(package_config: dict) -> dict: + """This will be called for each package found in the config.""" + merge_list.append(validate_package(package_config)) + return package_config + + _walk_packages(config, process_package_callback, validate_deprecated=False) + # Merge all packages into the main config: + config = reduce(lambda new, old: merge_config(old, new), merge_list, config) + del config[CONF_PACKAGES] return config diff --git a/esphome/config.py b/esphome/config.py index 1c4cdd93c6..694716be34 100644 --- a/esphome/config.py +++ b/esphome/config.py @@ -1012,14 +1012,20 @@ def validate_config( CORE.raw_config = config - # 1.1. Resolve !extend and !remove and check for REPLACEME + # 1.1. Merge packages + if CONF_PACKAGES in config: + from esphome.components.packages import merge_packages + + config = merge_packages(config) + + # 1.2. Resolve !extend and !remove and check for REPLACEME # After this step, there will not be any Extend or Remove values in the config anymore try: resolve_extend_remove(config) except vol.Invalid as err: result.add_error(err) - # 1.2. Load external_components + # 1.3. Load external_components if CONF_EXTERNAL_COMPONENTS in config: from esphome.components.external_components import do_external_components_pass diff --git a/tests/component_tests/packages/test_packages.py b/tests/component_tests/packages/test_packages.py index 34760587df..3829e540d7 100644 --- a/tests/component_tests/packages/test_packages.py +++ b/tests/component_tests/packages/test_packages.py @@ -5,7 +5,7 @@ from unittest.mock import MagicMock, patch import pytest -from esphome.components.packages import CONFIG_SCHEMA, do_packages_pass +from esphome.components.packages import CONFIG_SCHEMA, do_packages_pass, merge_packages from esphome.config import resolve_extend_remove from esphome.config_helpers import Extend, Remove import esphome.config_validation as cv @@ -27,6 +27,7 @@ from esphome.const import ( CONF_REFRESH, CONF_SENSOR, CONF_SSID, + CONF_SUBSTITUTIONS, CONF_UPDATE_INTERVAL, CONF_URL, CONF_VARS, @@ -68,11 +69,12 @@ def fixture_basic_esphome(): def packages_pass(config): """Wrapper around packages_pass that also resolves Extend and Remove.""" config = do_packages_pass(config) + config = merge_packages(config) resolve_extend_remove(config) return config -def test_package_unused(basic_esphome, basic_wifi): +def test_package_unused(basic_esphome, basic_wifi) -> None: """ Ensures do_package_pass does not change a config if packages aren't used. """ @@ -82,7 +84,7 @@ def test_package_unused(basic_esphome, basic_wifi): assert actual == config -def test_package_invalid_dict(basic_esphome, basic_wifi): +def test_package_invalid_dict(basic_esphome, basic_wifi) -> None: """ If a url: key is present, it's expected to be well-formed remote package spec. Ensure an error is raised if not. Any other simple dict passed as a package will be merged as usual but may fail later validation. @@ -107,7 +109,7 @@ def test_package_invalid_dict(basic_esphome, basic_wifi): ], ], ) -def test_package_shorthand(packages): +def test_package_shorthand(packages) -> None: CONFIG_SCHEMA(packages) @@ -133,12 +135,12 @@ def test_package_shorthand(packages): [3], ], ) -def test_package_invalid(packages): +def test_package_invalid(packages) -> None: with pytest.raises(cv.Invalid): CONFIG_SCHEMA(packages) -def test_package_include(basic_wifi, basic_esphome): +def test_package_include(basic_wifi, basic_esphome) -> None: """ Tests the simple case where an independent config present in a package is added to the top-level config as is. @@ -159,7 +161,7 @@ def test_single_package( basic_esphome, basic_wifi, caplog: pytest.LogCaptureFixture, -): +) -> None: """ Tests the simple case where a single package is added to the top-level config as is. In this test, the CONF_WIFI config is expected to be simply added to the top-level config. @@ -179,7 +181,7 @@ def test_single_package( assert "This method for including packages will go away in 2026.7.0" in caplog.text -def test_package_append(basic_wifi, basic_esphome): +def test_package_append(basic_wifi, basic_esphome) -> None: """ Tests the case where a key is present in both a package and top-level config. @@ -204,7 +206,7 @@ def test_package_append(basic_wifi, basic_esphome): assert actual == expected -def test_package_override(basic_wifi, basic_esphome): +def test_package_override(basic_wifi, basic_esphome) -> None: """ Ensures that the top-level configuration takes precedence over duplicate keys defined in a package. @@ -228,7 +230,7 @@ def test_package_override(basic_wifi, basic_esphome): assert actual == expected -def test_multiple_package_order(): +def test_multiple_package_order() -> None: """ Ensures that mutiple packages are merged in order. """ @@ -257,7 +259,7 @@ def test_multiple_package_order(): assert actual == expected -def test_package_list_merge(): +def test_package_list_merge() -> None: """ Ensures lists defined in both a package and the top-level config are merged correctly """ @@ -313,7 +315,7 @@ def test_package_list_merge(): assert actual == expected -def test_package_list_merge_by_id(): +def test_package_list_merge_by_id() -> None: """ Ensures that components with matching IDs are merged correctly. @@ -391,7 +393,7 @@ def test_package_list_merge_by_id(): assert actual == expected -def test_package_merge_by_id_with_list(): +def test_package_merge_by_id_with_list() -> None: """ Ensures that components with matching IDs are merged correctly when their configuration contains lists. @@ -430,7 +432,7 @@ def test_package_merge_by_id_with_list(): assert actual == expected -def test_package_merge_by_missing_id(): +def test_package_merge_by_missing_id() -> None: """ Ensures that a validation error is thrown when trying to extend a missing ID. """ @@ -466,7 +468,7 @@ def test_package_merge_by_missing_id(): assert error_raised -def test_package_list_remove_by_id(): +def test_package_list_remove_by_id() -> None: """ Ensures that components with matching IDs are removed correctly. @@ -517,7 +519,7 @@ def test_package_list_remove_by_id(): assert actual == expected -def test_multiple_package_list_remove_by_id(): +def test_multiple_package_list_remove_by_id() -> None: """ Ensures that components with matching IDs are removed correctly. @@ -563,7 +565,7 @@ def test_multiple_package_list_remove_by_id(): assert actual == expected -def test_package_dict_remove_by_id(basic_wifi, basic_esphome): +def test_package_dict_remove_by_id(basic_wifi, basic_esphome) -> None: """ Ensures that components with missing IDs are removed from dict. Ensures that the top-level configuration takes precedence over duplicate keys defined in a package. @@ -584,7 +586,7 @@ def test_package_dict_remove_by_id(basic_wifi, basic_esphome): assert actual == expected -def test_package_remove_by_missing_id(): +def test_package_remove_by_missing_id() -> None: """ Ensures that components with missing IDs are not merged. """ @@ -632,7 +634,7 @@ def test_package_remove_by_missing_id(): @patch("esphome.git.clone_or_update") def test_remote_packages_with_files_list( mock_clone_or_update, mock_is_file, mock_load_yaml -): +) -> None: """ Ensures that packages are loaded as mixed list of dictionary and strings """ @@ -704,7 +706,7 @@ def test_remote_packages_with_files_list( @patch("esphome.git.clone_or_update") def test_remote_packages_with_files_and_vars( mock_clone_or_update, mock_is_file, mock_load_yaml -): +) -> None: """ Ensures that packages are loaded as mixed list of dictionary and strings with vars """ @@ -793,3 +795,199 @@ def test_remote_packages_with_files_and_vars( actual = packages_pass(config) assert actual == expected + + +def test_packages_merge_substitutions() -> None: + """ + Tests that substitutions from packages in a complex package hierarchy + are extracted and merged into the top-level config. + """ + config = { + CONF_SUBSTITUTIONS: { + "a": 1, + "b": 2, + "c": 3, + }, + CONF_PACKAGES: { + "package1": { + "logger": { + "level": "DEBUG", + }, + CONF_PACKAGES: [ + { + CONF_SUBSTITUTIONS: { + "a": 10, + "e": 5, + }, + "sensor": [ + {"platform": "template", "id": "sensor1"}, + ], + }, + ], + "sensor": [ + {"platform": "template", "id": "sensor2"}, + ], + }, + "package2": { + "logger": { + "level": "VERBOSE", + }, + }, + "package3": { + CONF_PACKAGES: [ + { + CONF_PACKAGES: [ + { + CONF_SUBSTITUTIONS: { + "b": 20, + "d": 4, + }, + "sensor": [ + {"platform": "template", "id": "sensor3"}, + ], + }, + ], + CONF_SUBSTITUTIONS: { + "b": 20, + "d": 6, + }, + "sensor": [ + {"platform": "template", "id": "sensor4"}, + ], + }, + ], + }, + }, + } + + expected = { + CONF_SUBSTITUTIONS: {"a": 1, "e": 5, "b": 2, "d": 6, "c": 3}, + CONF_PACKAGES: { + "package1": { + "logger": { + "level": "DEBUG", + }, + CONF_PACKAGES: [ + { + "sensor": [ + {"platform": "template", "id": "sensor1"}, + ], + }, + ], + "sensor": [ + {"platform": "template", "id": "sensor2"}, + ], + }, + "package2": { + "logger": { + "level": "VERBOSE", + }, + }, + "package3": { + CONF_PACKAGES: [ + { + CONF_PACKAGES: [ + { + "sensor": [ + {"platform": "template", "id": "sensor3"}, + ], + }, + ], + "sensor": [ + {"platform": "template", "id": "sensor4"}, + ], + }, + ], + }, + }, + } + + actual = do_packages_pass(config) + assert actual == expected + + +def test_package_merge() -> None: + """ + Tests that all packages are merged into the top-level config. + """ + config = { + CONF_SUBSTITUTIONS: {"a": 1, "e": 5, "b": 2, "d": 6, "c": 3}, + CONF_PACKAGES: { + "package1": { + "logger": { + "level": "DEBUG", + }, + CONF_PACKAGES: [ + { + "sensor": [ + {"platform": "template", "id": "sensor1"}, + ], + }, + ], + "sensor": [ + {"platform": "template", "id": "sensor2"}, + ], + }, + "package2": { + "logger": { + "level": "VERBOSE", + }, + }, + "package3": { + CONF_PACKAGES: [ + { + CONF_PACKAGES: [ + { + "sensor": [ + {"platform": "template", "id": "sensor3"}, + ], + }, + ], + "sensor": [ + {"platform": "template", "id": "sensor4"}, + ], + }, + ], + }, + }, + } + expected = { + "sensor": [ + {"platform": "template", "id": "sensor1"}, + {"platform": "template", "id": "sensor2"}, + {"platform": "template", "id": "sensor3"}, + {"platform": "template", "id": "sensor4"}, + ], + "logger": {"level": "VERBOSE"}, + CONF_SUBSTITUTIONS: {"a": 1, "e": 5, "b": 2, "d": 6, "c": 3}, + } + actual = merge_packages(config) + + assert actual == expected + + +@pytest.mark.parametrize( + "invalid_package", + [ + 6, + "some string", + ["some string"], + None, + True, + {"some_component": 8}, + {3: 2}, + {"some_component": r"${unevaluated expression}"}, + ], +) +def test_package_merge_invalid(invalid_package) -> None: + """ + Tests that trying to merge an invalid package raises an error. + """ + config = { + CONF_PACKAGES: { + "some_package": invalid_package, + }, + } + + with pytest.raises(cv.Invalid): + merge_packages(config) diff --git a/tests/unit_tests/fixtures/substitutions/06-package_merging.approved.yaml b/tests/unit_tests/fixtures/substitutions/06-package_merging.approved.yaml new file mode 100644 index 0000000000..3fbf5660d5 --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/06-package_merging.approved.yaml @@ -0,0 +1,43 @@ +fancy_component: &id001 + - id: component9 + value: 9 +some_component: + - id: component1 + value: 1 + - id: component2 + value: 2 + - id: component3 + value: 3 + - id: component4 + value: 4 + - id: component5 + value: 79 + power: 200 + - id: component6 + value: 6 + - id: component7 + value: 7 +switch: &id002 + - platform: gpio + id: switch1 + pin: 12 + - platform: gpio + id: switch2 + pin: 13 +display: + - platform: ili9xxx + dimensions: + width: 100 + height: 480 +substitutions: + extended_component: component5 + package_options: + alternative_package: + alternative_component: + - id: component8 + value: 8 + fancy_package: + fancy_component: *id001 + pin: 12 + some_switches: *id002 + package_selection: fancy_package diff --git a/tests/unit_tests/fixtures/substitutions/06-package_merging.input.yaml b/tests/unit_tests/fixtures/substitutions/06-package_merging.input.yaml new file mode 100644 index 0000000000..d937a89306 --- /dev/null +++ b/tests/unit_tests/fixtures/substitutions/06-package_merging.input.yaml @@ -0,0 +1,61 @@ +substitutions: + package_options: + alternative_package: + alternative_component: + - id: component8 + value: 8 + fancy_package: + fancy_component: + - id: component9 + value: 9 + + pin: 12 + some_switches: + - platform: gpio + id: switch1 + pin: ${pin} + - platform: gpio + id: switch2 + pin: ${pin+1} + + package_selection: fancy_package + +packages: + - ${ package_options[package_selection] } + - some_component: + - id: component1 + value: 1 + - some_component: + - id: component2 + value: 2 + - switch: ${ some_switches } + - packages: + package_with_defaults: !include + file: display.yaml + vars: + native_width: 100 + high_dpi: false + my_package: + packages: + - packages: + special_package: + substitutions: + extended_component: component5 + some_component: + - id: component3 + value: 3 + some_component: + - id: component4 + value: 4 + - id: !extend ${ extended_component } + power: 200 + value: 79 + some_component: + - id: component5 + value: 5 + +some_component: + - id: component6 + value: 6 + - id: component7 + value: 7 diff --git a/tests/unit_tests/test_substitutions.py b/tests/unit_tests/test_substitutions.py index cba1e398c3..1d8cb7631d 100644 --- a/tests/unit_tests/test_substitutions.py +++ b/tests/unit_tests/test_substitutions.py @@ -8,7 +8,7 @@ import pytest from esphome import config as config_module, yaml_util from esphome.components import substitutions -from esphome.components.packages import do_packages_pass +from esphome.components.packages import do_packages_pass, merge_packages from esphome.config import resolve_extend_remove from esphome.config_helpers import merge_config from esphome.const import CONF_SUBSTITUTIONS @@ -74,6 +74,8 @@ def verify_database(value: Any, path: str = "") -> str | None: return None if isinstance(value, dict): for k, v in value.items(): + if path == "" and k == CONF_SUBSTITUTIONS: + return None # ignore substitutions key at top level since it is merged. key_result = verify_database(k, f"{path}/{k}") if key_result is not None: return key_result @@ -144,6 +146,8 @@ def test_substitutions_fixtures( substitutions.do_substitution_pass(config, command_line_substitutions) + config = merge_packages(config) + resolve_extend_remove(config) verify_database_result = verify_database(config) if verify_database_result is not None: