From 2564ad0deb2e7467195721e901de907968de6367 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 13:57:30 -0500 Subject: [PATCH 1/3] Warn once per non-compiler path, accept versioned compiler names, and pin the trigger table --- esphome/build_helpers/idedata.py | 35 ++++++++++++------- tests/script/test_determine_jobs.py | 8 ++++- .../unit_tests/build_helpers/test_idedata.py | 31 ++++++++++------ 3 files changed, 50 insertions(+), 24 deletions(-) diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 5bf2a4b5ec..86391e688b 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -1,16 +1,17 @@ -"""Derive idedata from an ESP-IDF native-toolchain ``compile_commands.json``. +"""Derive idedata from a native (non-PlatformIO) build's ``compile_commands.json``. -PlatformIO exposes a curated ``pio run -t idedata`` JSON; the native ESP-IDF -toolchain has no such command, but its CMake build emits -``build/compile_commands.json`` (CMAKE_EXPORT_COMPILE_COMMANDS). This module -turns that file into the same fields consumers (IDE integration, clang-tidy) -expect: +PlatformIO exposes a curated ``pio run -t idedata`` JSON; the native +toolchains have no such command, but each build produces a +``compile_commands.json`` (CMAKE_EXPORT_COMPILE_COMMANDS for ESP-IDF, ninja's +compdb tool otherwise). This module turns that file into the same fields +consumers (IDE integration, clang-tidy) expect: {cc_path, cxx_path, cxx_flags, defines, includes: {build, toolchain}} """ from __future__ import annotations +import functools import json import logging import os @@ -123,7 +124,20 @@ def _pick_entry(entries: list[dict]) -> dict: # The compiler basename a compile_commands entry must lead with (an # optional target-triple prefix ends in one of these) -_COMPILER_STEM = re.compile(r"(?:gcc|g\+\+|cc|c\+\+|clang|clang\+\+)$") +# The stem must BE a compiler name (optionally versioned), alone or after a +# target-triple separator: "cc", "xtensa-lx106-elf-g++", "gcc-8.4.0" match; +# "ccache" and "distcc" do not. +_COMPILER_STEM = re.compile( + r"(?:^|[-_.])(?:gcc|g\+\+|cc|c\+\+|clang|clang\+\+)(?:-[\d.]+)?$" +) + + +@functools.cache +def _warn_not_a_compiler(token: str) -> None: + # A stale compile DB built with a launcher the current run no longer + # configures would otherwise cache the launcher as the compiler path. + # Cached so a database of hundreds of entries warns once per path. + _LOGGER.warning("compile_commands entry does not start with a compiler: %s", token) def parse_entry( @@ -150,12 +164,7 @@ def parse_entry( if launcher is not None and tokens[0] == launcher: tokens = tokens[1:] if not _COMPILER_STEM.search(Path(tokens[0]).stem): - # A stale compile DB built with a launcher the current run no longer - # configures would otherwise cache the launcher as the compiler path - _LOGGER.warning( - "compile_commands entry does not start with a compiler: %s", - tokens[0], - ) + _warn_not_a_compiler(tokens[0]) # token0 is the compiler path; the rest of the command already uses forward # slashes on Windows, so normalize it too for a consistent idedata file. cxx_path = tokens[0].replace("\\", "/") diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 80f572d9fe..297752b3ac 100644 --- a/tests/script/test_determine_jobs.py +++ b/tests/script/test_determine_jobs.py @@ -1120,7 +1120,13 @@ def test_should_run_esp32_platformio_with_branch() -> None: (["esphome/espidf/runner.py"], True), (["esphome/espidf/framework.py"], True), (["esphome/build_gen/espidf.py"], True), - # PlatformIO build gen and esp32 component are NOT IDF-infra triggers + # Shared native-build modules the IDF build imports -> trigger + (["esphome/build_helpers/idedata.py"], True), + (["esphome/platformio/library.py"], True), + (["esphome/platformio/extra_script.py"], True), + # PlatformIO build gen, its toolchain, and the esp32 component are + # NOT IDF-infra triggers + (["esphome/platformio/toolchain.py"], False), (["esphome/build_gen/platformio.py"], False), (["esphome/components/esp32/__init__.py"], False), (["README.md"], False), diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index eca74ded35..e65802dd74 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -21,7 +21,7 @@ def _entry(directory: str, file: str, command: str) -> dict: return {"directory": directory, "file": file, "command": command} -def testparse_entry_extracts_fields() -> None: +def test_parse_entry_extracts_fields() -> None: """cxx_path, defines, includes and remaining flags are split apart.""" entry = _entry( f"{ABS}build", @@ -45,7 +45,7 @@ def testparse_entry_extracts_fields() -> None: assert "app.cpp.o" not in cxx_flags -def testparse_entry_space_separated_args() -> None: +def test_parse_entry_space_separated_args() -> None: """``-D X`` / ``-I path`` (separate arg) and ``-isystem`` (joined).""" entry = _entry( f"{ABS}build", @@ -60,7 +60,7 @@ def testparse_entry_space_separated_args() -> None: assert f"{ABS}sys/joined" in includes -def testparse_entry_resolves_relative_includes() -> None: +def test_parse_entry_resolves_relative_includes() -> None: """Relative includes are resolved against the entry's ``directory``.""" directory = f"{ABS}build/proj" entry = _entry( @@ -83,7 +83,7 @@ def testparse_entry_resolves_relative_includes() -> None: assert all(Path(inc).is_absolute() for inc in includes) -def testparse_entry_skips_dependency_flags() -> None: +def test_parse_entry_skips_dependency_flags() -> None: """Dependency-generation flags (and their args) are dropped.""" entry = _entry( "/build", @@ -198,7 +198,7 @@ def test_idedata_from_build(tmp_path: Path) -> None: assert data["includes"]["toolchain"] == ["/tc/inc/c++", "/tc/inc"] -def testget_toolchain_includes_raises_on_probe_failure() -> None: +def test_get_toolchain_includes_raises_on_probe_failure() -> None: """A failed compiler probe is a hard error, not a silent empty list.""" fake_proc = MagicMock(returncode=1, stderr="xtensa-esp32-elf-g++: not found") with ( @@ -208,7 +208,7 @@ def testget_toolchain_includes_raises_on_probe_failure() -> None: idedata.get_toolchain_includes("/bad/compiler") -def testget_toolchain_includes_raises_when_no_dirs_found() -> None: +def test_get_toolchain_includes_raises_when_no_dirs_found() -> None: """Markers present but no dirs (anomalous output) also raises.""" fake_proc = MagicMock( returncode=0, @@ -248,7 +248,7 @@ def test_split_command_empty_returns_empty() -> None: @pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization") -def testparse_entry_normalizes_windows_cxx_path() -> None: +def test_parse_entry_normalizes_windows_cxx_path() -> None: """A backslash compiler path is emitted forward-slashed; define unescaped.""" entry = _entry( r"C:\b", @@ -264,7 +264,7 @@ def testparse_entry_normalizes_windows_cxx_path() -> None: assert "C:/inc/a" in includes -def testparse_entry_strips_launcher_prefix() -> None: +def test_parse_entry_strips_launcher_prefix() -> None: """A launcher-wrapped compile names the compiler second; the exact configured launcher is stripped, not anything ccache-shaped.""" entry = _entry( @@ -279,14 +279,17 @@ def testparse_entry_strips_launcher_prefix() -> None: assert cxx_path == "/tools/xtensa-lx106-elf-g++" assert defines == ["USE_ESP8266"] # Without a configured launcher nothing is stripped, even a token that - # happens to be named ccache -- but the surprise is warned about + # happens to be named ccache -- but the surprise is warned about (once + # per path, however many entries the compile DB has) + idedata._warn_not_a_compiler.cache_clear() cxx_path, _, _, _ = idedata.parse_entry(entry) assert cxx_path == "/opt/homebrew/bin/ccache" -def testparse_entry_warns_when_first_token_is_not_a_compiler( +def test_parse_entry_warns_when_first_token_is_not_a_compiler( caplog: pytest.LogCaptureFixture, ) -> None: + idedata._warn_not_a_compiler.cache_clear() entry = _entry( f"{ABS}build", f"{ABS}build/src/esphome/core/application.cpp", @@ -391,3 +394,11 @@ def test_load_or_build_idedata_rebuilds_non_dict_cache(tmp_path: Path) -> None: ) assert isinstance(data, dict) assert "cc_path" in data + + +def test_parse_entry_accepts_versioned_compilers() -> None: + """Versioned compiler names (g++-13, gcc-8.4.0) are not warned about.""" + for stem in ("g++-13", "gcc-8.4.0", "clang++-17"): + assert idedata._COMPILER_STEM.search(stem) + assert not idedata._COMPILER_STEM.search("ccache") + assert not idedata._COMPILER_STEM.search("distcc") From bbf278868498fa0ca1b7f320deb37a1c91e88cde Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 13:58:46 -0500 Subject: [PATCH 2/3] Share one unsupported-toolchain check across platforms --- esphome/components/esp32/__init__.py | 6 +----- esphome/components/host/__init__.py | 2 +- esphome/components/nrf52/__init__.py | 6 +----- esphome/components/rp2/__init__.py | 2 +- esphome/config_validation.py | 24 +++++++++++++++++------- esphome/core/config.py | 6 +++--- 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index a8a5abab25..201c69a8c0 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -1083,11 +1083,7 @@ def _resolve_toolchain(value: ConfigType) -> ConfigType: # 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) - if CORE.toolchain not in (Toolchain.PLATFORMIO, Toolchain.ESP_IDF): - raise cv.Invalid( - f"Unsupported toolchain '{CORE.toolchain.value}' for ESP32. " - "Supported toolchains are 'platformio' and 'esp-idf'." - ) + cv.check_supported_toolchain("ESP32", (Toolchain.PLATFORMIO, Toolchain.ESP_IDF)) return value diff --git a/esphome/components/host/__init__.py b/esphome/components/host/__init__.py index 6f0fcb9d52..1717870238 100644 --- a/esphome/components/host/__init__.py +++ b/esphome/components/host/__init__.py @@ -36,8 +36,8 @@ CONFIG_SCHEMA = cv.All( cv.Optional(CONF_MAC_ADDRESS, default="98:35:69:ab:f6:79"): cv.mac_address, } ), - set_core_data, cv.require_platformio_toolchain("host"), + set_core_data, ) diff --git a/esphome/components/nrf52/__init__.py b/esphome/components/nrf52/__init__.py index 2328d444f7..fe0c1a12de 100644 --- a/esphome/components/nrf52/__init__.py +++ b/esphome/components/nrf52/__init__.py @@ -128,11 +128,7 @@ def set_core_data(config: ConfigType) -> ConfigType: def _resolve_toolchain(config: ConfigType) -> ConfigType: if CORE.toolchain is None: CORE.toolchain = config.get(CONF_TOOLCHAIN, Toolchain.SDK_NRF) - if CORE.toolchain not in (Toolchain.PLATFORMIO, Toolchain.SDK_NRF): - raise cv.Invalid( - f"Unsupported toolchain '{CORE.toolchain.value}' for nRF52. " - "Supported toolchains are 'platformio' and 'sdk-nrf'." - ) + cv.check_supported_toolchain("nRF52", (Toolchain.PLATFORMIO, Toolchain.SDK_NRF)) return config diff --git a/esphome/components/rp2/__init__.py b/esphome/components/rp2/__init__.py index 14806b26ce..dae7df26c3 100644 --- a/esphome/components/rp2/__init__.py +++ b/esphome/components/rp2/__init__.py @@ -312,8 +312,8 @@ CONFIG_SCHEMA = cv.All( ), cv.has_at_least_one_key(CONF_BOARD, CONF_VARIANT), _detect_variant, - set_core_data, cv.require_platformio_toolchain("RP2"), + set_core_data, ) diff --git a/esphome/config_validation.py b/esphome/config_validation.py index c3ee760761..009b844986 100644 --- a/esphome/config_validation.py +++ b/esphome/config_validation.py @@ -75,6 +75,7 @@ from esphome.const import ( TYPE_GIT, TYPE_LOCAL, Framework, + Toolchain, __version__ as ESPHOME_VERSION, ) from esphome.core import ( @@ -2532,6 +2533,21 @@ def platformio_version_constraint(value): return constraints +def check_supported_toolchain(platform_name: str, supported: tuple) -> None: + """Raise when the resolved ``CORE.toolchain`` is not in ``supported``. + + One message shape for every platform, so a ``--toolchain`` a platform + cannot serve always fails by name instead of silently building with a + different backend. + """ + if CORE.toolchain not in supported: + names = ", ".join(f"'{tc.value}'" for tc in supported) + raise Invalid( + f"Unsupported toolchain '{CORE.toolchain.value}' for " + f"{platform_name}. Supported: {names}." + ) + + def require_platformio_toolchain(platform_name: str): """Reject a CLI-selected toolchain other than PlatformIO. @@ -2540,15 +2556,9 @@ def require_platformio_toolchain(platform_name: str): """ def validator(config): - from esphome.const import Toolchain - if CORE.toolchain is None: CORE.toolchain = Toolchain.PLATFORMIO - if CORE.toolchain != Toolchain.PLATFORMIO: - raise Invalid( - f"Unsupported toolchain '{CORE.toolchain.value}' for " - f"{platform_name}. The only supported toolchain is 'platformio'." - ) + check_supported_toolchain(platform_name, (Toolchain.PLATFORMIO,)) return config return validator diff --git a/esphome/core/config.py b/esphome/core/config.py index be577cccbb..9fef173b48 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -557,9 +557,9 @@ 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: - if CORE.using_toolchain_esp_idf or ( - CORE.using_toolchain_arduino and CORE.is_esp8266 - ): + # 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: # 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 0fe9d871247b66bdc4279cca2c7ba8af6ca05181 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 14:04:38 -0500 Subject: [PATCH 3/3] Share the ccache and tools-path helpers across the native backends --- esphome/arduino8266/framework.py | 87 ++++-------- esphome/components/esp8266/__init__.py | 6 +- esphome/components/nrf52/framework.py | 16 +-- esphome/espidf/framework.py | 25 +--- esphome/framework_helpers.py | 132 ++++++++++++++++++ esphome/platformio/toolchain.py | 84 +---------- .../unit_tests/test_arduino8266_framework.py | 14 +- tests/unit_tests/test_platformio_toolchain.py | 2 +- 8 files changed, 189 insertions(+), 177 deletions(-) diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index 066667806c..8d828dc936 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -25,19 +25,17 @@ from pathlib import Path import platform import shutil -import platformdirs - -import esphome.config_validation as cv -from esphome.core import CORE, EsphomeError +from esphome.core import EsphomeError, Version from esphome.framework_helpers import ( archive_extract_all, + ccache_defaults_env, download_from_mirrors, download_with_resume, + resolve_ccache_path, rmdir, str_to_lst_of_str, + tools_cache_path, ) -from esphome.helpers import get_bool_env, get_str_env -from esphome.platformio.library import ensure_list _LOGGER = logging.getLogger(__name__) @@ -61,25 +59,16 @@ ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS = str_to_lst_of_str( def get_arduino8266_tools_path() -> Path: - # Treat an empty/whitespace prefix as unset: Path("") resolves to the CWD, - # which clean-all would then delete. - if prefix := get_str_env("ESPHOME_ARDUINO8266_PREFIX", "").strip(): - path = Path(prefix).expanduser() - else: - # Machine-global so all projects share one install; see - # espidf.framework.get_idf_tools_path for the location rationale. - path = ( - Path(platformdirs.user_cache_dir("esphome", appauthor=False)) - / "arduino8266" - ) - return path.resolve() + # Machine-global so all projects share one install; see + # espidf.framework.get_idf_tools_path for the location rationale. + return tools_cache_path("ESPHOME_ARDUINO8266_PREFIX", "arduino8266") # 3.1.1 rather than 3.1.0: the registry has no package for 3.1.0 -MIN_FRAMEWORK_VERSION = cv.Version(3, 1, 1) +MIN_FRAMEWORK_VERSION = Version(3, 1, 1) -def framework_package_version(ver: cv.Version) -> str: +def framework_package_version(ver: Version) -> str: """Map an Arduino core version (e.g. 3.1.2) to its package version. Same encoding as the PlatformIO package registry uses for core 3.x @@ -158,8 +147,10 @@ def _registry_download(package: str, version: str) -> tuple[str, str, int | None if ver.get("name") != version: continue for file in ver.get("files", []): - # ensure_list: a bare string would make ``in`` a substring test - systems = ensure_list(file.get("system") or "*") + # A bare string would make ``in`` a substring test + systems = file.get("system") or "*" + if isinstance(systems, str): + systems = [systems] if "*" in systems or system in systems: sha256 = (file.get("checksum") or {}).get("sha256") if not sha256: @@ -241,22 +232,21 @@ def _find_ninja() -> Path: return Path(binary) try: import ninja - except ImportError as err: - raise EsphomeError( - "ninja not found on PATH or in the ninja package; reinstall the " - "esphome Python environment" - ) from err - - binary = Path(ninja.BIN_DIR) / ("ninja.exe" if os.name == "nt" else "ninja") - if not binary.is_file(): + 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 binary + return wheel_binary -def check_and_install(framework_version: cv.Version) -> dict[str, Path]: +def check_and_install(framework_version: Version) -> dict[str, Path]: """Ensure framework, toolchain, and ninja are installed; return their paths.""" package_version = framework_package_version(framework_version) framework_path = get_framework_path(package_version) @@ -291,29 +281,8 @@ def get_build_env(toolchain_path: Path) -> dict[str, str]: @functools.cache def ccache_path() -> str | None: - """The ccache binary to prefix compiles with, or None when disabled. - - Same convention as the PlatformIO path: on by default when the binary is - on PATH, ``ESPHOME_CCACHE_ENABLE=0`` disables it, and an explicit ``=1`` - warns when no binary is found and skips the runnability probe. - """ - from esphome.platformio.toolchain import _ccache_runs, _strip_win_long_path_prefix - - explicit = "ESPHOME_CCACHE_ENABLE" in os.environ - if explicit and not get_bool_env("ESPHOME_CCACHE_ENABLE"): - return None - ccache = shutil.which("ccache") - if ccache is None: - if explicit: - _LOGGER.warning( - "ESPHOME_CCACHE_ENABLE is set but no ccache binary is on PATH; " - "compiling without ccache" - ) - return None - ccache = _strip_win_long_path_prefix(ccache) - if not explicit and not _ccache_runs(ccache): - return None - return ccache + """The ccache binary to prefix compiles with, or None when disabled.""" + return resolve_ccache_path() def ccache_env() -> dict[str, str]: @@ -326,10 +295,4 @@ def ccache_env() -> dict[str, str]: """ if ccache_path() is None: return {} - defaults = { - "CCACHE_DIR": str(get_arduino8266_tools_path() / "ccache"), - "CCACHE_NOHASHDIR": "true", - "CCACHE_DEPEND": "1", - "CCACHE_BASEDIR": str(Path(CORE.build_path).resolve()), - } - return {k: v for k, v in defaults.items() if k not in os.environ} + return ccache_defaults_env(get_arduino8266_tools_path() / "ccache") diff --git a/esphome/components/esp8266/__init__.py b/esphome/components/esp8266/__init__.py index abc2f5dc91..4084893e7c 100644 --- a/esphome/components/esp8266/__init__.py +++ b/esphome/components/esp8266/__init__.py @@ -134,7 +134,11 @@ def _format_framework_arduino_version(ver: cv.Version) -> str: return f"~1.{ver.major}{ver.minor:02d}{ver.patch:02d}.0" if ver <= cv.Version(2, 6, 2): return f"~2.{ver.major}{ver.minor:02d}{ver.patch:02d}.0" - return f"~3.{ver.major}{ver.minor:02d}{ver.patch:02d}.0" + # Same encoding the native toolchain uses for its package download, so a + # version bump cannot drift between the two paths. + from esphome.arduino8266.framework import framework_package_version + + return f"~{framework_package_version(ver)}" # NOTE: Keep this in mind when updating the recommended version: diff --git a/esphome/components/nrf52/framework.py b/esphome/components/nrf52/framework.py index d487820440..48963fa89f 100644 --- a/esphome/components/nrf52/framework.py +++ b/esphome/components/nrf52/framework.py @@ -7,8 +7,6 @@ import shutil import sys import tempfile -import platformdirs - import esphome.config_validation as cv from esphome.const import KEY_CORE, KEY_FRAMEWORK_VERSION from esphome.core import CORE, EsphomeError @@ -20,8 +18,8 @@ from esphome.framework_helpers import ( rmdir, run_command_ok, str_to_lst_of_str, + tools_cache_path, ) -from esphome.helpers import get_str_env _LOGGER = logging.getLogger(__name__) @@ -51,15 +49,9 @@ SDK_NG_MINIMAL_MIRRORS = str_to_lst_of_str( def get_sdk_nrf_tools_path() -> Path: - # A blank ESPHOME_SDK_NRF_PREFIX must be treated as unset: Path("") - # resolves to the CWD, which clean-all would then delete. - if prefix := get_str_env("ESPHOME_SDK_NRF_PREFIX", "").strip(): - path = Path(prefix).expanduser() - else: - # Machine-global (OS user cache dir) so all projects share one install; - # see espidf.framework.get_idf_tools_path for the location rationale. - path = Path(platformdirs.user_cache_dir("esphome", appauthor=False)) / "sdk-nrf" - return path.resolve() + # Machine-global (OS user cache dir) so all projects share one install; + # see espidf.framework.get_idf_tools_path for the location rationale. + return tools_cache_path("ESPHOME_SDK_NRF_PREFIX", "sdk-nrf") def _needs_venv_rebuild( diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 0f6ef873b8..6c01058044 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -11,8 +11,6 @@ import re import shutil from typing import Any, NoReturn -import platformdirs - from esphome.core import CORE, Version from esphome.framework_helpers import ( PathType, @@ -26,8 +24,9 @@ from esphome.framework_helpers import ( run_command, run_command_ok, str_to_lst_of_str, + tools_cache_path, ) -from esphome.helpers import get_bool_env, get_str_env, write_file_if_changed +from esphome.helpers import get_bool_env, write_file_if_changed _LOGGER = logging.getLogger(__name__) @@ -88,22 +87,10 @@ def get_idf_tools_path() -> Path: Returns: Path object pointing to the ESP-IDF tools directory """ - # Treat an empty/whitespace ESPHOME_ESP_IDF_PREFIX as unset: Path("") - # resolves to the CWD, which would install into (and let clean-all delete) - # the working directory by accident. - if prefix := get_str_env("ESPHOME_ESP_IDF_PREFIX", "").strip(): - path = Path(prefix).expanduser() - else: - # Machine-global so all projects share the multi-GB install instead of - # a per-config-directory copy. The user cache dir (not ~/.esphome) - # avoids colliding with data_dir when configs live in the home dir. - # appauthor=False drops the redundant \ segment on Windows - # (which otherwise repeats "esphome\esphome\") to keep the path short. - path = Path(platformdirs.user_cache_dir("esphome", appauthor=False)) / "idf" - # Resolve so an unnormalized config path (e.g. compiling ``../config/x.yaml``) - # doesn't leave ``..`` segments in the IDF_TOOLS_PATH handed to idf.py, which - # otherwise warns that the venv interpreter path doesn't match the install. - return path.resolve() + # Machine-global so all projects share the multi-GB install instead of + # a per-config-directory copy; see framework_helpers.tools_cache_path + # for the env-override and normalization rules. + return tools_cache_path("ESPHOME_ESP_IDF_PREFIX", "idf") # Windows' default MAX_PATH is 260 characters. ESP-IDF toolchains nest deeply diff --git a/esphome/framework_helpers.py b/esphome/framework_helpers.py index b8a43220ff..224223f05d 100644 --- a/esphome/framework_helpers.py +++ b/esphome/framework_helpers.py @@ -1169,3 +1169,135 @@ def download_from_mirrors( f"No mirror URL template matched the provided substitutions:{details}" ) raise ValueError("download_from_mirrors called with an empty mirrors list") + + +def tools_cache_path(env_var: str, subdir: str) -> Path: + """A backend's machine-global tools directory, with an env override. + + A blank/whitespace override is treated as unset: ``Path("")`` resolves + to the CWD, which ``clean-all`` would then delete. + """ + import platformdirs + + from esphome.helpers import get_str_env + + if prefix := get_str_env(env_var, "").strip(): + return Path(prefix).expanduser().resolve() + return ( + Path(platformdirs.user_cache_dir("esphome", appauthor=False)) / subdir + ).resolve() + + +def strip_win_long_path_prefix(path: str) -> str: + r"""Strip the Windows extended-length path prefix from ``path``. + + Handles both forms documented at + https://learn.microsoft.com/windows/win32/fileio/naming-a-file: + + * ``\\?\C:\path\to\file`` -> ``C:\path\to\file`` + * ``\\?\UNC\server\share\path`` -> ``\\server\share\path`` + + The NSIS-installed ``esphome.exe`` launcher on Windows starts Python with + ``sys.executable`` already prefixed with ``\\?\``. That prefix propagates + into PlatformIO's ``$PYTHONEXE`` (PlatformIO reads ``PYTHONEXEPATH`` from + the environment, falling back to ``os.path.normpath(sys.executable)``) + and ends up baked into SCons-emitted command lines for build steps such + as the esp8266 ``elf2bin`` invocation. ``cmd.exe`` does not understand + the ``\\?\`` prefix, so the build fails with + "The system cannot find the path specified." Stripping the prefix early + keeps the path shell-quotable. + + Also applied to the ccache path exported by the ccache helpers, which + ``shutil.which`` can return with the same prefix. + + No-op on non-Windows platforms. + """ + if sys.platform != "win32": + return path + if path.startswith("\\\\?\\UNC\\"): + # \\?\UNC\server\share\... -> \\server\share\... + return "\\\\" + path[len("\\\\?\\UNC\\") :] + if path.startswith("\\\\?\\"): + return path[len("\\\\?\\") :] + return path + + +def _ccache_runs(ccache: str) -> bool: + """Return True when the ``ccache`` found on PATH actually runs. + + ``shutil.which`` proves existence, not runnability: on Windows it also + matches ``.bat``/``.cmd`` wrappers and stale package-manager shims whose + target is gone. Wrapping compiles around such a find fails every compile + step with an opaque OS error, so probe once and fall back to compiling + without ccache when the probe fails. + """ + try: + subprocess.run( + [ccache, "--version"], + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + timeout=15, + ) + except (OSError, subprocess.SubprocessError): + _LOGGER.warning( + "Ignoring ccache at %s because it failed to run; compiling without ccache", + ccache, + ) + return False + return True + + +def resolve_ccache_path() -> str | None: + """The ccache binary to wrap compiles with, or None when disabled. + + Shared policy for every backend: on by default when a runnable ccache is + on PATH, ``ESPHOME_CCACHE_ENABLE=0`` opts out, and an explicit ``=1`` + warns when no binary is found and skips the runnability probe. The + Windows extended-length prefix is stripped before probing so the probe + validates the exact string the build will execute (#18399). + """ + import shutil + + from esphome.helpers import get_bool_env + + explicit = "ESPHOME_CCACHE_ENABLE" in os.environ + if explicit and not get_bool_env("ESPHOME_CCACHE_ENABLE"): + return None + ccache = shutil.which("ccache") + if ccache is None: + if explicit: + _LOGGER.warning( + "ESPHOME_CCACHE_ENABLE is set but no ccache binary is on PATH; " + "compiling without ccache" + ) + return None + ccache = strip_win_long_path_prefix(ccache) + if not explicit and not _ccache_runs(ccache): + return None + return ccache + + +def ccache_defaults_env(cache_dir: Path) -> dict[str, str]: + """Default ``CCACHE_*`` values for a build subprocess (not os.environ). + + Values the user already set in the environment are respected. Depend + mode is on: both native backends emit depfiles (-MMD / CMake), which + keeps cache-miss overhead low. + """ + from esphome.core import CORE + + # build_path is set during preload for every config-loading command; unset + # means the caller built the environment too early. Fail loudly rather + # than silently drop CCACHE_BASEDIR (losing cross-device cache hits). + if CORE.build_path is None: + raise ValueError( + "CORE.build_path must be set before constructing the build environment" + ) + defaults = { + "CCACHE_DIR": str(cache_dir), + "CCACHE_NOHASHDIR": "true", + "CCACHE_DEPEND": "1", + "CCACHE_BASEDIR": str(Path(CORE.build_path).resolve()), + } + return {k: v for k, v in defaults.items() if k not in os.environ} diff --git a/esphome/platformio/toolchain.py b/esphome/platformio/toolchain.py index d76581d032..3eeaecf7e4 100644 --- a/esphome/platformio/toolchain.py +++ b/esphome/platformio/toolchain.py @@ -4,8 +4,6 @@ import logging import os from pathlib import Path import re -import shutil -import subprocess import sys from typing import TYPE_CHECKING, Any @@ -13,10 +11,10 @@ import platformdirs from esphome.const import CONF_COMPILE_PROCESS_LIMIT, CONF_ESPHOME, KEY_CORE from esphome.core import CORE, EsphomeError +from esphome.framework_helpers import resolve_ccache_path, strip_win_long_path_prefix from esphome.helpers import ( add_git_ceiling_directory, copy_file_if_changed, - get_bool_env, rmtree, write_file, ) @@ -41,40 +39,6 @@ _PIO_PYTHON_STAMP_LOCK = ".esphome.pio.stamp.lock" _PIO_PYTHON_STAMP_SCHEMA = "0" -def _strip_win_long_path_prefix(path: str) -> str: - r"""Strip the Windows extended-length path prefix from ``path``. - - Handles both forms documented at - https://learn.microsoft.com/windows/win32/fileio/naming-a-file: - - * ``\\?\C:\path\to\file`` -> ``C:\path\to\file`` - * ``\\?\UNC\server\share\path`` -> ``\\server\share\path`` - - The NSIS-installed ``esphome.exe`` launcher on Windows starts Python with - ``sys.executable`` already prefixed with ``\\?\``. That prefix propagates - into PlatformIO's ``$PYTHONEXE`` (PlatformIO reads ``PYTHONEXEPATH`` from - the environment, falling back to ``os.path.normpath(sys.executable)``) - and ends up baked into SCons-emitted command lines for build steps such - as the esp8266 ``elf2bin`` invocation. ``cmd.exe`` does not understand - the ``\\?\`` prefix, so the build fails with - "The system cannot find the path specified." Stripping the prefix early - keeps the path shell-quotable. - - Also applied to the ccache path exported by ``_ccache_env()``, which - ``shutil.which`` can return with the same prefix. - - No-op on non-Windows platforms. - """ - if sys.platform != "win32": - return path - if path.startswith("\\\\?\\UNC\\"): - # \\?\UNC\server\share\... -> \\server\share\... - return "\\\\" + path[len("\\\\?\\UNC\\") :] - if path.startswith("\\\\?\\"): - return path[len("\\\\?\\") :] - return path - - def get_platformio_config() -> "ProjectConfig | None": """Return PlatformIO's ``ProjectConfig``, or None when PlatformIO is absent.""" try: @@ -238,32 +202,6 @@ def _check_platformio_python_stamp(config: "ProjectConfig") -> None: _write_pio_stamp_python(stamp_file, current) -def _ccache_runs(ccache: str) -> bool: - """Return True when the ``ccache`` found on PATH actually runs. - - ``shutil.which`` proves existence, not runnability: on Windows it also - matches ``.bat``/``.cmd`` wrappers and stale package-manager shims whose - target is gone. Wrapping compiles around such a find fails every compile - step with an opaque OS error, so probe once and fall back to compiling - without ccache when the probe fails. - """ - try: - subprocess.run( - [ccache, "--version"], - check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - timeout=15, - ) - except (OSError, subprocess.SubprocessError): - _LOGGER.warning( - "Ignoring ccache at %s because it failed to run; compiling without ccache", - ccache, - ) - return False - return True - - def _ccache_env() -> dict[str, str]: r"""Return ccache settings for PlatformIO builds. @@ -282,7 +220,7 @@ def _ccache_env() -> dict[str, str]: runs fine through ``CreateProcess``, which is how ESP-IDF invokes it, but SCons runs every compile through ``cmd.exe``, which fails on it with "The system cannot find the path specified." (#18399), so the prefix is - stripped here with ``_strip_win_long_path_prefix()`` before the + stripped here with ``strip_win_long_path_prefix()`` before the runnability probe, which therefore validates the exact string the build will execute. ``ESPHOME_CCACHE_PATH`` is an internal channel, not a user setting: the @@ -308,22 +246,8 @@ def _ccache_env() -> dict[str, str]: build dir. The other ``CCACHE_*`` values the user already set in the environment are respected. """ - explicit = "ESPHOME_CCACHE_ENABLE" in os.environ - if explicit and not get_bool_env("ESPHOME_CCACHE_ENABLE"): - return {"ESPHOME_CCACHE_ENABLE": "0"} - ccache_path = shutil.which("ccache") + ccache_path = resolve_ccache_path() if ccache_path is None: - if explicit: - _LOGGER.warning( - "ESPHOME_CCACHE_ENABLE is set but no ccache binary is on PATH; " - "compiling without ccache" - ) - return {"ESPHOME_CCACHE_ENABLE": "0"} - # Strip before probing so the probe validates (and the failure warning - # names) the exact string the build will execute through cmd.exe. - ccache_path = _strip_win_long_path_prefix(ccache_path) - # An explicit opt-in skips the runnability probe. - if not explicit and not _ccache_runs(ccache_path): return {"ESPHOME_CCACHE_ENABLE": "0"} env = { "ESPHOME_CCACHE_ENABLE": "1", @@ -385,7 +309,7 @@ def run_platformio_cli(*args, **kwargs) -> str | int: # Strip the Windows extended-length path prefix from sys.executable so it # doesn't propagate into PlatformIO's $PYTHONEXE and break SCons-emitted # command lines run through cmd.exe. - python_exe = _strip_win_long_path_prefix(sys.executable) + python_exe = strip_win_long_path_prefix(sys.executable) if python_exe != sys.executable: # Only override PYTHONEXEPATH when we actually stripped a prefix. # PlatformIO's get_pythonexe_path() reads this and falls back to diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index 2e72dfb26a..7602674c16 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -202,7 +202,7 @@ def test_registry_download_no_system_match() -> None: def test_registry_download_version_not_found() -> None: - resp = _registry_response([]) + resp = MagicMock() resp.json.return_value = {"versions": [{"name": "2.0.0", "files": []}]} with ( patch("requests.get", return_value=resp), @@ -364,7 +364,7 @@ def test_ccache_path_explicit_skips_probe(monkeypatch: pytest.MonkeyPatch) -> No monkeypatch.setenv("ESPHOME_CCACHE_ENABLE", "1") with ( patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.platformio.toolchain._ccache_runs", side_effect=AssertionError), + patch("esphome.framework_helpers._ccache_runs", side_effect=AssertionError), ): assert framework.ccache_path() == "/usr/bin/ccache" @@ -429,3 +429,13 @@ def test_install_package_marker_rechecked_under_lock(tmp_path: Path) -> None: framework._install_package("pkg", "1.0.0", dest, ["http://m"]) mock_download.assert_not_called() mock_rmdir.assert_not_called() + + +def test_ccache_env_requires_build_path() -> None: + """Building the env before preload set build_path fails loudly.""" + CORE.build_path = None + with ( + patch.object(framework, "ccache_path", return_value="/cc/ccache"), + pytest.raises(ValueError, match="build_path"), + ): + framework.ccache_env() diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index 172b288c25..cb421c6d01 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -874,7 +874,7 @@ def test_strip_win_long_path_prefix( ) -> None: r"""``\\?\`` and ``\\?\UNC\`` prefixes are stripped only on win32.""" with patch("esphome.platformio.toolchain.sys.platform", platform): - assert toolchain._strip_win_long_path_prefix(input_path) == expected + assert toolchain.strip_win_long_path_prefix(input_path) == expected def test_run_platformio_cli_strips_win_long_path_prefix(