From 6ad3e0d8ad0210045fb63a63a20626267bd63c4c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 15:44:23 -0500 Subject: [PATCH 1/4] Add toolchain validator factories and a native-toolchain predicate --- esphome/components/esp32/__init__.py | 19 ++++--------- esphome/components/nrf52/__init__.py | 12 +++------ esphome/config_validation.py | 40 ++++++++++++++++++++++------ esphome/const.py | 6 +++++ esphome/core/__init__.py | 7 +++++ esphome/core/config.py | 4 +-- 6 files changed, 54 insertions(+), 34 deletions(-) diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index 201c69a8c0..120e4d8ccd 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -1071,20 +1071,11 @@ def _check_esp_idf_versions(config: ConfigType) -> ConfigType: return config -def _validate_toolchain(value) -> Toolchain: - return Toolchain( - cv.one_of(Toolchain.PLATFORMIO, Toolchain.ESP_IDF, lower=True)(value) - ) - - -def _resolve_toolchain(value: ConfigType) -> ConfigType: - # Resolve toolchain: CLI (already on CORE.toolchain) > YAML > default. - # Runs before _detect_variant so downstream validators can rely on - # CORE.toolchain instead of re-resolving it from the config dict. - if CORE.toolchain is None: - CORE.toolchain = value.get(CONF_TOOLCHAIN, Toolchain.ESP_IDF) - cv.check_supported_toolchain("ESP32", (Toolchain.PLATFORMIO, Toolchain.ESP_IDF)) - return value +_TOOLCHAINS = (Toolchain.PLATFORMIO, Toolchain.ESP_IDF) +_validate_toolchain = cv.toolchain_enum(_TOOLCHAINS) +# Runs before _detect_variant so downstream validators can rely on +# CORE.toolchain instead of re-resolving it from the config dict. +_resolve_toolchain = cv.resolve_toolchain("ESP32", _TOOLCHAINS, Toolchain.ESP_IDF) def _check_versions(config: ConfigType) -> ConfigType: diff --git a/esphome/components/nrf52/__init__.py b/esphome/components/nrf52/__init__.py index fe0c1a12de..aeeaba0c11 100644 --- a/esphome/components/nrf52/__init__.py +++ b/esphome/components/nrf52/__init__.py @@ -125,11 +125,8 @@ def set_core_data(config: ConfigType) -> ConfigType: return config -def _resolve_toolchain(config: ConfigType) -> ConfigType: - if CORE.toolchain is None: - CORE.toolchain = config.get(CONF_TOOLCHAIN, Toolchain.SDK_NRF) - cv.check_supported_toolchain("nRF52", (Toolchain.PLATFORMIO, Toolchain.SDK_NRF)) - return config +_TOOLCHAINS = (Toolchain.PLATFORMIO, Toolchain.SDK_NRF) +_resolve_toolchain = cv.resolve_toolchain("nRF52", _TOOLCHAINS, Toolchain.SDK_NRF) def set_framework(config: ConfigType) -> ConfigType: @@ -171,10 +168,7 @@ BOOTLOADERS = [ ] -def _validate_toolchain(value) -> Toolchain: - return Toolchain( - cv.one_of(Toolchain.PLATFORMIO, Toolchain.SDK_NRF, lower=True)(value) - ) +_validate_toolchain = cv.toolchain_enum(_TOOLCHAINS) def _detect_bootloader(config: ConfigType) -> ConfigType: diff --git a/esphome/config_validation.py b/esphome/config_validation.py index df0c152b13..58b89e836f 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -53,6 +53,7 @@ from esphome.const import ( CONF_SETUP_PRIORITY, CONF_STATE_TOPIC, CONF_SUBSCRIBE_QOS, + CONF_TOOLCHAIN, CONF_TOPIC, CONF_TYPE, CONF_TYPE_ID, @@ -2555,20 +2556,43 @@ def check_supported_toolchain( ) +def toolchain_enum(supported: tuple[Toolchain, ...]): + """Schema validator for a platform's ``toolchain`` config key.""" + + def validator(value) -> Toolchain: + return Toolchain(one_of(*supported, lower=True)(value)) + + return validator + + +def resolve_toolchain( + platform_name: str, supported: tuple[Toolchain, ...], default: Toolchain +): + """Resolve ``CORE.toolchain`` (CLI > YAML > default) and reject one the + platform cannot serve. + + Add to the platform's validation chain before anything that reads + ``CORE.toolchain``. + """ + + def validator(config: ConfigType) -> ConfigType: + if CORE.toolchain is None: + CORE.toolchain = config.get(CONF_TOOLCHAIN, default) + check_supported_toolchain(platform_name, supported) + return config + + return validator + + def require_platformio_toolchain(platform_name: str): """Reject a CLI-selected toolchain other than PlatformIO. For platforms with only the PlatformIO backend; without this a ``--toolchain`` they cannot serve would silently build with PlatformIO. """ - - def validator(config: ConfigType) -> ConfigType: - if CORE.toolchain is None: - CORE.toolchain = Toolchain.PLATFORMIO - check_supported_toolchain(platform_name, (Toolchain.PLATFORMIO,)) - return config - - return validator + return resolve_toolchain( + platform_name, (Toolchain.PLATFORMIO,), Toolchain.PLATFORMIO + ) def require_framework_version( diff --git a/esphome/const.py b/esphome/const.py index 6e9378ec9a..6f83f0c937 100644 --- a/esphome/const.py +++ b/esphome/const.py @@ -25,6 +25,12 @@ class Toolchain(StrEnum): ARDUINO = "arduino" +# Toolchains that drive their build natively and never read platformio.ini. +# SDK_NRF is absent on purpose: the zephyr backend keeps consuming +# platformio_options. +NATIVE_TOOLCHAINS = frozenset({Toolchain.ESP_IDF, Toolchain.ARDUINO}) + + class Platform(StrEnum): """Platform identifiers for ESPHome.""" diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index afe57f29b4..a20a60e322 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -21,6 +21,7 @@ from esphome.const import ( KEY_CORE, KEY_TARGET_FRAMEWORK, KEY_TARGET_PLATFORM, + NATIVE_TOOLCHAINS, PLATFORM_BK72XX, PLATFORM_ESP32, PLATFORM_ESP8266, @@ -992,6 +993,12 @@ class EsphomeCore: """ return self.toolchain == Toolchain.ARDUINO + @property + def using_native_toolchain(self): + """Whether the selected toolchain builds natively, without reading + ``platformio.ini`` (see ``NATIVE_TOOLCHAINS`` in ``esphome.const``).""" + return self.toolchain in NATIVE_TOOLCHAINS + @property def using_zephyr(self): return self.target_framework == "zephyr" diff --git a/esphome/core/config.py b/esphome/core/config.py index 350e4fd557..9be3d611b4 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -557,9 +557,7 @@ def _add_library_str(lib: str) -> None: @coroutine_with_priority(CoroPriority.FINAL) async def _add_platformio_options(pio_options: dict[str, str | list[str]]) -> None: - # Every platform's toolchain validation rejects values it cannot serve, - # so using_toolchain_arduino by itself implies the native ESP8266 build. - if CORE.using_toolchain_esp_idf or CORE.using_toolchain_arduino: + if CORE.using_native_toolchain: # The native builds don't read platformio.ini; honor the options # with a native equivalent and warn about the rest, which would # otherwise be silently ignored. From b90aeb17e306cbe13dc61dcdf3c4e7b3eb21edda Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 15:46:07 -0500 Subject: [PATCH 2/4] Hoist find_ninja to build_helpers, type the installed paths, dedupe espidf ccache env --- esphome/arduino8266/framework.py | 41 +++++---------- esphome/build_helpers/ninja.py | 33 ++++++++++++ esphome/espidf/framework.py | 28 +++-------- tests/unit_tests/build_helpers/test_ninja.py | 50 +++++++++++++++++++ .../unit_tests/test_arduino8266_framework.py | 50 ++----------------- tests/unit_tests/test_espidf_framework.py | 3 +- 6 files changed, 110 insertions(+), 95 deletions(-) create mode 100644 esphome/build_helpers/ninja.py create mode 100644 tests/unit_tests/build_helpers/test_ninja.py diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index 994179658c..475e61b6c0 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -22,8 +22,9 @@ import functools import logging import os from pathlib import Path -import shutil +from typing import NamedTuple +from esphome.build_helpers.ninja import find_ninja from esphome.core import EsphomeError, Version from esphome.framework_helpers import ( ccache_defaults_env, @@ -78,31 +79,15 @@ def get_toolchain_path() -> Path: return get_arduino8266_tools_path() / "toolchains" / TOOLCHAIN_VERSION -def _find_ninja() -> Path: - """Locate the ninja binary: PATH first, else the ninja PyPI wheel. +class InstalledPaths(NamedTuple): + """Locations of the installed framework, toolchain, and ninja binary.""" - The wheel is a requirements.txt dependency, so pip has already - integrity-checked it; no download logic is needed here. - """ - if binary := shutil.which("ninja"): - return Path(binary) - try: - import ninja - except ImportError: - wheel_binary = None - else: - wheel_binary = Path(ninja.BIN_DIR) / ( - "ninja.exe" if os.name == "nt" else "ninja" - ) - if wheel_binary is None or not wheel_binary.is_file(): - raise EsphomeError( - "ninja not found on PATH or in the ninja package; reinstall the " - "esphome Python environment" - ) - return wheel_binary + framework: Path + toolchain: Path + ninja: Path -def check_and_install(framework_version: Version) -> dict[str, Path]: +def check_and_install(framework_version: Version) -> InstalledPaths: """Ensure framework, toolchain, and ninja are installed; return their paths.""" if framework_version < MIN_FRAMEWORK_VERSION: # Config validation enforces this too; keep the module honest when @@ -112,7 +97,7 @@ def check_and_install(framework_version: Version) -> dict[str, Path]: f">= {MIN_FRAMEWORK_VERSION}, got {framework_version}" ) # Probe the cheap local dependency before ~110 MB of downloads - ninja_path = _find_ninja() + ninja_path = find_ninja() package_version = framework_package_version(framework_version) framework_path = get_framework_path(package_version) downloads_dir = get_arduino8266_tools_path() / "downloads" @@ -133,11 +118,9 @@ def check_and_install(framework_version: Version) -> dict[str, Path]: downloads_dir, expect=("bin",), ) - return { - "framework_path": framework_path, - "toolchain_path": toolchain_path, - "ninja_path": ninja_path, - } + return InstalledPaths( + framework=framework_path, toolchain=toolchain_path, ninja=ninja_path + ) def get_build_env(toolchain_path: Path) -> dict[str, str]: diff --git a/esphome/build_helpers/ninja.py b/esphome/build_helpers/ninja.py new file mode 100644 index 0000000000..98af05a031 --- /dev/null +++ b/esphome/build_helpers/ninja.py @@ -0,0 +1,33 @@ +"""Platform-neutral helpers for ninja-driven native builds.""" + +from __future__ import annotations + +import os +from pathlib import Path +import shutil + +from esphome.core import EsphomeError + + +def find_ninja() -> Path: + """Locate the ninja binary: PATH first, else the ninja PyPI wheel. + + The wheel is a requirements.txt dependency, so pip has already + integrity-checked it; no download logic is needed here. + """ + if binary := shutil.which("ninja"): + return Path(binary) + try: + import ninja + except ImportError: + wheel_binary = None + else: + wheel_binary = Path(ninja.BIN_DIR) / ( + "ninja.exe" if os.name == "nt" else "ninja" + ) + if wheel_binary is None or not wheel_binary.is_file(): + raise EsphomeError( + "ninja not found on PATH or in the ninja package; reinstall the " + "esphome Python environment" + ) + return wheel_binary diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 6c01058044..b4b03ce113 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -11,10 +11,11 @@ import re import shutil from typing import Any, NoReturn -from esphome.core import CORE, Version +from esphome.core import Version from esphome.framework_helpers import ( PathType, archive_extract_all, + ccache_defaults_env, create_venv, download_from_mirrors, download_with_resume, @@ -1156,25 +1157,12 @@ def _ccache_env() -> dict[str, str]: # ESP-IDF silently skips ccache without the binary; don't enable it. return {} - # ccache is enabled past here. build_path is set during preload for every - # config-loading command, so it being unset means a caller built the IDF env - # too early -- fail loudly rather than silently drop CCACHE_BASEDIR (which - # would quietly cost cross-device cache hits). - if CORE.build_path is None: - raise ValueError( - "CORE.build_path must be set before constructing the ESP-IDF build " - "environment" - ) - - defaults = { - "IDF_CCACHE_ENABLE": "1", - "CCACHE_DIR": str(get_idf_tools_path() / "ccache"), - "CCACHE_NOHASHDIR": "true", - "CCACHE_DEPEND": "1", - "CCACHE_BASEDIR": str(Path(CORE.build_path).resolve()), - } - # Don't override CCACHE_* values the user already set in their environment. - return {k: v for k, v in defaults.items() if k not in os.environ} + # ccache is enabled past here; the shared helper carries the CCACHE_* + # policy (and the fail-loud build_path guard). + env = ccache_defaults_env(get_idf_tools_path() / "ccache") + if "IDF_CCACHE_ENABLE" not in os.environ: + env["IDF_CCACHE_ENABLE"] = "1" + return env def get_framework_env( diff --git a/tests/unit_tests/build_helpers/test_ninja.py b/tests/unit_tests/build_helpers/test_ninja.py new file mode 100644 index 0000000000..cf51d1b8d8 --- /dev/null +++ b/tests/unit_tests/build_helpers/test_ninja.py @@ -0,0 +1,50 @@ +"""Tests for esphome.build_helpers.ninja.""" + +from __future__ import annotations + +import os +from pathlib import Path +import sys +from unittest.mock import MagicMock, patch + +import pytest + +from esphome.build_helpers import ninja as ninja_helper +from esphome.core import EsphomeError + + +def test_find_ninja_prefers_path(tmp_path: Path) -> None: + with patch("shutil.which", return_value=str(tmp_path / "ninja")): + assert ninja_helper.find_ninja() == tmp_path / "ninja" + + +def test_find_ninja_falls_back_to_wheel(tmp_path: Path) -> None: + """Without a PATH entry, the ninja PyPI wheel's binary is used.""" + binary_name = "ninja.exe" if os.name == "nt" else "ninja" + (tmp_path / binary_name).touch() + wheel = MagicMock(BIN_DIR=str(tmp_path)) + with ( + patch("shutil.which", return_value=None), + patch.dict(sys.modules, {"ninja": wheel}), + ): + assert ninja_helper.find_ninja() == tmp_path / binary_name + + +def test_find_ninja_package_not_installed() -> None: + """A missing ninja package raises the actionable message, not ImportError.""" + with ( + patch("shutil.which", return_value=None), + patch.dict(sys.modules, {"ninja": None}), + pytest.raises(EsphomeError, match="ninja not found"), + ): + ninja_helper.find_ninja() + + +def test_find_ninja_missing_everywhere(tmp_path: Path) -> None: + wheel = MagicMock(BIN_DIR=str(tmp_path)) + with ( + patch("shutil.which", return_value=None), + patch.dict(sys.modules, {"ninja": wheel}), + pytest.raises(EsphomeError, match="ninja not found"), + ): + ninja_helper.find_ninja() diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index a9d554cec0..fac85964ce 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -5,8 +5,7 @@ from __future__ import annotations import os from pathlib import Path import subprocess -import sys -from unittest.mock import MagicMock, patch +from unittest.mock import patch import pytest @@ -36,55 +35,16 @@ def test_tools_path_default_and_prefix(tmp_path: Path) -> None: assert path != Path.cwd() -def test_find_ninja_prefers_path(tmp_path: Path) -> None: - with patch("shutil.which", return_value=str(tmp_path / "ninja")): - assert framework._find_ninja() == tmp_path / "ninja" - - -def test_find_ninja_falls_back_to_wheel(tmp_path: Path) -> None: - """Without a PATH entry, the ninja PyPI wheel's binary is used.""" - binary_name = "ninja.exe" if os.name == "nt" else "ninja" - (tmp_path / binary_name).touch() - wheel = MagicMock(BIN_DIR=str(tmp_path)) - with ( - patch("shutil.which", return_value=None), - patch.dict(sys.modules, {"ninja": wheel}), - ): - assert framework._find_ninja() == tmp_path / binary_name - - -def test_find_ninja_package_not_installed() -> None: - """A missing ninja package raises the actionable message, not ImportError.""" - with ( - patch("shutil.which", return_value=None), - patch.dict(sys.modules, {"ninja": None}), - pytest.raises(EsphomeError, match="ninja not found"), - ): - framework._find_ninja() - - -def test_find_ninja_missing_everywhere(tmp_path: Path) -> None: - wheel = MagicMock(BIN_DIR=str(tmp_path)) - with ( - patch("shutil.which", return_value=None), - patch.dict(sys.modules, {"ninja": wheel}), - pytest.raises(EsphomeError, match="ninja not found"), - ): - framework._find_ninja() - - def test_check_and_install_returns_paths(tmp_path: Path) -> None: with ( patch.dict(os.environ, {"ESPHOME_ARDUINO8266_PREFIX": str(tmp_path)}), patch.object(framework, "install_package") as mock_install, - patch.object(framework, "_find_ninja", return_value=tmp_path / "ninja"), + patch.object(framework, "find_ninja", return_value=tmp_path / "ninja"), ): paths = framework.check_and_install(cv.Version(3, 1, 2)) - assert paths["framework_path"] == tmp_path / "frameworks" / "3.30102.0" - assert ( - paths["toolchain_path"] == tmp_path / "toolchains" / framework.TOOLCHAIN_VERSION - ) - assert paths["ninja_path"] == tmp_path / "ninja" + assert paths.framework == tmp_path / "frameworks" / "3.30102.0" + assert paths.toolchain == tmp_path / "toolchains" / framework.TOOLCHAIN_VERSION + assert paths.ninja == tmp_path / "ninja" assert mock_install.call_count == 2 # The layout checks cover the directories write_project needs, including # the bundled libraries/ tree diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index d8e7738569..4c9e67a1c6 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -1400,8 +1400,9 @@ def _ccache_patches(tmp_path: Path, which: str | None, build_path: Path | None): "esphome.espidf.framework.get_idf_tools_path", return_value=tmp_path / "tools", ), + # ccache_defaults_env (framework_helpers) reads CORE at call time patch( - "esphome.espidf.framework.CORE", + "esphome.core.CORE", SimpleNamespace(build_path=build_path), ), ) From d4e3603a19ac40571aed029a521e2be8b4c6f2ec Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 15:46:39 -0500 Subject: [PATCH 3/4] Drop the dead name injection and pass board_mcu lazily --- esphome/arduino/library.py | 6 ++++-- tests/unit_tests/test_arduino_library.py | 8 +++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index d673b5fd73..75ce906b77 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -142,7 +142,7 @@ def _bundled_library(framework_path: Path, name: str) -> ArduinoLibrary: else: manifest = lib_dir / "library.properties" data = parse_library_properties(manifest) if manifest.is_file() else {} - return _library_info(name, lib_dir, {"name": name, **data}) + return _library_info(name, lib_dir, data) def resolve_libraries( @@ -219,7 +219,9 @@ def resolve_libraries( bundled.append(_bundled_library(framework_path, name)) def _emit(component: ConvertedLibrary) -> None: - apply_extra_script(component, board_mcu=board_mcu, pio_platform=pio_platform) + apply_extra_script( + component, board_mcu=lambda: board_mcu, pio_platform=pio_platform + ) converted.append( _library_info( component.get_require_name(), component.source_dir, component.data diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 912038a20c..f8c0994c0c 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -214,9 +214,11 @@ def test_resolve_libraries_external_and_bundled_deps(tmp_path: Path) -> None: cache_key="arduino8266", ) - mock_extra.assert_called_once_with( - converted, board_mcu="esp8266", pio_platform="espressif8266" - ) + mock_extra.assert_called_once() + assert mock_extra.call_args.args == (converted,) + assert mock_extra.call_args.kwargs["pio_platform"] == "espressif8266" + # board_mcu is passed lazily, as the shared helper requires + assert mock_extra.call_args.kwargs["board_mcu"]() == "esp8266" assert [lib.name for lib in libs] == [ "Wire", "esp32async__ESPAsyncWebServer", From aa4dd3d0a10f61149f7b3fd8dc0aab2513b4ab8c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 15:48:56 -0500 Subject: [PATCH 4/4] Hoist the ninja quoting helpers, type the installed paths, format flash size from board data --- esphome/build_gen/arduino8266.py | 78 +++++-------------- esphome/build_helpers/ninja.py | 43 ++++++++++ .../unit_tests/build_gen/test_arduino8266.py | 26 +++---- 3 files changed, 74 insertions(+), 73 deletions(-) diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 432647046b..1d72971434 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -18,10 +18,15 @@ from dataclasses import dataclass, field import logging import os from pathlib import Path -import re import subprocess import sys +from typing import TYPE_CHECKING +from esphome.build_helpers.ninja import ( + escape as _e, + quote_path as _q, + shell_token as _shell_token, +) from esphome.components.esp8266 import build_surgery from esphome.components.esp8266.boards import ( BOARDS, @@ -45,6 +50,9 @@ from esphome.platformio.library import ( split_flag_entry, ) +if TYPE_CHECKING: + from esphome.arduino8266.framework import InstalledPaths + _LOGGER = logging.getLogger(__name__) # Compile rule per source suffix, derived from the shared suffix -> kind map @@ -285,48 +293,6 @@ def _flash_ld_name(board: str) -> str: return ESP8266_LD_SCRIPTS[BOARDS[board][KEY_FLASH_SIZE]][1] -def _e(value) -> str: - """Escape a path or token for a ninja file.""" - return str(value).replace("$", "$$").replace(":", "$:").replace(" ", "$ ") - - -def _quote_arg(tok: str) -> str: - """Wrap a token in double quotes with the Windows argv rule. - - Same escaping rule as ``subprocess.list2cmdline``: a backslash run - doubles only immediately before a quote (or the closing quote), and the - quote itself is escaped. POSIX sh parses the result identically for - backslashes and quotes. ``$`` must already be doubled for ninja. - """ - quoted = re.sub(r'(\\*)"', lambda m: m.group(1) * 2 + '\\"', tok) - quoted = re.sub(r"(\\+)\Z", lambda m: m.group(1) * 2, quoted) - return f'"{quoted}"' - - -_NEEDS_QUOTE = re.compile(r'[\s"\']') - - -def _shell_token(tok: str, force: bool = False) -> str: - """Quote a lexed token only when needed; ``force`` always quotes. - - Lexing strips the quoting a user wrote (``-DX="a b"`` becomes the single - token ``-DX=a b``); re-quote on the way out so the compiler receives the - same argv element SCons would pass under PlatformIO. After ninja - un-doubles ``$$``, sh still expands ``$VAR`` while CreateProcess passes - it literally -- the same divergence SCons-under-sh has, so this stays - PlatformIO parity. - """ - tok = tok.replace("$", "$$") # ninja would expand a bare $ to nothing - if force or _NEEDS_QUOTE.search(tok): - return _quote_arg(tok) - return tok - - -def _q(value) -> str: - """Force-quote a path for the ninja command line (shell/CreateProcess).""" - return _shell_token(str(value), force=True) - - def _defines_flags( config: _BuildConfig, flash_mode: str, board: str, board_defines: tuple[str, ...] ) -> list[str]: @@ -399,7 +365,7 @@ def _collect_sources(root: Path, exclude: set[str] = frozenset()) -> list[Path]: def generate_ld_scripts( - paths: dict[str, Path], config: _BuildConfig, flash_ld_name: str + paths: InstalledPaths, config: _BuildConfig, flash_ld_name: str ) -> None: """Generate the common linker script (and testing-mode flash ld copy). @@ -407,8 +373,8 @@ def generate_ld_scripts( ``eagle.app.v6.common.ld.h``, then applies ESPHome's surgeries: the wifi rate-table DRAM relocation, and enlarged memory segments in testing mode. """ - framework = paths["framework_path"] - gcc = paths["toolchain_path"] / "bin" / "xtensa-lx106-elf-gcc" + framework = paths.framework + gcc = paths.toolchain / "bin" / "xtensa-lx106-elf-gcc" ld_dir = CORE.relative_pioenvs_path(CORE.name, "ld") mkdir_p(ld_dir) @@ -493,7 +459,7 @@ def _common_parent(paths: list[Path]) -> Path: return Path(os.path.commonpath([str(p.parent) for p in paths])) -def write_project(paths: dict[str, Path]) -> bool: +def write_project(paths: InstalledPaths) -> bool: """Write the ninja build for the current configuration. Returns True when ``build.ninja`` changed, so the caller can skip work @@ -502,8 +468,8 @@ def write_project(paths: dict[str, Path]) -> bool: from esphome.arduino.library import resolve_libraries from esphome.arduino8266.framework import ccache_path - framework = paths["framework_path"] - toolchain_bin = paths["toolchain_path"] / "bin" + framework = paths.framework + toolchain_bin = paths.toolchain / "bin" build_dir = CORE.relative_pioenvs_path(CORE.name) mkdir_p(build_dir) @@ -535,7 +501,7 @@ def write_project(paths: dict[str, Path]) -> bool: src_dir, sdk / "include", core_dir, - paths["toolchain_path"] / "include", + paths.toolchain / "include", sdk / "lwip2" / "include", variant_dir, ] @@ -639,7 +605,7 @@ def write_project(paths: dict[str, Path]) -> bool: " rspfile_content = $in_newline", " description = LINK $out", "rule elf2bin", - f" command = $python {_q(framework / 'tools' / 'elf2bin.py')} --eboot {_q(framework / 'bootloaders' / 'eboot' / 'eboot.elf')} --app $in --flash_mode {esp8266_data[KEY_FLASH_MODE]} --flash_freq 40 --flash_size {_flash_size_str(flash_ld_name)} --path {_q(toolchain_bin)} --out $out", + f" command = $python {_q(framework / 'tools' / 'elf2bin.py')} --eboot {_q(framework / 'bootloaders' / 'eboot' / 'eboot.elf')} --app $in --flash_mode {esp8266_data[KEY_FLASH_MODE]} --flash_freq 40 --flash_size {_flash_size_str(BOARDS[board][KEY_FLASH_SIZE])} --path {_q(toolchain_bin)} --out $out", " description = BIN $out", "rule copy", " command = $python $buildtool copy $in $out", @@ -737,9 +703,7 @@ def get_flash_ld_path(build_dir: Path) -> Path: return get_framework_path(version) / "tools" / "sdk" / "ld" / name -def _flash_size_str(flash_ld_name: str) -> str: - """Flash size for elf2bin, derived from the ld script name (PIO logic).""" - match = re.search(r"\.flash\.(\d+)([mk])", flash_ld_name) - if not match: - raise EsphomeError(f"Cannot parse flash size from {flash_ld_name}") - return f"{match.group(1)}{match.group(2).upper()}" +def _flash_size_str(flash_size: int) -> str: + """Flash size argument for elf2bin (e.g. ``4M``, ``512K``).""" + mb = 1024 * 1024 + return f"{flash_size // mb}M" if flash_size >= mb else f"{flash_size // 1024}K" diff --git a/esphome/build_helpers/ninja.py b/esphome/build_helpers/ninja.py index 98af05a031..9a9189e8ee 100644 --- a/esphome/build_helpers/ninja.py +++ b/esphome/build_helpers/ninja.py @@ -4,6 +4,7 @@ from __future__ import annotations import os from pathlib import Path +import re import shutil from esphome.core import EsphomeError @@ -31,3 +32,45 @@ def find_ninja() -> Path: "esphome Python environment" ) return wheel_binary + + +def escape(value) -> str: + """Escape a path or token for a ninja file.""" + return str(value).replace("$", "$$").replace(":", "$:").replace(" ", "$ ") + + +def quote_arg(tok: str) -> str: + """Wrap a token in double quotes with the Windows argv rule. + + Same escaping rule as ``subprocess.list2cmdline``: a backslash run + doubles only immediately before a quote (or the closing quote), and the + quote itself is escaped. POSIX sh parses the result identically for + backslashes and quotes. ``$`` must already be doubled for ninja. + """ + quoted = re.sub(r'(\\*)"', lambda m: m.group(1) * 2 + '\\"', tok) + quoted = re.sub(r"(\\+)\Z", lambda m: m.group(1) * 2, quoted) + return f'"{quoted}"' + + +_NEEDS_QUOTE = re.compile(r'[\s"\']') + + +def shell_token(tok: str, force: bool = False) -> str: + """Quote a lexed token only when needed; ``force`` always quotes. + + Lexing strips the quoting a user wrote (``-DX="a b"`` becomes the single + token ``-DX=a b``); re-quote on the way out so the compiler receives the + same argv element SCons would pass under PlatformIO. After ninja + un-doubles ``$$``, sh still expands ``$VAR`` while CreateProcess passes + it literally -- the same divergence SCons-under-sh has, so this stays + PlatformIO parity. + """ + tok = tok.replace("$", "$$") # ninja would expand a bare $ to nothing + if force or _NEEDS_QUOTE.search(tok): + return quote_arg(tok) + return tok + + +def quote_path(value) -> str: + """Force-quote a path for the ninja command line (shell/CreateProcess).""" + return shell_token(str(value), force=True) diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index 69f84896ca..a3e99a8159 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -17,6 +17,7 @@ from unittest.mock import MagicMock, patch import pytest +from esphome.arduino8266.framework import InstalledPaths from esphome.build_gen import arduino8266 from esphome.build_gen.arduino8266 import ( _defines_flags, @@ -181,15 +182,11 @@ def _make_framework(tmp_path: Path) -> dict[str, Path]: toolchain = tmp_path / "toolchain" (toolchain / "bin").mkdir(parents=True) (toolchain / "include").mkdir() - return { - "framework_path": framework, - "toolchain_path": toolchain, - "ninja_path": Path("ninja"), - } + return InstalledPaths(framework=framework, toolchain=toolchain, ninja=Path("ninja")) def _write_ninja( - paths: dict[str, Path], + paths: InstalledPaths, libraries: list | None = None, ccache: str | None = None, ) -> str: @@ -390,7 +387,7 @@ SECTIONS """ -def _run_generate_ld_scripts(paths: dict[str, Path]) -> Path: +def _run_generate_ld_scripts(paths: InstalledPaths) -> Path: config = _resolve_build_config(_flag_defines()) arduino8266.generate_ld_scripts(paths, config, "eagle.flash.4m.ld") @@ -442,7 +439,7 @@ def test_generate_ld_scripts_failure(tmp_path: Path) -> None: def test_generate_ld_scripts_testing_mode(tmp_path: Path) -> None: paths = _make_framework(tmp_path) - (paths["framework_path"] / "tools" / "sdk" / "ld" / "eagle.flash.4m.ld").write_text( + (paths.framework / "tools" / "sdk" / "ld" / "eagle.flash.4m.ld").write_text( "MEMORY\n{\n" " dram0_0_seg : org = 0x3FFE8000, len = 0x14000\n" " iram1_0_seg : org = 0x40100000, len = 0x8000\n" @@ -463,7 +460,7 @@ def test_write_project_libraries_and_variant( from esphome.arduino.library import ArduinoLibrary paths = _make_framework(tmp_path) - variant_src = paths["framework_path"] / "variants" / "nodemcu" / "variant.cpp" + variant_src = paths.framework / "variants" / "nodemcu" / "variant.cpp" variant_src.write_text("") lib_dir = tmp_path / "libsrc" @@ -529,11 +526,8 @@ def test_get_flash_ld_path(tmp_path: Path) -> None: def test_flash_size_str() -> None: - - assert _flash_size_str("eagle.flash.4m.ld") == "4M" - assert _flash_size_str("eagle.flash.512k.ld") == "512K" - with pytest.raises(EsphomeError, match="Cannot parse flash size"): - _flash_size_str("bogus.ld") + assert _flash_size_str(4 * 1024 * 1024) == "4M" + assert _flash_size_str(512 * 1024) == "512K" def test_write_project_testing_mode(tmp_path: Path) -> None: @@ -550,7 +544,7 @@ def test_write_project_missing_framework_dir_raises(tmp_path: Path) -> None: import shutil paths = _make_framework(tmp_path) - shutil.rmtree(paths["framework_path"] / "tools" / "sdk" / "lwip2") + shutil.rmtree(paths.framework / "tools" / "sdk" / "lwip2") _set_flags() with pytest.raises(EsphomeError, match="incomplete.*lwip2"): _write_ninja(paths) @@ -663,7 +657,7 @@ def test_shell_token_escaping() -> None: def test_write_project_empty_core_raises(tmp_path: Path) -> None: """A framework tree with no core sources fails at generation, not link.""" paths = _make_framework(tmp_path) - core = paths["framework_path"] / "cores" / "esp8266" + core = paths.framework / "cores" / "esp8266" for f in core.iterdir(): f.unlink() _set_flags()