[core] Packages refactor and conditional package inclusion (package refactor part 1) (#11605)

Co-authored-by: J. Nick Koston <nick@koston.org>

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

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
 

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)

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

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

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: