Merge branch 'esp8266-native-build-surgery' into esp8266-native-toolchain-plumbing

esphome/__main__.py

@@ -857,18 +857,14 @@ def compile_program(args: ArgsProtocol, config: ConfigType) -> int:
         toolchain.create_factory_bin()
         toolchain.create_ota_bin()
         toolchain.create_elf_copy()
+        from esphome.build_helpers.idedata import IDEDATA_BEST_EFFORT_ERRORS
+
         try:
             if toolchain.get_idedata() is None:
                 _LOGGER.warning("No idedata was generated for this build")
-        except (
-            EsphomeError,
-            LookupError,
-            OSError,
-            RuntimeError,
-            ValueError,
-        ) as err:
-            # Broad on purpose: the firmware already built; an idedata
-            # failure must not fail a successful build.
+        except IDEDATA_BEST_EFFORT_ERRORS as err:
+            # The firmware already built; an idedata failure must not fail
+            # a successful build.
             _LOGGER.warning("Could not generate idedata: %s", err)
             _LOGGER.debug("Idedata failure detail", exc_info=True)
     else:

esphome/build_helpers/idedata.py

@@ -21,6 +21,17 @@ import subprocess
 from esphome.core import EsphomeError
 from esphome.helpers import write_file
 
+# Everything idedata generation may raise after a successful link. Broad on
+# purpose, and shared by every consumer: idedata is a bonus artifact, so
+# these must be caught and warned about, never allowed to fail the build.
+IDEDATA_BEST_EFFORT_ERRORS = (
+    EsphomeError,
+    LookupError,
+    OSError,
+    RuntimeError,
+    ValueError,
+)
+
 _LOGGER = logging.getLogger(__name__)
 
 # C++ translation-unit suffixes used to identify ESPHome source files.
@@ -305,9 +316,24 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d
     build_includes: dict[str, None] = dict.fromkeys(
         rep_includes if _is_esphome_src(representative["file"]) else ()
     )
+
+    def _shape(entry: dict) -> str:
+        # The command minus its TU-specific paths: entries sharing a shape
+        # carry identical include sets (one ninja rule), so tokenize once
+        # per shape instead of once per TU
+        return (
+            entry["command"]
+            .replace(entry.get("file", ""), "")
+            .replace(entry.get("output", ""), "")
+        )
+
+    seen_shapes = {_shape(representative)}
     for entry in entries:
         if entry is representative or not _is_esphome_src(entry["file"]):
             continue
+        if (shape := _shape(entry)) in seen_shapes:
+            continue
+        seen_shapes.add(shape)
         for inc in parse_entry(entry, launcher)[2]:
             build_includes.setdefault(inc, None)
 

esphome/espidf/framework.py

@@ -1,7 +1,6 @@
 """ESP-IDF framework tools for ESPHome."""
 
 from collections.abc import Callable
-from concurrent.futures import ThreadPoolExecutor
 from ctypes.util import find_library
 import json
 import logging
@@ -25,6 +24,7 @@ from esphome.framework_helpers import (
     get_python_env_executable_path,
     get_system_python_path,
     rmdir,
+    run_batch_downloads,
     run_command,
     run_command_ok,
     str_to_lst_of_str,
@@ -692,12 +692,6 @@ def _patch_tools_json_demote_unused_tools(framework_path: Path) -> None:
     )
 
 
-# Tool archives are large (tens to hundreds of MB) and served by GitHub /
-# dl.espressif.com; a few streams at once saturate most links without
-# hammering the host. Smaller than external_files' 8: those are tiny files.
-_PREFETCH_WORKERS = 4
-
-
 def _prefetch_idf_tool_archives(
     framework_path: Path,
     targets_str: str,
@@ -735,17 +729,16 @@ def _prefetch_idf_tool_archives(
             )
             return
         dist_path = get_idf_tools_path() / "dist"
-        pending = [
-            entry
-            for entry in json.loads(stdout)
-            if not (dist_path / entry["dest"]).is_file()
-        ]
-        # tools.json always carries sha256 and size; an entry missing either
-        # must not be downloaded unverified here, so leave it to the
-        # installer (which fails loudly on a bad archive).
-        entries = [e for e in pending if e.get("sha256") and e.get("size")]
-        for entry in pending:
-            if entry not in entries:
+        entries = []
+        for entry in json.loads(stdout):
+            if (dist_path / entry["dest"]).is_file():
+                continue
+            # tools.json always carries sha256 and size; an entry missing
+            # either must not be downloaded unverified here, so leave it to
+            # the installer (which fails loudly on a bad archive).
+            if entry.get("sha256") and entry.get("size"):
+                entries.append(entry)
+            else:
                 _LOGGER.warning(
                     "Tool %s has no sha256/size in the download list; "
                     "leaving it to the installer",
@@ -758,42 +751,28 @@ def _prefetch_idf_tool_archives(
             len(entries),
             ", ".join(entry["name"] for entry in entries),
         )
+
         # Every entry carries a size (checked above), so the combined bar can
         # be trusted. Unlike the library prefetch there is no sequential
         # fallback: per-file bars from several threads would interleave, and
         # skipping the prefetch would lose the resume workaround for #17703.
-        progress = BatchDownloadProgress(
-            "Downloading ESP-IDF tools", sum(entry["size"] for entry in entries)
+        def _download(entry: dict):
+            return lambda tracker: download_with_resume(
+                entry["url"],
+                dist_path / entry["dest"],
+                sha256=entry["sha256"],
+                size=entry["size"],
+                progress=tracker,
+            )
+
+        # A failed archive is retried by the installer itself (without
+        # resume); keep prefetching the rest.
+        failures = run_batch_downloads(
+            BatchDownloadProgress(
+                "Downloading ESP-IDF tools", sum(entry["size"] for entry in entries)
+            ),
+            [(entry["name"], _download(entry)) for entry in entries],
         )
-        # Reported after the bar is done so the warnings do not land on
-        # its row; list.append is atomic under the GIL.
-        failures: list[tuple[str, Exception]] = []
-
-        def _download(entry: dict) -> None:
-            tracker = progress.tracker()
-            try:
-                download_with_resume(
-                    entry["url"],
-                    dist_path / entry["dest"],
-                    sha256=entry["sha256"],
-                    size=entry["size"],
-                    progress=tracker,
-                )
-            except Exception as e:  # noqa: BLE001  # pylint: disable=broad-exception-caught
-                # Keep prefetching the remaining archives; the installer
-                # will retry this one itself (without resume).
-                tracker(0)
-                failures.append((entry["name"], e))
-
-        ex = ThreadPoolExecutor(max_workers=min(_PREFETCH_WORKERS, len(entries)))
-        try:
-            for future in [ex.submit(_download, entry) for entry in entries]:
-                future.result()
-        finally:
-            # On Ctrl-C drop the queued archives instead of downloading them
-            # all before the process can exit; in-flight ones still finish.
-            ex.shutdown(wait=True, cancel_futures=True)
-            progress.done()
         for name, e in failures:
             _LOGGER.warning("Could not prefetch %s: %s", name, e)
     except Exception as e:  # noqa: BLE001  # pylint: disable=broad-exception-caught

esphome/framework_helpers.py

@@ -1,6 +1,7 @@
 """Generic toolchain installation helpers shared across framework implementations."""
 
 from collections.abc import Callable, Iterable
+from concurrent.futures import ThreadPoolExecutor
 from contextlib import ExitStack
 import hashlib
 import io
@@ -737,6 +738,46 @@ def _stream_response_to_file(
         own_bar.update(1)
 
 
+# Concurrent downloads per batch; enough to hide latency without
+# hammering the host or the mirrors.
+BATCH_DOWNLOAD_WORKERS = 4
+
+
+def run_batch_downloads(
+    progress: "BatchDownloadProgress",
+    jobs: list[tuple[str, Callable[[Callable[[int], None]], None]]],
+    max_workers: int = BATCH_DOWNLOAD_WORKERS,
+) -> list[tuple[str, Exception]]:
+    """Run download jobs concurrently, reporting into one combined bar.
+
+    ``jobs`` holds ``(name, fetch)`` pairs where ``fetch(tracker)`` performs
+    one download reporting absolute byte counts to ``tracker``. Failures are
+    collected (list.append is atomic under the GIL) and returned after the
+    bar is done, so the caller's warnings never land on the bar's row; a
+    failed job credits its tracker 0 so the bar can still complete. Ctrl-C
+    drops queued jobs instead of downloading them all before the process
+    can exit; in-flight ones still finish.
+    """
+    failures: list[tuple[str, Exception]] = []
+
+    def _run(name: str, fetch: Callable[[Callable[[int], None]], None]) -> None:
+        tracker = progress.tracker()
+        try:
+            fetch(tracker)
+        except Exception as err:  # noqa: BLE001  # pylint: disable=broad-exception-caught
+            failures.append((name, err))
+            tracker(0)
+
+    ex = ThreadPoolExecutor(max_workers=min(max_workers, len(jobs)))
+    try:
+        for future in [ex.submit(_run, name, fetch) for name, fetch in jobs]:
+            future.result()
+    finally:
+        ex.shutdown(wait=True, cancel_futures=True)
+        progress.done()
+    return failures
+
+
 class BatchDownloadProgress:
     """One progress bar across several concurrent ``download_with_resume`` calls.
 

esphome/platformio/library.py

@@ -32,10 +32,12 @@ from urllib.request import url2pathname
 from esphome import git
 from esphome.core import CORE, EsphomeError, Library
 from esphome.framework_helpers import (
+    BATCH_DOWNLOAD_WORKERS,
     BatchDownloadProgress,
     archive_extract_all,
     download_from_mirrors,
     rmdir,
+    run_batch_downloads,
 )
 
 _LOGGER = logging.getLogger(__name__)
@@ -896,10 +898,6 @@ def is_lib_ignored(name: str | None, lib_ignore: set[str]) -> bool:
     )
 
 
-# A few streams saturate most links without hammering the registry
-_DOWNLOAD_WORKERS = 4
-
-
 def _content_lengths(urls: list[str]) -> list[int | None]:
     """Content-Length per URL via HEAD requests; None when unknown."""
     import requests
@@ -915,7 +913,7 @@ def _content_lengths(urls: list[str]) -> list[int | None]:
             _LOGGER.debug("HEAD %s failed: %s", url, err)
             return None
 
-    with ThreadPoolExecutor(max_workers=min(_DOWNLOAD_WORKERS, len(urls))) as ex:
+    with ThreadPoolExecutor(max_workers=min(BATCH_DOWNLOAD_WORKERS, len(urls))) as ex:
         return list(ex.map(head, urls))
 
 
@@ -972,28 +970,16 @@ def _prefetch_wave(
             ),
         )
         return
-    progress = BatchDownloadProgress("Downloading libraries", sum(sizes))
-    # Reported after the bar is done so the warnings do not land on its
-    # row; list.append is atomic under the GIL.
-    failures: list[tuple[str, Exception]] = []
 
-    def _fetch(component: ConvertedLibrary) -> None:
-        tracker = progress.tracker()
-        try:
-            component.download(salt=salt, namespace=namespace, progress=tracker)
-        except Exception as err:  # noqa: BLE001  # pylint: disable=broad-exception-caught
-            failures.append((component.name, err))
-            tracker(0)
+    def _fetch(component: ConvertedLibrary):
+        return lambda tracker: component.download(
+            salt=salt, namespace=namespace, progress=tracker
+        )
 
-    ex = ThreadPoolExecutor(max_workers=min(_DOWNLOAD_WORKERS, len(components)))
-    try:
-        for future in [ex.submit(_fetch, component) for component in components]:
-            future.result()
-    finally:
-        # On Ctrl-C drop the queued archives instead of downloading them
-        # all before the process can exit; in-flight ones still finish.
-        ex.shutdown(wait=True, cancel_futures=True)
-        progress.done()
+    failures = run_batch_downloads(
+        BatchDownloadProgress("Downloading libraries", sum(sizes)),
+        [(component.name, _fetch(component)) for component in components],
+    )
     for name, err in failures:
         # The sequential call below retries and raises the real error
         _LOGGER.warning("Prefetch of %s failed (retrying sequentially): %s", name, err)

tests/unit_tests/build_helpers/test_idedata.py

@@ -151,6 +151,40 @@ def test_is_esphome_src_handles_backslash_paths() -> None:
     assert not idedata._is_esphome_src(r"C:\b\src\esphome\core\app.h")
 
 
+def test_idedata_from_build_dedupes_identical_command_shapes(
+    tmp_path: Path,
+) -> None:
+    """Translation units sharing one ninja rule (same command modulo
+    file/output) carry
+    identical includes, so only one per shape is tokenized; a differing
+    shape still contributes its includes."""
+
+    def _tu(name: str, inc: str) -> dict:
+        # ninja's compdb embeds the file and output strings verbatim
+        file = f"{ABS}build/src/esphome/core/{name}.cpp"
+        return _entry(
+            f"{ABS}build", file, f"/tools/g++ -I{ABS}inc/{inc} -c {file} -o {name}.o"
+        ) | {"output": f"{name}.o"}
+
+    entries = [_tu(name, "shared") for name in ("application", "component", "helpers")]
+    entries.append(_tu("extra", "extra"))
+    compile_commands = tmp_path / "compile_commands.json"
+    compile_commands.write_text(json.dumps(entries))
+    with (
+        patch.object(idedata, "get_toolchain_includes", return_value=[]),
+        patch.object(idedata, "parse_entry", wraps=idedata.parse_entry) as spy,
+    ):
+        data = idedata.idedata_from_build(compile_commands)
+    includes = set(data["includes"]["build"])
+    assert f"{ABS}inc/shared".replace("\\", "/") in {
+        i.replace("\\", "/") for i in includes
+    }
+    assert any("inc/extra" in i for i in includes)
+    # Representative + one distinct shape; the two same-shape duplicates
+    # are never tokenized
+    assert spy.call_count == 2
+
+
 def test_idedata_from_build(tmp_path: Path) -> None:
     """Full transform: representative entry + include union + toolchain dirs."""
     compile_commands = tmp_path / "compile_commands.json"

tests/unit_tests/test_espidf_framework.py

@@ -989,7 +989,7 @@ def test_prefetch_downloads_archives_concurrently(tmp_path: Path) -> None:
         patch("esphome.espidf.framework.download_with_resume") as download,
         patch("esphome.espidf.framework.get_system_python_path", return_value="python"),
         patch(
-            "esphome.espidf.framework.ThreadPoolExecutor", wraps=ThreadPoolExecutor
+            "esphome.framework_helpers.ThreadPoolExecutor", wraps=ThreadPoolExecutor
         ) as pool,
     ):
         _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None)
@@ -1008,7 +1008,7 @@ def test_prefetch_single_archive_uses_one_worker(tmp_path: Path) -> None:
         patch("esphome.espidf.framework.download_with_resume") as download,
         patch("esphome.espidf.framework.get_system_python_path", return_value="python"),
         patch(
-            "esphome.espidf.framework.ThreadPoolExecutor", wraps=ThreadPoolExecutor
+            "esphome.framework_helpers.ThreadPoolExecutor", wraps=ThreadPoolExecutor
         ) as pool,
     ):
         _prefetch_idf_tool_archives(tmp_path, "esp32", ["required"], None)
@@ -1108,7 +1108,7 @@ def test_prefetch_finishes_progress_bar_and_cancels_queue(tmp_path: Path) -> Non
         patch("esphome.espidf.framework.get_system_python_path", return_value="python"),
         patch("esphome.espidf.framework.BatchDownloadProgress") as progress_cls,
         patch(
-            "esphome.espidf.framework.ThreadPoolExecutor", wraps=ThreadPoolExecutor
+            "esphome.framework_helpers.ThreadPoolExecutor", wraps=ThreadPoolExecutor
         ) as pool_cls,
     ):
         pool = MagicMock(wraps=ThreadPoolExecutor(max_workers=2))