Merge branch 'esp8266-native-toolchain-plumbing' into esp8266-native-framework-installer

esphome/build_helpers/idedata.py

@@ -1,16 +1,17 @@
-"""Derive idedata from an ESP-IDF native-toolchain ``compile_commands.json``.
+"""Derive idedata from a native (non-PlatformIO) build's ``compile_commands.json``.
 
-PlatformIO exposes a curated ``pio run -t idedata`` JSON; the native ESP-IDF
-toolchain has no such command, but its CMake build emits
-``build/compile_commands.json`` (CMAKE_EXPORT_COMPILE_COMMANDS). This module
-turns that file into the same fields consumers (IDE integration, clang-tidy)
-expect:
+PlatformIO exposes a curated ``pio run -t idedata`` JSON; the native
+toolchains have no such command, but each build produces a
+``compile_commands.json`` (CMAKE_EXPORT_COMPILE_COMMANDS for ESP-IDF, ninja's
+compdb tool otherwise). This module turns that file into the same fields
+consumers (IDE integration, clang-tidy) expect:
 
     {cc_path, cxx_path, cxx_flags, defines, includes: {build, toolchain}}
 """
 
 from __future__ import annotations
 
+import functools
 import json
 import logging
 import os
@@ -123,7 +124,20 @@ def _pick_entry(entries: list[dict]) -> dict:
 
 # The compiler basename a compile_commands entry must lead with (an
 # optional target-triple prefix ends in one of these)
-_COMPILER_STEM = re.compile(r"(?:gcc|g\+\+|cc|c\+\+|clang|clang\+\+)$")
+# The stem must BE a compiler name (optionally versioned), alone or after a
+# target-triple separator: "cc", "xtensa-lx106-elf-g++", "gcc-8.4.0" match;
+# "ccache" and "distcc" do not.
+_COMPILER_STEM = re.compile(
+    r"(?:^|[-_.])(?:gcc|g\+\+|cc|c\+\+|clang|clang\+\+)(?:-[\d.]+)?$"
+)
+
+
+@functools.cache
+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.
+    _LOGGER.warning("compile_commands entry does not start with a compiler: %s", token)
 
 
 def parse_entry(
@@ -150,12 +164,7 @@ def parse_entry(
     if launcher is not None and tokens[0] == launcher:
         tokens = tokens[1:]
     if not _COMPILER_STEM.search(Path(tokens[0]).stem):
-        # A stale compile DB built with a launcher the current run no longer
-        # configures would otherwise cache the launcher as the compiler path
-        _LOGGER.warning(
-            "compile_commands entry does not start with a compiler: %s",
-            tokens[0],
-        )
+        _warn_not_a_compiler(tokens[0])
     # token0 is the compiler path; the rest of the command already uses forward
     # slashes on Windows, so normalize it too for a consistent idedata file.
     cxx_path = tokens[0].replace("\\", "/")

esphome/components/esp32/__init__.py

@@ -1083,11 +1083,7 @@ def _resolve_toolchain(value: ConfigType) -> ConfigType:
     # CORE.toolchain instead of re-resolving it from the config dict.
     if CORE.toolchain is None:
         CORE.toolchain = value.get(CONF_TOOLCHAIN, Toolchain.ESP_IDF)
-    if CORE.toolchain not in (Toolchain.PLATFORMIO, Toolchain.ESP_IDF):
-        raise cv.Invalid(
-            f"Unsupported toolchain '{CORE.toolchain.value}' for ESP32. "
-            "Supported toolchains are 'platformio' and 'esp-idf'."
-        )
+    cv.check_supported_toolchain("ESP32", (Toolchain.PLATFORMIO, Toolchain.ESP_IDF))
     return value
 
 

esphome/components/host/__init__.py

@@ -36,8 +36,8 @@ CONFIG_SCHEMA = cv.All(
             cv.Optional(CONF_MAC_ADDRESS, default="98:35:69:ab:f6:79"): cv.mac_address,
         }
     ),
-    set_core_data,
     cv.require_platformio_toolchain("host"),
+    set_core_data,
 )
 
 

esphome/components/nrf52/__init__.py

@@ -128,11 +128,7 @@ def set_core_data(config: ConfigType) -> ConfigType:
 def _resolve_toolchain(config: ConfigType) -> ConfigType:
     if CORE.toolchain is None:
         CORE.toolchain = config.get(CONF_TOOLCHAIN, Toolchain.SDK_NRF)
-    if CORE.toolchain not in (Toolchain.PLATFORMIO, Toolchain.SDK_NRF):
-        raise cv.Invalid(
-            f"Unsupported toolchain '{CORE.toolchain.value}' for nRF52. "
-            "Supported toolchains are 'platformio' and 'sdk-nrf'."
-        )
+    cv.check_supported_toolchain("nRF52", (Toolchain.PLATFORMIO, Toolchain.SDK_NRF))
     return config
 
 

esphome/components/rp2/__init__.py

@@ -312,8 +312,8 @@ CONFIG_SCHEMA = cv.All(
     ),
     cv.has_at_least_one_key(CONF_BOARD, CONF_VARIANT),
     _detect_variant,
-    set_core_data,
     cv.require_platformio_toolchain("RP2"),
+    set_core_data,
 )
 
 

esphome/config_validation.py

@@ -75,6 +75,7 @@ from esphome.const import (
     TYPE_GIT,
     TYPE_LOCAL,
     Framework,
+    Toolchain,
     __version__ as ESPHOME_VERSION,
 )
 from esphome.core import (
@@ -2532,6 +2533,21 @@ def platformio_version_constraint(value):
     return constraints
 
 
+def check_supported_toolchain(platform_name: str, supported: tuple) -> None:
+    """Raise when the resolved ``CORE.toolchain`` is not in ``supported``.
+
+    One message shape for every platform, so a ``--toolchain`` a platform
+    cannot serve always fails by name instead of silently building with a
+    different backend.
+    """
+    if CORE.toolchain not in supported:
+        names = ", ".join(f"'{tc.value}'" for tc in supported)
+        raise Invalid(
+            f"Unsupported toolchain '{CORE.toolchain.value}' for "
+            f"{platform_name}. Supported: {names}."
+        )
+
+
 def require_platformio_toolchain(platform_name: str):
     """Reject a CLI-selected toolchain other than PlatformIO.
 
@@ -2540,15 +2556,9 @@ def require_platformio_toolchain(platform_name: str):
     """
 
     def validator(config):
-        from esphome.const import Toolchain
-
         if CORE.toolchain is None:
             CORE.toolchain = Toolchain.PLATFORMIO
-        if CORE.toolchain != Toolchain.PLATFORMIO:
-            raise Invalid(
-                f"Unsupported toolchain '{CORE.toolchain.value}' for "
-                f"{platform_name}. The only supported toolchain is 'platformio'."
-            )
+        check_supported_toolchain(platform_name, (Toolchain.PLATFORMIO,))
         return config
 
     return validator

esphome/core/config.py

@@ -557,9 +557,9 @@ def _add_library_str(lib: str) -> None:
 
 @coroutine_with_priority(CoroPriority.FINAL)
 async def _add_platformio_options(pio_options: dict[str, str | list[str]]) -> None:
-    if CORE.using_toolchain_esp_idf or (
-        CORE.using_toolchain_arduino and CORE.is_esp8266
-    ):
+    # Every platform's toolchain validation rejects values it cannot serve,
+    # so using_toolchain_arduino by itself implies the native ESP8266 build.
+    if CORE.using_toolchain_esp_idf or CORE.using_toolchain_arduino:
         # The native builds don't read platformio.ini; honor the options
         # with a native equivalent and warn about the rest, which would
         # otherwise be silently ignored.

tests/script/test_determine_jobs.py

@@ -1120,7 +1120,13 @@ def test_should_run_esp32_platformio_with_branch() -> None:
         (["esphome/espidf/runner.py"], True),
         (["esphome/espidf/framework.py"], True),
         (["esphome/build_gen/espidf.py"], True),
-        # PlatformIO build gen and esp32 component are NOT IDF-infra triggers
+        # Shared native-build modules the IDF build imports -> trigger
+        (["esphome/build_helpers/idedata.py"], True),
+        (["esphome/platformio/library.py"], True),
+        (["esphome/platformio/extra_script.py"], True),
+        # PlatformIO build gen, its toolchain, and the esp32 component are
+        # NOT IDF-infra triggers
+        (["esphome/platformio/toolchain.py"], False),
         (["esphome/build_gen/platformio.py"], False),
         (["esphome/components/esp32/__init__.py"], False),
         (["README.md"], False),

tests/unit_tests/build_helpers/test_idedata.py

@@ -21,7 +21,7 @@ def _entry(directory: str, file: str, command: str) -> dict:
     return {"directory": directory, "file": file, "command": command}
 
 
-def testparse_entry_extracts_fields() -> None:
+def test_parse_entry_extracts_fields() -> None:
     """cxx_path, defines, includes and remaining flags are split apart."""
     entry = _entry(
         f"{ABS}build",
@@ -45,7 +45,7 @@ def testparse_entry_extracts_fields() -> None:
     assert "app.cpp.o" not in cxx_flags
 
 
-def testparse_entry_space_separated_args() -> None:
+def test_parse_entry_space_separated_args() -> None:
     """``-D X`` / ``-I path`` (separate arg) and ``-isystem<path>`` (joined)."""
     entry = _entry(
         f"{ABS}build",
@@ -60,7 +60,7 @@ def testparse_entry_space_separated_args() -> None:
     assert f"{ABS}sys/joined" in includes
 
 
-def testparse_entry_resolves_relative_includes() -> None:
+def test_parse_entry_resolves_relative_includes() -> None:
     """Relative includes are resolved against the entry's ``directory``."""
     directory = f"{ABS}build/proj"
     entry = _entry(
@@ -83,7 +83,7 @@ def testparse_entry_resolves_relative_includes() -> None:
     assert all(Path(inc).is_absolute() for inc in includes)
 
 
-def testparse_entry_skips_dependency_flags() -> None:
+def test_parse_entry_skips_dependency_flags() -> None:
     """Dependency-generation flags (and their args) are dropped."""
     entry = _entry(
         "/build",
@@ -198,7 +198,7 @@ def test_idedata_from_build(tmp_path: Path) -> None:
     assert data["includes"]["toolchain"] == ["/tc/inc/c++", "/tc/inc"]
 
 
-def testget_toolchain_includes_raises_on_probe_failure() -> None:
+def test_get_toolchain_includes_raises_on_probe_failure() -> None:
     """A failed compiler probe is a hard error, not a silent empty list."""
     fake_proc = MagicMock(returncode=1, stderr="xtensa-esp32-elf-g++: not found")
     with (
@@ -208,7 +208,7 @@ def testget_toolchain_includes_raises_on_probe_failure() -> None:
         idedata.get_toolchain_includes("/bad/compiler")
 
 
-def testget_toolchain_includes_raises_when_no_dirs_found() -> None:
+def test_get_toolchain_includes_raises_when_no_dirs_found() -> None:
     """Markers present but no dirs (anomalous output) also raises."""
     fake_proc = MagicMock(
         returncode=0,
@@ -248,7 +248,7 @@ def test_split_command_empty_returns_empty() -> None:
 
 
 @pytest.mark.skipif(os.name != "nt", reason="Windows argv tokenization")
-def testparse_entry_normalizes_windows_cxx_path() -> None:
+def test_parse_entry_normalizes_windows_cxx_path() -> None:
     """A backslash compiler path is emitted forward-slashed; define unescaped."""
     entry = _entry(
         r"C:\b",
@@ -264,7 +264,7 @@ def testparse_entry_normalizes_windows_cxx_path() -> None:
     assert "C:/inc/a" in includes
 
 
-def testparse_entry_strips_launcher_prefix() -> None:
+def test_parse_entry_strips_launcher_prefix() -> None:
     """A launcher-wrapped compile names the compiler second; the exact
     configured launcher is stripped, not anything ccache-shaped."""
     entry = _entry(
@@ -279,14 +279,17 @@ def testparse_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
+    # 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 testparse_entry_warns_when_first_token_is_not_a_compiler(
+def test_parse_entry_warns_when_first_token_is_not_a_compiler(
     caplog: pytest.LogCaptureFixture,
 ) -> None:
+    idedata._warn_not_a_compiler.cache_clear()
     entry = _entry(
         f"{ABS}build",
         f"{ABS}build/src/esphome/core/application.cpp",
@@ -391,3 +394,11 @@ def test_load_or_build_idedata_rebuilds_non_dict_cache(tmp_path: Path) -> None:
             )
         assert isinstance(data, dict)
         assert "cc_path" in data
+
+
+def test_parse_entry_accepts_versioned_compilers() -> None:
+    """Versioned compiler names (g++-13, gcc-8.4.0) are not warned about."""
+    for stem in ("g++-13", "gcc-8.4.0", "clang++-17"):
+        assert idedata._COMPILER_STEM.search(stem)
+    assert not idedata._COMPILER_STEM.search("ccache")
+    assert not idedata._COMPILER_STEM.search("distcc")