From 1b3891866c87b03f9c79ecd09644b60cac863473 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 3 Aug 2026 20:59:38 -0500 Subject: [PATCH] [core] Parse stored framework versions without importing the validation stack (#18045) --- esphome/components/esp32/const.py | 11 ++-- esphome/config_validation.py | 38 +------------- esphome/const.py | 5 ++ esphome/core/__init__.py | 37 +++++++++++++ esphome/espidf/clang_tidy.py | 15 ++++-- esphome/espidf/framework.py | 3 +- esphome/helpers.py | 2 +- esphome/storage_json.py | 49 ++++++++--------- .../lazy_imports/storage_json_fast_path.py | 52 +++++++++++++++++++ tests/unit_tests/test_compiled_config.py | 7 +-- tests/unit_tests/test_lazy_imports.py | 45 ++++++++++++++++ 11 files changed, 182 insertions(+), 82 deletions(-) create mode 100644 tests/unit_tests/fixtures/lazy_imports/storage_json_fast_path.py diff --git a/esphome/components/esp32/const.py b/esphome/components/esp32/const.py index 248f84c6bc..af386b618a 100644 --- a/esphome/components/esp32/const.py +++ b/esphome/components/esp32/const.py @@ -1,9 +1,15 @@ import esphome.codegen as cg -KEY_ESP32 = "esp32" +# Re-exported for the many esp32-side users; defined in esphome.const so +# the upload/logs fast path can read them without importing this package. +from esphome.const import ( # noqa: F401 # pylint: disable=unused-import + KEY_ESP32, + KEY_IDF_VERSION, + KEY_VARIANT, +) + KEY_BOARD = "board" KEY_FLASH_SIZE = "flash_size" -KEY_VARIANT = "variant" KEY_SDKCONFIG_OPTIONS = "sdkconfig_options" KEY_COMPONENTS = "components" KEY_EXCLUDE_COMPONENTS = "exclude_components" @@ -15,7 +21,6 @@ KEY_PATH = "path" KEY_SUBMODULES = "submodules" KEY_EXTRA_BUILD_FILES = "extra_build_files" KEY_FULL_CERT_BUNDLE = "full_cert_bundle" -KEY_IDF_VERSION = "idf_version" KEY_NETWORK_SDKCONFIG = "network_sdkconfig" VARIANT_ESP32 = "ESP32" diff --git a/esphome/config_validation.py b/esphome/config_validation.py index ff9170813c..2de6898177 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -4,7 +4,6 @@ from __future__ import annotations from collections.abc import Callable from contextlib import contextmanager, suppress -from dataclasses import dataclass from datetime import datetime from ipaddress import ( AddressValueError, @@ -88,6 +87,7 @@ from esphome.core import ( TimePeriodMinutes, TimePeriodNanoseconds, TimePeriodSeconds, + Version, ) from esphome.enum import StrEnum from esphome.expression import SUBSTITUTION_VARIABLE_PROG as VARIABLE_PROG @@ -408,42 +408,6 @@ class FinalExternalInvalid(Invalid): """Represents an invalid value in the final validation phase where the path should not be prepended.""" -@dataclass(frozen=True, order=True) -class Version: - major: int - minor: int - patch: int - extra: str = "" - - def __str__(self): - if self.extra: - return f"{self.major}.{self.minor}.{self.patch}-{self.extra}" - return f"{self.major}.{self.minor}.{self.patch}" - - @classmethod - def parse(cls, value: str) -> Version: - # The patch component is optional and defaults to 0, so "6.0" and - # "6.0-rc1" parse as 6.0.0 and 6.0.0-rc1. - match = re.match(r"^(\d+)\.(\d+)(?:\.(\d+))?[-.]?(\w*)$", value) - if match is None: - raise ValueError(f"Not a valid version number {value}") - major = int(match[1]) - minor = int(match[2]) - patch = int(match[3] or 0) - extra = match[4] or "" - return Version(major=major, minor=minor, patch=patch, extra=extra) - - @property - def is_beta(self) -> bool: - """Check if this version is a beta version.""" - return self.extra.startswith("b") - - @property - def is_dev(self) -> bool: - """Check if this version is a development version.""" - return self.extra.startswith("dev") - - def check_not_templatable(value): if isinstance(value, Lambda): raise Invalid("This option is not templatable!") diff --git a/esphome/const.py b/esphome/const.py index f2d305dced..b1302af922 100644 --- a/esphome/const.py +++ b/esphome/const.py @@ -1422,6 +1422,11 @@ KEY_FRAMEWORK_VERSION = "framework_version" KEY_NAME = "name" KEY_VARIANT = "variant" KEY_PAST_SAFE_MODE = "past_safe_mode" +# esp32 storage keys; defined here so the upload/logs fast path +# (storage_json.apply_to_core) can use them without importing the +# esp32 component package. +KEY_ESP32 = "esp32" +KEY_IDF_VERSION = "idf_version" # Entity categories ENTITY_CATEGORY_NONE = "" diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index deee127f49..e5b3ebb84d 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -1,5 +1,6 @@ from collections import defaultdict from contextlib import contextmanager +from dataclasses import dataclass import logging import math import os @@ -279,6 +280,42 @@ class TimePeriodMinutes(TimePeriod): pass +@dataclass(frozen=True, order=True) +class Version: + major: int + minor: int + patch: int + extra: str = "" + + def __str__(self): + if self.extra: + return f"{self.major}.{self.minor}.{self.patch}-{self.extra}" + return f"{self.major}.{self.minor}.{self.patch}" + + @classmethod + def parse(cls, value: str) -> "Version": + # The patch component is optional and defaults to 0, so "6.0" and + # "6.0-rc1" parse as 6.0.0 and 6.0.0-rc1. + match = re.match(r"^(\d+)\.(\d+)(?:\.(\d+))?[-.]?(\w*)$", value) + if match is None: + raise ValueError(f"Not a valid version number {value}") + major = int(match[1]) + minor = int(match[2]) + patch = int(match[3] or 0) + extra = match[4] or "" + return Version(major=major, minor=minor, patch=patch, extra=extra) + + @property + def is_beta(self) -> bool: + """Check if this version is a beta version.""" + return self.extra.startswith("b") + + @property + def is_dev(self) -> bool: + """Check if this version is a development version.""" + return self.extra.startswith("dev") + + LAMBDA_PROG = re.compile(r"\bid\(\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*\)(\.?)") diff --git a/esphome/espidf/clang_tidy.py b/esphome/espidf/clang_tidy.py index 88ecda60b9..c91db775a3 100644 --- a/esphome/espidf/clang_tidy.py +++ b/esphome/espidf/clang_tidy.py @@ -141,10 +141,15 @@ idf_component_register( def _setup_core(work_dir: Path, settings: _Settings) -> None: """Point CORE at the tidy project + IDF version, without any YAML config.""" - from esphome.components.esp32.const import KEY_ESP32, KEY_IDF_VERSION, KEY_VARIANT - import esphome.config_validation as cv - from esphome.const import KEY_CORE, KEY_TARGET_FRAMEWORK, KEY_TARGET_PLATFORM - from esphome.core import CORE + from esphome.const import ( + KEY_CORE, + KEY_ESP32, + KEY_IDF_VERSION, + KEY_TARGET_FRAMEWORK, + KEY_TARGET_PLATFORM, + KEY_VARIANT, + ) + from esphome.core import CORE, Version CORE.name = TIDY_PROJECT_NAME # config_path's parent is the data dir root for per-run artifacts (idedata, @@ -153,7 +158,7 @@ def _setup_core(work_dir: Path, settings: _Settings) -> None: CORE.config_path = work_dir.parent / "tidy.yaml" CORE.build_path = work_dir esp32 = CORE.data.setdefault(KEY_ESP32, {}) - esp32[KEY_IDF_VERSION] = cv.Version.parse(settings.idf_version) + esp32[KEY_IDF_VERSION] = Version.parse(settings.idf_version) esp32[KEY_VARIANT] = settings.variant # The target framework drives the PlatformIO-library -> IDF-component # converter and ESPHome's CORE.using_arduino / using_esp_idf helpers. diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 0ca7a9d14b..39bf0465d5 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -13,8 +13,7 @@ from typing import Any, NoReturn import platformdirs -from esphome.config_validation import Version -from esphome.core import CORE +from esphome.core import CORE, Version from esphome.framework_helpers import ( PathType, archive_extract_all, diff --git a/esphome/helpers.py b/esphome/helpers.py index 5c57a2823b..5458f5edfc 100644 --- a/esphome/helpers.py +++ b/esphome/helpers.py @@ -705,7 +705,7 @@ class ProgressBar: def docs_url(path: str) -> str: """Return the URL to the documentation for a given path.""" # Local import to avoid circular import - from esphome.config_validation import Version + from esphome.core import Version version = Version.parse(ESPHOME_VERSION) if version.is_beta: diff --git a/esphome/storage_json.py b/esphome/storage_json.py index 6376e573c4..2aa76aabaa 100644 --- a/esphome/storage_json.py +++ b/esphome/storage_json.py @@ -12,12 +12,15 @@ from esphome.const import ( CONF_DISABLED, CONF_MDNS, KEY_CORE, + KEY_ESP32, KEY_FRAMEWORK_VERSION, + KEY_IDF_VERSION, KEY_TARGET_FRAMEWORK, KEY_TARGET_PLATFORM, + KEY_VARIANT, Toolchain, ) -from esphome.core import CORE, EsphomeError +from esphome.core import CORE, EsphomeError, Version from esphome.helpers import write_file_if_changed from esphome.types import CoreType @@ -69,6 +72,17 @@ def _to_path_if_not_none(value: str | None) -> Path | None: return Path(value) if value is not None else None +def _parse_framework_version(framework_version: str) -> Version: + try: + return Version.parse(framework_version) + except ValueError as err: + raise EsphomeError( + f"Could not parse the framework version " + f"{framework_version!r} from {storage_path()}. " + f"Please clean the build files and recompile." + ) from err + + class StorageJSON: """Persisted device metadata sidecar. @@ -319,37 +333,16 @@ class StorageJSON: # esp32.get_esp32_variant(). target_platform on disk is the variant # (e.g. "ESP32S3"); core_platform is the family (e.g. "esp32"). if target_platform == const.PLATFORM_ESP32: - from esphome.components.esp32.const import KEY_ESP32, KEY_IDF_VERSION - from esphome.const import KEY_VARIANT - esp32_data = {KEY_VARIANT: self.target_platform} if self.framework_version: - import esphome.config_validation as cv - - try: - esp32_data[KEY_IDF_VERSION] = cv.Version.parse( - self.framework_version - ) - except ValueError as err: - raise EsphomeError( - f"Could not parse the framework version " - f"{self.framework_version!r} from {storage_path()}. " - f"Please clean the build files and recompile." - ) from err - CORE.data[KEY_ESP32] = esp32_data - elif target_platform == const.PLATFORM_NRF52 and self.framework_version: - import esphome.config_validation as cv - - try: - CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION] = cv.Version.parse( + esp32_data[KEY_IDF_VERSION] = _parse_framework_version( self.framework_version ) - except ValueError as err: - raise EsphomeError( - f"Could not parse the framework version " - f"{self.framework_version!r} from {storage_path()}. " - f"Please clean the build files and recompile." - ) from err + CORE.data[KEY_ESP32] = esp32_data + elif target_platform == const.PLATFORM_NRF52 and self.framework_version: + CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION] = _parse_framework_version( + self.framework_version + ) def __eq__(self, o) -> bool: return isinstance(o, StorageJSON) and self.as_dict() == o.as_dict() diff --git a/tests/unit_tests/fixtures/lazy_imports/storage_json_fast_path.py b/tests/unit_tests/fixtures/lazy_imports/storage_json_fast_path.py new file mode 100644 index 0000000000..01b23b8f04 --- /dev/null +++ b/tests/unit_tests/fixtures/lazy_imports/storage_json_fast_path.py @@ -0,0 +1,52 @@ +"""Run the esp32 storage fast path and report which heavy modules loaded. + +Executed as a subprocess by test_lazy_imports.py: heavy module names come +in on argv, the ones found in sys.modules afterwards go out on stdout. +""" + +import sys + +from esphome.const import KEY_ESP32, KEY_IDF_VERSION, KEY_VARIANT +from esphome.core import CORE, Version +from esphome.storage_json import StorageJSON + +storage = StorageJSON( + storage_version=1, + name="test", + friendly_name="Test", + comment=None, + esphome_version="2026.1.0", + src_version=1, + address="1.2.3.4", + web_port=None, + target_platform="ESP32S3", + build_path=None, + firmware_bin_path=None, + loaded_integrations=set(), + loaded_platforms=set(), + no_mdns=False, + framework="esp-idf", + core_platform="esp32", + area=None, + framework_version="5.3.1", +) +storage.apply_to_core() + +# Fail loudly if the esp32 fast path stopped doing its work; otherwise an +# empty leak list could just mean nothing ran. Explicit exits rather than +# asserts so PYTHONOPTIMIZE in the ambient environment can't strip them. +esp32_data = CORE.data.get(KEY_ESP32, {}) +if esp32_data.get(KEY_VARIANT) != "ESP32S3": + sys.exit(f"apply_to_core did not record the variant: {esp32_data!r}") +if esp32_data.get(KEY_IDF_VERSION) != Version(5, 3, 1): + sys.exit(f"apply_to_core did not parse the framework version: {esp32_data!r}") + +# Any component package counts as a leak, not just the ones on the watch +# list: executing one drags in codegen/validation machinery by design. +leaked = [module for module in sys.argv[1:] if module in sys.modules] +leaked += [ + module + for module in sys.modules + if module.startswith("esphome.components.") and module not in leaked +] +print(",".join(leaked)) diff --git a/tests/unit_tests/test_compiled_config.py b/tests/unit_tests/test_compiled_config.py index e17271e2b4..4219424aa1 100644 --- a/tests/unit_tests/test_compiled_config.py +++ b/tests/unit_tests/test_compiled_config.py @@ -20,6 +20,7 @@ from esphome.const import ( CONF_ESPHOME, CONF_NAME, KEY_CORE, + KEY_ESP32, KEY_TARGET_FRAMEWORK, KEY_TARGET_PLATFORM, KEY_VARIANT, @@ -130,15 +131,11 @@ def test_load_compiled_config_happy_path(fresh_cache_files: Path) -> None: assert CORE.data[KEY_CORE][KEY_TARGET_PLATFORM] == "esp32" assert CORE.data[KEY_CORE][KEY_TARGET_FRAMEWORK] == "arduino" # upload_using_esptool reads get_esp32_variant() off CORE.data[KEY_ESP32]. - from esphome.components.esp32.const import KEY_ESP32 - assert CORE.data[KEY_ESP32][KEY_VARIANT] == "ESP32" def test_load_compiled_config_populates_esp32_variant(tmp_path: Path) -> None: """ESP32 variants survive the cache fast path so esptool gets the right --chip.""" - from esphome.components.esp32.const import KEY_ESP32 - yaml_path = tmp_path / "lite_test.yaml" yaml_path.write_text("esphome:\n name: lite_test\n") CORE.config_path = yaml_path @@ -156,8 +153,6 @@ def test_load_compiled_config_skips_esp32_block_for_other_platforms( tmp_path: Path, ) -> None: """Non-esp32 targets shouldn't fabricate an esp32 data block.""" - from esphome.components.esp32.const import KEY_ESP32 - yaml_path = tmp_path / "lite_test.yaml" yaml_path.write_text("esphome:\n name: lite_test\n") CORE.config_path = yaml_path diff --git a/tests/unit_tests/test_lazy_imports.py b/tests/unit_tests/test_lazy_imports.py index 72ba73c9dc..7eff5c9ef5 100644 --- a/tests/unit_tests/test_lazy_imports.py +++ b/tests/unit_tests/test_lazy_imports.py @@ -14,6 +14,9 @@ test pins down *which* heavy modules must stay out entirely. from __future__ import annotations +import importlib.util +import os +from pathlib import Path import subprocess import sys @@ -30,6 +33,10 @@ HEAVY_MODULES = ( "voluptuous", ) +# Everything the storage fast path must keep out of sys.modules; the +# existence guard and the leak check must watch the same list. +FAST_PATH_HEAVY_MODULES = HEAVY_MODULES + ("esphome.components.esp32",) + def _leaked_heavy_modules(module: str) -> str: """Import ``module`` in a subprocess and report the heavy modules it pulled. @@ -63,6 +70,44 @@ def test_main_module_does_not_import_heavy_modules() -> None: ) +def test_watched_heavy_modules_exist() -> None: + """A renamed heavy module would silently disable the leak checks.""" + for module in FAST_PATH_HEAVY_MODULES: + assert importlib.util.find_spec(module) is not None, ( + f"{module} no longer resolves; update the heavy-module lists" + ) + + +def test_storage_json_fast_path_does_not_import_heavy_modules( + fixture_path: Path, +) -> None: + """``apply_to_core`` runs on the upload/logs fast path for every + platform; parsing the stored framework version must not drag in the + validation stack or the esp32 component package. + """ + script = fixture_path / "lazy_imports" / "storage_json_fast_path.py" + # Running a script file drops the cwd from sys.path, so prepend the + # repo root for the child; check=False keeps its stderr visible. + python_path = str(Path(__file__).parents[2]) + if ambient := os.environ.get("PYTHONPATH"): + python_path = os.pathsep.join((python_path, ambient)) + env = os.environ | {"PYTHONPATH": python_path} + result = subprocess.run( + [sys.executable, str(script), *FAST_PATH_HEAVY_MODULES], + capture_output=True, + text=True, + env=env, + check=False, + ) + assert result.returncode == 0, result.stderr + leaked = result.stdout.strip() + assert not leaked, ( + f"storage_json.apply_to_core pulls in heavy modules: {leaked}. " + "The upload/logs fast path skips validation; importing the " + "validation stack anyway defeats the validated-config cache." + ) + + def test_api_client_does_not_import_heavy_modules() -> None: """``esphome.api_client`` is on the logs fast path and must stay light.