From 3d540b1597eb247e570d602445974e6db3673cf1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 21:16:27 -0500 Subject: [PATCH] Require a resolved ccache, dedupe the package specs, use the shared cache spec --- esphome/arduino8266/framework.py | 92 +++++++------------ .../unit_tests/test_arduino8266_framework.py | 40 ++------ 2 files changed, 41 insertions(+), 91 deletions(-) diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index 125238a30a..1edbe4b36f 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -15,11 +15,11 @@ from __future__ import annotations import os from pathlib import Path -from typing import Any, NamedTuple +from typing import NamedTuple -from esphome.build_helpers.ccache import ccache_defaults_env, resolve_ccache_path +from esphome.build_helpers.ccache import ccache_defaults_env from esphome.build_helpers.ninja import find_ninja -from esphome.build_helpers.tools_cache import tools_cache_path +from esphome.build_helpers.tools_cache import ARDUINO8266_TOOLS_CACHE, tools_cache_path from esphome.core import EsphomeError, Version from esphome.framework_helpers import str_to_lst_of_str from esphome.platformio.registry import install_package, prefetch_packages @@ -41,7 +41,7 @@ ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS = str_to_lst_of_str( def get_arduino8266_tools_path() -> Path: # 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") + return tools_cache_path(*ARDUINO8266_TOOLS_CACHE) # 3.1.1 rather than 3.1.0: the registry has no package for 3.1.0, and the @@ -90,8 +90,8 @@ class InstalledPaths(NamedTuple): 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 will enforce this once the native backend is - # wired in; keep the module honest when called directly. + # Config validation enforces this too; keep the module honest when + # called directly. raise EsphomeError( f"The native toolchain requires the Arduino core " f">= {MIN_FRAMEWORK_VERSION}, got {framework_version}" @@ -102,50 +102,33 @@ def check_and_install(framework_version: Version) -> InstalledPaths: framework_path = get_framework_path(package_version) downloads_dir = get_arduino8266_tools_path() / "downloads" toolchain_path = get_toolchain_path() + # One spec per package: the prefetch and the installs must agree + specs = ( + ( + FRAMEWORK_PACKAGE, + package_version, + framework_path, + ESPHOME_ARDUINO8266_FRAMEWORK_MIRRORS, + ("cores/esp8266", "tools/sdk", "libraries"), + ), + ( + TOOLCHAIN_PACKAGE, + TOOLCHAIN_VERSION, + toolchain_path, + ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS, + # xtensa-lx106-elf pins the target: every gcc package has a bin/ + ("bin", "xtensa-lx106-elf"), + ), + ) # Fetch both archives at once; the installs below verify and extract - prefetch_packages( - [ - ( - FRAMEWORK_PACKAGE, - package_version, - framework_path, - ESPHOME_ARDUINO8266_FRAMEWORK_MIRRORS, - ), - ( - TOOLCHAIN_PACKAGE, - TOOLCHAIN_VERSION, - toolchain_path, - ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS, - ), - ], - downloads_dir, - ) - install_package( - FRAMEWORK_PACKAGE, - package_version, - framework_path, - ESPHOME_ARDUINO8266_FRAMEWORK_MIRRORS, - downloads_dir, - expect=("cores/esp8266", "tools/sdk", "libraries"), - ) - install_package( - TOOLCHAIN_PACKAGE, - TOOLCHAIN_VERSION, - toolchain_path, - ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS, - downloads_dir, - # xtensa-lx106-elf pins the target: every gcc package has a bin/ - expect=("bin", "xtensa-lx106-elf"), - ) + prefetch_packages([spec[:4] for spec in specs], downloads_dir) + for name, version, dest, mirrors, expect in specs: + install_package(name, version, dest, mirrors, downloads_dir, expect=expect) return InstalledPaths( framework=framework_path, toolchain=toolchain_path, ninja=ninja_path ) -# Sentinel: "resolve for me"; None is a real value meaning disabled. -CCACHE_UNRESOLVED: Any = object() - - def toolchain_tool(toolchain_path: Path, name: str) -> Path: """Path to one toolchain tool (gcc, g++, ar, size, addr2line, ...). @@ -156,9 +139,7 @@ def toolchain_tool(toolchain_path: Path, name: str) -> Path: return toolchain_path / "bin" / f"xtensa-lx106-elf-{name}{suffix}" -def get_build_env( - toolchain_path: Path, ccache: str | None = CCACHE_UNRESOLVED -) -> dict[str, str]: +def get_build_env(toolchain_path: Path, ccache: str | None) -> dict[str, str]: env = os.environ.copy() # Drop empty entries: a trailing separator from an absent PATH would # make the shell search the current directory for tools @@ -171,22 +152,13 @@ def get_build_env( return env -def ccache_path() -> str | None: - """The ccache binary to prefix compiles with, or None when disabled. - - Deliberately uncached: env/PATH can change between builds in a - long-lived host process. - """ - return resolve_ccache_path() - - -def ccache_env(ccache: str | None = CCACHE_UNRESOLVED) -> dict[str, str]: +def ccache_env(ccache: str | None) -> dict[str, str]: """Return ccache settings for the build subprocess (not os.environ). - Values the user already set in the environment are respected. + ``ccache`` is the pre-resolved binary (resolve_ccache_path), or None + when disabled. Values the user already set in the environment are + respected. """ - if ccache is CCACHE_UNRESOLVED: - ccache = ccache_path() if ccache is None: return {} return ccache_defaults_env(get_arduino8266_tools_path() / "ccache") diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index 932958d667..bd0a620e10 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -112,33 +112,15 @@ def test_check_and_install_returns_paths(tmp_path: Path) -> None: def test_get_build_env_prepends_toolchain_bin(tmp_path: Path) -> None: with patch.object(framework, "ccache_env", return_value={"CCACHE_DIR": "x"}): - env = framework.get_build_env(tmp_path) + env = framework.get_build_env(tmp_path, None) assert env["PATH"].startswith(str(tmp_path / "bin") + os.pathsep) assert env["CCACHE_DIR"] == "x" -def test_ccache_path_delegates_uncached( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Delegates on every call; the env/PATH decision must not freeze for - the process lifetime.""" - monkeypatch.delenv("ESPHOME_CCACHE_ENABLE", raising=False) - with patch.object( - framework, "resolve_ccache_path", return_value="/usr/bin/ccache" - ) as mock_resolve: - assert framework.ccache_path() == "/usr/bin/ccache" - assert framework.ccache_path() == "/usr/bin/ccache" - assert mock_resolve.call_count == 2 - - def test_ccache_env(tmp_path: Path) -> None: - with patch.object(framework, "ccache_path", return_value=None): - assert framework.ccache_env() == {} - with ( - patch.object(framework, "ccache_path", return_value="/usr/bin/ccache"), - patch.dict(os.environ, {"CCACHE_NOHASHDIR": "false"}, clear=True), - ): - env = framework.ccache_env() + assert framework.ccache_env(None) == {} + with patch.dict(os.environ, {"CCACHE_NOHASHDIR": "false"}, clear=True): + env = framework.ccache_env("/usr/bin/ccache") # User-set values are respected; the rest get defaults assert "CCACHE_NOHASHDIR" not in env assert env["CCACHE_DEPEND"] == "1" @@ -159,7 +141,7 @@ def test_get_build_env_without_path_has_no_empty_entry(tmp_path: Path) -> None: patch.dict(os.environ, {}, clear=True), patch.object(framework, "ccache_env", return_value={}), ): - env = framework.get_build_env(tmp_path) + env = framework.get_build_env(tmp_path, None) assert env["PATH"] == str(tmp_path / "bin") with ( patch.dict( @@ -167,20 +149,16 @@ def test_get_build_env_without_path_has_no_empty_entry(tmp_path: Path) -> None: ), patch.object(framework, "ccache_env", return_value={}), ): - env = framework.get_build_env(tmp_path) + env = framework.get_build_env(tmp_path, None) assert env["PATH"].split(os.pathsep) == [str(tmp_path / "bin"), "/usr/bin", "/bin"] def test_ccache_env_accepts_a_preresolved_path() -> None: - """A caller that already resolved ccache threads it through; the probe - must not run again (None means resolved-and-disabled).""" - with ( - patch.dict(os.environ, {}, clear=True), - patch.object(framework, "ccache_path") as mock_resolve, - ): + """The caller resolves ccache once and threads it through; None means + resolved-and-disabled.""" + with patch.dict(os.environ, {}, clear=True): assert framework.ccache_env(None) == {} env = framework.ccache_env("/usr/bin/ccache") - mock_resolve.assert_not_called() assert env["CCACHE_DIR"].endswith("ccache")