[core] Configure the platform again after prefetching its packages (#18830)

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:

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."""