diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 0eb695d2a2..8d6d09e0f6 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -1208,7 +1208,10 @@ def _ccache_env() -> dict[str, str]: # ESPHOME_CCACHE_ENABLE. idf_knob = parse_enable_env("IDF_CCACHE_ENABLE") if idf_knob is False: - return {} + # The raw value (e.g. "disable") is still inherited by idf.py via + # os.environ, where a non-false-constant string reads as truthy; + # export the canonical off spelling instead + return {"IDF_CCACHE_ENABLE": "0"} if idf_knob is None and resolve_ccache_path() is None: # ESP-IDF silently skips ccache without the binary; don't enable it. return {} diff --git a/esphome/platformio/registry.py b/esphome/platformio/registry.py index acf5b69801..a5c47e9199 100644 --- a/esphome/platformio/registry.py +++ b/esphome/platformio/registry.py @@ -151,12 +151,22 @@ def prefetch_packages( an optimization: ``install_package`` verifies every archive and re-downloads anything this pass left unfinished. Mirror overrides and registry entries without a size stay on the sequential path so its - per-file bars remain trustworthy. + per-file bars remain trustworthy. Each fetch holds the same per-dest + lock as ``install_package``: the archive's ``.part`` file is shared, and + two concurrent writers would truncate each other's bytes. """ - pending: list[tuple[str, str, str, str, int]] = [] + from filelock import FileLock + + pending: list[tuple[str, str, Path, str, str, int]] = [] + seen: set[str] = set() for name, version, dest, mirrors in packages: if mirrors or (dest / ".esphome_extracted").is_file(): continue + archive_name = f"{name}-{version}" + if archive_name in seen: + # A duplicate entry would race itself between two workers + continue + seen.add(archive_name) try: url, sha256, size = registry_download(name, version) except EsphomeError as err: @@ -165,32 +175,35 @@ def prefetch_packages( continue if not size: continue - archive = downloads_dir / f"{name}-{version}" + archive = downloads_dir / archive_name if archive.is_file() and archive.stat().st_size == size: continue - pending.append((name, version, url, sha256, size)) + pending.append((name, version, dest, url, sha256, size)) if len(pending) < 2: return downloads_dir.mkdir(parents=True, exist_ok=True) _LOGGER.info( "Downloading %d package archive(s): %s", len(pending), - ", ".join(name for name, _, _, _, _ in pending), + ", ".join(name for name, *_ in pending), ) progress = BatchDownloadProgress( "Downloading packages", sum(size for *_, size in pending) ) - def _fetch(entry: tuple[str, str, str, str, int]) -> None: - name, version, url, sha256, size = entry + def _fetch(entry: tuple[str, str, Path, str, str, int]) -> None: + name, version, dest, url, sha256, size = entry + tracker = progress.tracker() try: - download_with_resume( - url, - downloads_dir / f"{name}-{version}", - sha256=sha256, - size=size, - progress=progress.tracker(), - ) + dest.parent.mkdir(parents=True, exist_ok=True) + with FileLock(f"{dest}.lock", fallback_to_soft=False): + download_with_resume( + url, + downloads_dir / f"{name}-{version}", + sha256=sha256, + size=size, + progress=tracker, + ) except Exception as err: # noqa: BLE001 # pylint: disable=broad-exception-caught # install_package retries this one itself, with a visible bar _LOGGER.debug("Prefetch of %s failed: %s", name, err) diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index 13ac8e714b..1ca25d8ca3 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -1568,7 +1568,9 @@ def test_ccache_env_opt_out_via_env(tmp_path: Path) -> None: # short-circuits before build_path is needed. p1, p2, p3 = _ccache_patches(tmp_path, "/usr/bin/ccache", None) with patch.dict("os.environ", {"IDF_CCACHE_ENABLE": "0"}, clear=True), p1, p2, p3: - assert _ccache_env() == {} + # The canonical off spelling is exported: the raw value is inherited + # by idf.py, where a spelling like "disable" would read as truthy + assert _ccache_env() == {"IDF_CCACHE_ENABLE": "0"} def test_ccache_env_opt_in_without_binary(tmp_path: Path) -> None: @@ -1598,7 +1600,7 @@ def test_ccache_env_idf_knob_parses_strictly(tmp_path: Path, value: str) -> None "off" disables instead of reading as truthy.""" p1, p2, p3 = _ccache_patches(tmp_path, "/usr/bin/ccache", tmp_path / "build") with patch.dict("os.environ", {"IDF_CCACHE_ENABLE": value}, clear=True), p1, p2, p3: - assert _ccache_env() == {} + assert _ccache_env() == {"IDF_CCACHE_ENABLE": "0"} def test_ccache_env_idf_knob_unrecognized_warns_and_defers( diff --git a/tests/unit_tests/test_platformio_registry.py b/tests/unit_tests/test_platformio_registry.py index cf995ad2f4..3f2b739ca9 100644 --- a/tests/unit_tests/test_platformio_registry.py +++ b/tests/unit_tests/test_platformio_registry.py @@ -481,8 +481,10 @@ def test_prefetch_packages_downloads_pending_in_parallel(tmp_path: Path) -> None tmp_path / "dl", ) assert mock_download.call_count == 2 + # Locking makes worker completion order nondeterministic + calls = sorted(mock_download.call_args_list, key=lambda c: c[0][0]) for call, (name, version, size) in zip( - mock_download.call_args_list, [("a", "1.0", 10), ("b", "2.0", 20)], strict=True + calls, [("a", "1.0", 10), ("b", "2.0", 20)], strict=True ): assert call[0][0] == f"http://x/{name}.tar.gz" assert call[0][1] == tmp_path / "dl" / f"{name}-{version}" @@ -491,6 +493,25 @@ def test_prefetch_packages_downloads_pending_in_parallel(tmp_path: Path) -> None assert callable(call[1]["progress"]) +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).""" + 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", tmp_path / "a", []), + ("a", "1.0", tmp_path / "a", []), + ], + tmp_path / "dl", + ) + mock_download.assert_not_called() + + def test_prefetch_packages_single_pending_skips(tmp_path: Path) -> None: """One pending package has nothing to parallelize; the sequential install keeps its own bar."""