From 9caeb948f3c8569be38d75340b2a26a7ea22ef42 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 25 Aug 2026 13:20:30 -0500 Subject: [PATCH] Warn when forced-on ccache has no usable binary, skip freshly installed dests in the prefetch, derive the clean-all sandbox from the cache registry --- esphome/espidf/framework.py | 11 ++++++++--- esphome/platformio/registry.py | 5 +++++ tests/unit_tests/test_espidf_framework.py | 9 ++++++--- tests/unit_tests/test_platformio_registry.py | 16 ++++++++++++++++ tests/unit_tests/test_writer.py | 16 +++++++++------- 5 files changed, 44 insertions(+), 13 deletions(-) diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 25fbc1aa52..4b7653616a 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -1209,9 +1209,14 @@ def _ccache_env() -> dict[str, str]: # export the canonical off spelling instead return {"IDF_CCACHE_ENABLE": "0"} if idf_knob is True: - # Forced on skips the runnability verdict, but still resolve for - # the "no ccache binary on PATH" warning - resolve_ccache_path() + # Forced on ignores the runnability verdict, but a missing or + # unusable binary is worth saying out loud: idf.py silently + # compiles without ccache in that case + if resolve_ccache_path() is None: + _LOGGER.warning( + "IDF_CCACHE_ENABLE=1 but no usable ccache binary was " + "found; idf.py will compile without ccache" + ) elif resolve_ccache_path() is None: # ESP-IDF silently skips ccache without the binary; export the # canonical off spelling so an unparsable inherited value (or a diff --git a/esphome/platformio/registry.py b/esphome/platformio/registry.py index ca012ccbbe..a04caa8487 100644 --- a/esphome/platformio/registry.py +++ b/esphome/platformio/registry.py @@ -218,6 +218,11 @@ def prefetch_packages( def _fetch(entry: _PendingArchive, tracker: Callable[[int], None]) -> None: entry.dest.parent.mkdir(parents=True, exist_ok=True) with FileLock(f"{entry.dest}.lock", fallback_to_soft=False): + if (entry.dest / ".esphome_extracted").is_file(): + # A concurrent build installed (and deleted the archive of) + # this package while we waited; re-downloading would orphan + # a fresh copy in downloads_dir + return download_with_resume( entry.url, downloads_dir / f"{entry.name}-{entry.version}", diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index 42a00ef0d7..ecf54349be 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -1602,15 +1602,18 @@ def test_ccache_env_opt_out_via_env(tmp_path: Path) -> None: assert _ccache_env() == {"IDF_CCACHE_ENABLE": "0"} -def test_ccache_env_opt_in_without_binary(tmp_path: Path) -> None: - # Explicit IDF_CCACHE_ENABLE=1 forces it on; the probe verdict is - # ignored but the resolver still runs for its no-binary warning. +def test_ccache_env_opt_in_without_binary( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + # Explicit IDF_CCACHE_ENABLE=1 forces it on; without a usable binary + # idf.py silently skips ccache, so this branch must say so out loud. p1, p2, p3 = _ccache_patches(tmp_path, None, tmp_path / "build") with patch.dict("os.environ", {"IDF_CCACHE_ENABLE": "1"}, clear=True), p1, p2, p3: env = _ccache_env() assert env["IDF_CCACHE_ENABLE"] == "1" assert env["CCACHE_DIR"] == str(tmp_path / "tools" / "ccache") assert env["CCACHE_DEPEND"] == "1" + assert "no usable ccache binary" in caplog.text def test_ccache_env_honors_shared_esphome_opt_out(tmp_path: Path) -> None: diff --git a/tests/unit_tests/test_platformio_registry.py b/tests/unit_tests/test_platformio_registry.py index 70e49d1f5b..56bfe4fe74 100644 --- a/tests/unit_tests/test_platformio_registry.py +++ b/tests/unit_tests/test_platformio_registry.py @@ -534,6 +534,22 @@ def test_prefetch_packages_downloads_pending_in_parallel(tmp_path: Path) -> None assert callable(call[1]["progress"]) +def test_prefetch_packages_skips_freshly_installed_dest(tmp_path: Path) -> None: + """A dest whose marker appeared while the worker waited on the lock is + already installed; re-downloading would orphan an archive copy.""" + dest = tmp_path / "a" + dest.mkdir() + (dest / ".esphome_extracted").touch() + with ( + patch.object(registry, "download_with_resume") as mock_download, + patch.object( + registry, "registry_download", side_effect=_resolve_for({"a": 10}) + ), + ): + registry.prefetch_packages([("a", "1.0", dest, [])], tmp_path / "dl") + mock_download.assert_not_called() + + def test_prefetch_packages_dedupes_duplicate_entries(tmp_path: Path) -> None: """Duplicate (name, version) entries would race each other between two workers; only one survives (and one is too few to parallelize).""" diff --git a/tests/unit_tests/test_writer.py b/tests/unit_tests/test_writer.py index 0a53dba9c2..9c20ee10d2 100644 --- a/tests/unit_tests/test_writer.py +++ b/tests/unit_tests/test_writer.py @@ -14,6 +14,7 @@ from unittest.mock import MagicMock, patch import pytest +from esphome.build_helpers.tools_cache import TOOLS_CACHE_SPECS from esphome.const import ( PLATFORM_BK72XX, PLATFORM_ESP32, @@ -68,15 +69,12 @@ def _isolate_platformio_paths(tmp_path_factory: pytest.TempPathFactory) -> Any: test_clean_all_partial_exists) install their own inner patch which stacks on top of this one and wins for the duration of their block. - Also pin ``ESPHOME_ESP_IDF_PREFIX`` and ``ESPHOME_SDK_NRF_PREFIX`` to - nonexistent tmp dirs, and patch ``platformdirs.user_cache_dir``, for the - same reason: ``clean_all`` removes the machine-global toolchain installs + Also pin every ``TOOLS_CACHE_SPECS`` env override to a nonexistent tmp + dir, and patch ``platformdirs.user_cache_dir``, for the same reason: ``clean_all`` removes the machine-global toolchain installs and their default cache root, which otherwise resolve to the real ``~/.cache/esphome``. """ pio_root = tmp_path_factory.mktemp("isolated_pio") / "nonexistent" - idf_root = tmp_path_factory.mktemp("isolated_idf") / "nonexistent" - sdk_nrf_root = tmp_path_factory.mktemp("isolated_sdk_nrf") / "nonexistent" cache_root = tmp_path_factory.mktemp("isolated_cache") / "nonexistent" mock_cfg = MagicMock() mock_cfg.get.side_effect = lambda section, option: ( @@ -90,8 +88,12 @@ def _isolate_platformio_paths(tmp_path_factory: pytest.TempPathFactory) -> Any: patch.dict( "os.environ", { - "ESPHOME_ESP_IDF_PREFIX": str(idf_root), - "ESPHOME_SDK_NRF_PREFIX": str(sdk_nrf_root), + # Derived from the registry so a new backend's cache can + # never drift out of the sandbox and hit a real toolchain + env_var: str( + tmp_path_factory.mktemp(f"isolated_{subdir}") / "nonexistent" + ) + for env_var, subdir in TOOLS_CACHE_SPECS }, ), patch("platformdirs.user_cache_dir", return_value=str(cache_root)),