Merge branch 'esp8266-native-build-spec' into esp8266-native-ninja-emission

esphome/build_helpers/idedata.py

@@ -11,7 +11,6 @@ consumers (IDE integration, clang-tidy) expect:
 
 from __future__ import annotations
 
-import functools
 import json
 import logging
 import os
@@ -20,6 +19,8 @@ import re
 import shlex
 import subprocess
 
+from esphome.helpers import write_file
+
 _LOGGER = logging.getLogger(__name__)
 
 # C++ translation-unit suffixes used to identify ESPHome source files.
@@ -132,11 +133,16 @@ _COMPILER_STEM = re.compile(
 )
 
 
-@functools.cache
+# Deduplicates the warning across a compile DB of hundreds of entries;
+# cleared per idedata build (a module-level functools.cache would suppress
+# the warning for the process lifetime in non-forking hosts).
+_warned_not_a_compiler: set[str] = set()
+
+
 def _warn_not_a_compiler(token: str) -> None:
-    # A stale compile DB built with a launcher the current run no longer
-    # configures would otherwise cache the launcher as the compiler path.
-    # Cached so a database of hundreds of entries warns once per path.
+    if token in _warned_not_a_compiler:
+        return
+    _warned_not_a_compiler.add(token)
     _LOGGER.warning("compile_commands entry does not start with a compiler: %s", token)
 
 
@@ -163,6 +169,14 @@ def parse_entry(
     # build, so this is a comparison, not a guess by name.
     if launcher is not None and tokens[0] == launcher:
         tokens = tokens[1:]
+    if (
+        not _COMPILER_STEM.search(Path(tokens[0]).stem)
+        and len(tokens) > 1
+        and _COMPILER_STEM.search(Path(tokens[1]).stem)
+    ):
+        # A stale compile DB built with a launcher the current run no longer
+        # configures: the real compiler is the next token.
+        tokens = tokens[1:]
     if not _COMPILER_STEM.search(Path(tokens[0]).stem):
         _warn_not_a_compiler(tokens[0])
     # token0 is the compiler path; the rest of the command already uses forward
@@ -268,8 +282,10 @@ def load_or_build_idedata(
     if cache.is_file() and cache.stat().st_mtime >= compile_commands.stat().st_mtime:
         try:
             cached = json.loads(cache.read_text(encoding="utf-8"))
-        except ValueError:
-            pass
+        except ValueError as err:
+            # A recurring cause (interrupted write, disk full) would otherwise
+            # look like unexplained slow builds
+            _LOGGER.warning("Discarding unreadable idedata cache %s: %s", cache, err)
         else:
             # Caches written before cc_path was emitted stay newer than
             # compile_commands.json forever, so rebuild them on the field rather
@@ -280,29 +296,48 @@ def load_or_build_idedata(
 
     data = idedata_from_build(compile_commands, launcher)
     data["prog_path"] = str(elf_path)
-    cache.parent.mkdir(parents=True, exist_ok=True)
-    cache.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8")
+    if _COMPILER_STEM.search(Path(data["cxx_path"]).stem):
+        cache.parent.mkdir(parents=True, exist_ok=True)
+        # Atomic so a crash mid-write cannot leave a truncated cache
+        write_file(cache, json.dumps(data, indent=2) + "\n")
+    else:
+        # parse_entry already warned; serve the data for this run but never
+        # persist a known-bad compiler path (the cache would outlive the
+        # timestamp check and hide the fault)
+        _LOGGER.debug("Not caching idedata with unrecognized compiler path")
     return data
 
 
 def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> dict:
     """Parse compile_commands.json into the idedata fields consumers expect.
 
-    A single ESP-IDF compile entry only carries its own component's REQUIRES
-    include set, but consumers (clang-tidy) analyze ESPHome headers that
-    transitively pull in other components. So take cxx_path / cxx_flags /
-    defines from a representative ESPHome TU, but union the include dirs across
-    all ESPHome TUs to get a project-wide superset (as PlatformIO's idedata
-    provides).
+    A single compile entry only carries the include set its own translation
+    unit was built with (per-component under ESP-IDF), but consumers
+    (clang-tidy) analyze ESPHome headers that transitively pull in other
+    components. So take cxx_path / cxx_flags / defines from a representative
+    ESPHome TU, but union the include dirs across all ESPHome TUs to get a
+    project-wide superset (as PlatformIO's idedata provides).
     """
+    _warned_not_a_compiler.clear()
     entries = json.loads(Path(compile_commands).read_text(encoding="utf-8"))
-    cxx_path, defines, _, cxx_flags = parse_entry(_pick_entry(entries), launcher)
+
+    # ninja-generated compile DBs repeat one identical multi-KB command per
+    # source file; parse each distinct (directory, command) once.
+    parsed: dict[tuple[str, str], tuple[str, list[str], list[str], list[str]]] = {}
+
+    def _parse(entry: dict) -> tuple[str, list[str], list[str], list[str]]:
+        key = (entry["directory"], entry["command"])
+        if key not in parsed:
+            parsed[key] = parse_entry(entry, launcher)
+        return parsed[key]
+
+    cxx_path, defines, _, cxx_flags = _parse(_pick_entry(entries))
 
     build_includes: dict[str, None] = {}
     for entry in entries:
         if not _is_esphome_src(entry["file"]):
             continue
-        for inc in parse_entry(entry, launcher)[2]:
+        for inc in _parse(entry)[2]:
             build_includes.setdefault(inc, None)
 
     return {

esphome/espidf/component.py

@@ -41,17 +41,6 @@ def _idf_framework() -> str:
     return "arduino" if CORE.using_arduino else "espidf"
 
 
-def _apply_extra_script(component: IDFComponent) -> None:
-    from esphome.components.esp32 import get_esp32_variant
-    from esphome.platformio.extra_script import apply_extra_script
-
-    apply_extra_script(
-        component,
-        board_mcu=lambda: variant_to_idf_target(get_esp32_variant()),
-        pio_platform="espressif32",
-    )
-
-
 def generate_cmakelists_txt(component: IDFComponent) -> str:
     """
     Generate a CMakeLists.txt file for an ESP-IDF component.
@@ -264,7 +253,14 @@ def generate_idf_component_yml(component: IDFComponent) -> str:
 
 def _emit_idf_component(component: IDFComponent) -> None:
     """Write the ESP-IDF build files for a resolved library into its cache dir."""
-    _apply_extra_script(component)
+    from esphome.components.esp32 import get_esp32_variant
+    from esphome.platformio.extra_script import apply_extra_script
+
+    apply_extra_script(
+        component,
+        board_mcu=lambda: variant_to_idf_target(get_esp32_variant()),
+        pio_platform="espressif32",
+    )
     write_file_if_changed(
         component.path / "CMakeLists.txt",
         generate_cmakelists_txt(component),

esphome/platformio/extra_script.py

@@ -45,15 +45,15 @@ _LOGGER = logging.getLogger(__name__)
 
 def apply_extra_script(
     component: ConvertedLibrary,
-    board_mcu: str | Callable[[], str],
+    board_mcu: Callable[[], str],
     pio_platform: str,
 ) -> None:
     """Run a library's PIO ``extraScript`` and fold its captured env vars into
     ``component.data["build"]["flags"]`` so the backend's -L/-l/-D extraction
     picks them up. Shared by the ESP-IDF and ESP8266 Arduino backends.
 
-    ``board_mcu`` may be a callable so a backend whose target lookup needs
-    build state (the esp32 variant) resolves it only when a script will run.
+    ``board_mcu`` is a callable so a backend whose target lookup needs build
+    state (the esp32 variant) resolves it only when a script will run.
     ``pio_platform`` is exposed to the script as PlatformIO's ``PIOPLATFORM``.
     """
     extra_script = component.data.get("build", {}).get("extraScript")
@@ -80,12 +80,10 @@ def apply_extra_script(
             component.name,
         )
         return
-    if callable(board_mcu):
-        board_mcu = board_mcu()
     result = run_extra_script(
         script_path,
         library_dir=source_path,
-        board_mcu=board_mcu,
+        board_mcu=board_mcu(),
         pio_platform=pio_platform,
     )
     extra_flags = captured_as_build_flags(result, library_dir=source_path)

tests/unit_tests/build_helpers/test_idedata.py

@@ -278,30 +278,39 @@ def test_parse_entry_strips_launcher_prefix() -> None:
     )
     assert cxx_path == "/tools/xtensa-lx106-elf-g++"
     assert defines == ["USE_ESP8266"]
-    # Without a configured launcher nothing is stripped, even a token that
-    # happens to be named ccache -- but the surprise is warned about (once
-    # per path, however many entries the compile DB has)
-    idedata._warn_not_a_compiler.cache_clear()
-    cxx_path, _, _, _ = idedata.parse_entry(entry)
-    assert cxx_path == "/opt/homebrew/bin/ccache"
 
 
-def test_parse_entry_warns_when_first_token_is_not_a_compiler(
+def test_parse_entry_recovers_from_unconfigured_launcher(
     caplog: pytest.LogCaptureFixture,
 ) -> None:
-    idedata._warn_not_a_compiler.cache_clear()
+    """A stale compile DB built with a launcher this run no longer configures
+    still yields the real compiler (the next token), not the launcher."""
     entry = _entry(
         f"{ABS}build",
         f"{ABS}build/src/esphome/core/application.cpp",
         "/opt/homebrew/bin/ccache /tools/xtensa-lx106-elf-g++ -c a.cpp -o a.o",
     )
-    idedata.parse_entry(entry)
-    assert "does not start with a compiler" in caplog.text
-    caplog.clear()
-    idedata.parse_entry(entry, launcher="/opt/homebrew/bin/ccache")
+    cxx_path, _, _, _ = idedata.parse_entry(entry)
+    assert cxx_path == "/tools/xtensa-lx106-elf-g++"
     assert "does not start with a compiler" not in caplog.text
 
 
+def test_parse_entry_warns_when_first_token_is_not_a_compiler(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """An unrecoverable non-compiler leading token is warned about, once per
+    path however many entries the compile DB has."""
+    idedata._warned_not_a_compiler.clear()
+    entry = _entry(
+        f"{ABS}build",
+        f"{ABS}build/src/esphome/core/application.cpp",
+        "/usr/bin/python3 wrapper.py -c a.cpp -o a.o",
+    )
+    idedata.parse_entry(entry)
+    idedata.parse_entry(entry)
+    assert caplog.text.count("does not start with a compiler") == 1
+
+
 def _write_compile_commands(tmp_path: Path) -> Path:
     compile_commands = tmp_path / "compile_commands.json"
     compile_commands.write_text(
@@ -402,3 +411,43 @@ def test_parse_entry_accepts_versioned_compilers() -> None:
         assert idedata._COMPILER_STEM.search(stem)
     assert not idedata._COMPILER_STEM.search("ccache")
     assert not idedata._COMPILER_STEM.search("distcc")
+
+
+def test_load_or_build_idedata_corrupted_cache_is_logged(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """A truncated cache is diagnosable, not a silent slow-build cause."""
+    compile_commands = _write_compile_commands(tmp_path)
+    cache = tmp_path / "c.json"
+    cache.write_text('{"cc_path": trunc')
+    os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 2)
+    with patch.object(idedata, "get_toolchain_includes", return_value=[]):
+        data = idedata.load_or_build_idedata(
+            compile_commands, tmp_path / "f.elf", cache
+        )
+    assert data["cxx_path"] == "/tools/g++"
+    assert "Discarding unreadable idedata cache" in caplog.text
+
+
+def test_load_or_build_idedata_never_caches_bad_compiler(tmp_path: Path) -> None:
+    """Idedata whose compiler path failed the sanity check is served for this
+    run but not persisted, so the next build re-parses."""
+    compile_commands = tmp_path / "compile_commands.json"
+    compile_commands.write_text(
+        json.dumps(
+            [
+                _entry(
+                    f"{ABS}build",
+                    f"{ABS}build/src/esphome/core/application.cpp",
+                    "/usr/bin/python3 wrapper.py -c app.cpp -o app.cpp.o",
+                )
+            ]
+        )
+    )
+    cache = tmp_path / "c.json"
+    with patch.object(idedata, "get_toolchain_includes", return_value=[]):
+        data = idedata.load_or_build_idedata(
+            compile_commands, tmp_path / "f.elf", cache
+        )
+    assert data["cxx_path"] == "/usr/bin/python3"
+    assert not cache.exists()

tests/unit_tests/test_espidf_component.py

@@ -1069,10 +1069,11 @@ def test_idf_component_download_passes_salt() -> None:
     assert c.path == Path("/converted/owner/name")
 
 
-def test_apply_extra_script_wrapper_wires_esp32_target(tmp_path, monkeypatch):
-    """The espidf wrapper resolves the esp32 variant into the shared helper."""
+def test_emit_idf_component_wires_esp32_target(tmp_path, monkeypatch):
+    """Emitting a component resolves the esp32 variant into the shared
+    extraScript helper."""
     from esphome.components import esp32 as esp32_module
-    from esphome.espidf.component import _apply_extra_script
+    from esphome.espidf.component import _emit_idf_component
 
     monkeypatch.setattr(esp32_module, "get_esp32_variant", lambda: "ESP32")
     (tmp_path / "src").mkdir()
@@ -1081,5 +1082,5 @@ def test_apply_extra_script_wrapper_wires_esp32_target(tmp_path, monkeypatch):
     c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
     c.path = tmp_path
     c.data = {"build": {"extraScript": "extra.py"}}
-    _apply_extra_script(c)
+    _emit_idf_component(c)
     assert c.data["build"]["flags"] == ["-lesp32"]

tests/unit_tests/test_platformio_extra_script.py

@@ -117,7 +117,7 @@ def test_apply_extra_script_path_traversal_is_rejected(tmp_path):
     c.path = library_dir
     c.data = {"build": {"extraScript": "../evil.py"}}
 
-    apply_extra_script(c, board_mcu="esp32", pio_platform="espressif32")
+    apply_extra_script(c, board_mcu=lambda: "esp32", pio_platform="espressif32")
 
     # Nothing was folded into flags: the traversal was rejected before
     # the script could run.
@@ -135,14 +135,14 @@ def test_apply_extra_script_merges_into_existing_flags(tmp_path):
     c.path = tmp_path
     c.data = {"build": {"extraScript": "extra.py", "flags": ["-DEXISTING"]}}
 
-    apply_extra_script(c, board_mcu="esp32", pio_platform="espressif32")
+    apply_extra_script(c, board_mcu=lambda: "esp32", pio_platform="espressif32")
 
     assert "-DEXISTING" in c.data["build"]["flags"]
     assert "-lalgobsec" in c.data["build"]["flags"]
 
 
 def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None:
-    """The shared helper resolves a callable board_mcu lazily and normalizes
+    """The shared helper resolves the board_mcu callable lazily and normalizes
     a string ``build.flags`` value into a list before extending it."""
     from esphome.platformio.extra_script import apply_extra_script
 
@@ -176,7 +176,7 @@ def test_apply_extra_script_no_script_and_no_flags(tmp_path) -> None:
     script = tmp_path / "noop.py"
     script.write_text("pass\n")
     c.data = {"build": {"extraScript": "noop.py"}}
-    apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266")
+    apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266")
     assert "flags" not in c.data["build"]
 
 
@@ -191,7 +191,7 @@ def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path) -> None:
     c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
     c.path = tmp_path
     c.data = {"build": {"extraScript": "extra.py"}}
-    apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266")
+    apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266")
     assert c.data["build"]["flags"] == ["-lsingle"]
 
 
@@ -204,7 +204,7 @@ def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None:
     c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
     c.path = tmp_path
     c.data = {"build": {"extraScript": "extra.py"}}
-    apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266")
+    apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266")
     assert "flags" not in c.data["build"]
     assert "skipping" in caplog.text
 
@@ -218,7 +218,7 @@ def test_apply_extra_script_pio_platform(tmp_path) -> None:
     c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
     c.path = tmp_path
     c.data = {"build": {"extraScript": "extra.py"}}
-    apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266")
+    apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266")
     assert c.data["build"]["flags"] == ["-lespressif8266"]
 
 
@@ -230,5 +230,5 @@ def test_apply_extra_script_missing_script_logged(tmp_path, caplog) -> None:
     c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
     c.path = tmp_path
     c.data = {"build": {"extraScript": "nope.py"}}
-    apply_extra_script(c, board_mcu="esp8266", pio_platform="espressif8266")
+    apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266")
     assert "not found" in caplog.text