From ad69718eccce320e1134a2f04a92711ad4ab8955 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 31 Aug 2026 13:01:13 -0500 Subject: [PATCH] [core] Configure the platform again after prefetching its packages (#18830) --- esphome/platformio/prefetch.py | 68 +++++++++++++------ tests/unit_tests/test_platformio_prefetch.py | 70 ++++++++++++++++++++ 2 files changed, 118 insertions(+), 20 deletions(-) diff --git a/esphome/platformio/prefetch.py b/esphome/platformio/prefetch.py index ef8c27c9aa..1df0a4b328 100644 --- a/esphome/platformio/prefetch.py +++ b/esphome/platformio/prefetch.py @@ -16,8 +16,9 @@ name and promote with an atomic rename. from __future__ import annotations +from collections.abc import Iterator from concurrent.futures import ThreadPoolExecutor -from contextlib import suppress +from contextlib import contextmanager, suppress import hashlib import json import logging @@ -43,6 +44,17 @@ from esphome.helpers import get_bool_env, get_usable_cpu_count, rmtree _LOGGER = logging.getLogger(__name__) + +@contextmanager +def _preserved_sys_path() -> Iterator[None]: + """Platform setup may rewrite sys.path (pioarduino's penv does); undo it.""" + saved = list(sys.path) + try: + yield + finally: + sys.path[:] = saved + + # Concurrent registry resolutions / HEAD probes (each is network-bound) _RESOLVE_WORKERS = 8 @@ -96,6 +108,14 @@ class _Resolved(NamedTuple): cached: bool +class _Group(NamedTuple): + """The installable ``(name, spec)`` entries of one package manager.""" + + manager: Any + entries: list[tuple[str, Any]] + is_platform: bool + + # Child records a no-work run; the parent skips the next spawn while valid _SENTINEL_NAME = ".esphome_prefetch.json" _SENTINEL_SCHEMA = 1 @@ -772,13 +792,9 @@ def _preinstall( # poison the next wave; pio run installs the rest cleanly _LOGGER.warning("Skipping the dependency wave") return - # The builtin probe may construct platforms whose setup rewrites - # sys.path (see _prefetch); restore it for later imports - saved_sys_path = list(sys.path) - try: + # The builtin probe may construct platforms + with _preserved_sys_path(): next_entries = _dependency_entries(manager, installed, seen) - finally: - sys.path[:] = saved_sys_path if next_entries: # Terminates without a cap: every wave admits only never-seen # names, so a cycle yields an empty next wave @@ -803,15 +819,13 @@ def _prefetch(build_dir: Path, env: str) -> None: return # The platform (manifest plus build scripts) installs first and - # resolves the rest. Its setup may rewrite sys.path (pioarduino's penv - # setup does); restore it so later imports here still resolve. - saved_sys_path = list(sys.path) - pm = PlatformPackageManager() - _sweep_stale_sidecars(Path(pm.get_download_dir()), pm.DOWNLOAD_CACHE_EXPIRE) - pkg = pm.install(platform_spec, skip_dependencies=True) - p = PlatformFactory.new(pkg) - p.configure_project_packages(env, ["run"]) - sys.path[:] = saved_sys_path + # resolves the rest + with _preserved_sys_path(): + pm = PlatformPackageManager() + _sweep_stale_sidecars(Path(pm.get_download_dir()), pm.DOWNLOAD_CACHE_EXPIRE) + pkg = pm.install(platform_spec, skip_dependencies=True) + p = PlatformFactory.new(pkg) + p.configure_project_packages(env, ["run"]) specs = [ p.get_package_spec(name) @@ -851,9 +865,9 @@ def _prefetch(build_dir: Path, env: str) -> None: seen: set[str] = set() jobs: list[tuple[str, int, Any]] = [] - groups: list[tuple[Any, list[tuple[str, Any]]]] = [] + groups: list[_Group] = [] unresolved = 0 - for mgr, batch in ((p.pm, specs), (lm, lib_specs)): + for mgr, batch, is_platform in ((p.pm, specs, True), (lm, lib_specs, False)): entries: list[tuple[str, Any]] = [] for build_jobs in (_registry_jobs, _uri_jobs): batch_jobs, failed, installable = build_jobs(mgr, batch, seen) @@ -861,7 +875,7 @@ def _prefetch(build_dir: Path, env: str) -> None: unresolved += failed entries += installable if entries: - groups.append((mgr, entries)) + groups.append(_Group(mgr, entries, is_platform)) sentinel = build_dir / _SENTINEL_NAME if jobs or groups: @@ -890,7 +904,8 @@ def _prefetch(build_dir: Path, env: str) -> None: encoding="utf-8", ) - for mgr, entries in groups: + platform_packages_installed = False + for mgr, entries, is_platform in groups: # One install per destination: pio derives the directory from # the package name, so key on the name part to_install = { @@ -901,6 +916,8 @@ def _prefetch(build_dir: Path, env: str) -> None: if to_install: try: _preinstall(mgr, list(to_install.values())) + if is_platform: + platform_packages_installed = True except Exception as err: # noqa: BLE001 # pylint: disable=broad-exception-caught # Each group degrades independently; pio run installs # whatever this one did not @@ -910,6 +927,17 @@ def _prefetch(build_dir: Path, env: str) -> None: failure_reason(err), ) _LOGGER.debug("Pre-install group failure detail", exc_info=True) + if platform_packages_installed: + # pioarduino installs its real toolchains from configure (the registry + # package is a stub); settle that here so pio run does not redo it + with _preserved_sys_path(), ThreadPoolExecutor(max_workers=1) as ex: + # A worker so SIGTERM joins it; exception() so a postinstall exit only warns + err = ex.submit(p.configure_project_packages, env, ["run"]).exception() + if err is not None: + _LOGGER.warning( + "Could not settle platform packages: %s", failure_reason(err) + ) + _LOGGER.debug("Platform settle failure detail", exc_info=err) def _sigterm(_signum, _frame) -> None: diff --git a/tests/unit_tests/test_platformio_prefetch.py b/tests/unit_tests/test_platformio_prefetch.py index 91fb78c6af..d0785d2724 100644 --- a/tests/unit_tests/test_platformio_prefetch.py +++ b/tests/unit_tests/test_platformio_prefetch.py @@ -1417,6 +1417,76 @@ def test_prefetch_installs_cached_archives_without_downloads( assert not (tmp_path / pf._SENTINEL_NAME).exists() +@pytest.mark.parametrize( + ("platform_group", "lib_group", "expected"), + [ + ( + [("toolchain-x@1", _FakeSpec(name="toolchain-x"))], + [], + ["configure", "install", "configure"], + ), + ([], [("noise-c@1.0", _FakeSpec(name="noise-c"))], ["configure", "install"]), + ], +) +def test_prefetch_reconfigures_only_after_platform_installs( + tmp_path: Path, platform_group: list, lib_group: list, expected: list[str] +) -> None: + """Installed platform packages get a second configure pass; libraries do not.""" + _write_ini(tmp_path, "[env:testenv]\nplatform = fake/p@1\n") + order: list[str] = [] + fake_platform = MagicMock() + fake_platform.packages = {} + fake_platform.configure_project_packages.side_effect = lambda env, targets: ( + order.append("configure") + ) + config = _fake_config( + tmp_path, {"platform": "fake/p@1", "lib_deps": ["esphome/noise-c@1.0"]} + ) + modules = _pio_modules(tmp_path, fake_platform, MagicMock(), config) + with ( + patch.dict("sys.modules", modules), + patch.object( + pf, + "_registry_jobs", + side_effect=[([], 0, platform_group), ([], 0, lib_group)], + ), + patch.object(pf, "_uri_jobs", return_value=([], 0, [])), + patch.object(pf, "_preinstall", side_effect=lambda *_: order.append("install")), + ): + pf._prefetch(tmp_path, "testenv") + assert order == expected + + +@pytest.mark.parametrize( + "err", [RuntimeError("idf_tools.py failed"), SystemExit("postinstall exited")] +) +def test_prefetch_settle_failure_warns_and_continues( + tmp_path: Path, caplog: pytest.LogCaptureFixture, err: BaseException +) -> None: + """A failing second configure pass only costs the speedup.""" + _write_ini(tmp_path, "[env:testenv]\nplatform = fake/p@1\n") + fake_platform = MagicMock() + fake_platform.packages = {} + fake_platform.configure_project_packages.side_effect = [None, err] + config = _fake_config(tmp_path, {"platform": "fake/p@1"}) + modules = _pio_modules(tmp_path, fake_platform, MagicMock(), config) + with ( + patch.dict("sys.modules", modules), + patch.object( + pf, + "_registry_jobs", + side_effect=[ + ([], 0, [("toolchain-x@1", _FakeSpec(name="toolchain-x"))]), + ([], 0, []), + ], + ), + patch.object(pf, "_uri_jobs", return_value=([], 0, [])), + patch.object(pf, "_preinstall"), + ): + pf._prefetch(tmp_path, "testenv") + assert f"Could not settle platform packages: {err}" in caplog.text + + def test_preinstall_extracts_in_parallel_under_one_lock(tmp_path: Path) -> None: """The manager lock wraps the whole batch; per-thread managers share its package dir; one failing install leaves the rest alone."""