diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 5bf2a4b5ec..86391e688b 100644 --- a/esphome/build_helpers/idedata.py +++ b/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("\\", "/") diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index a8a5abab25..201c69a8c0 100644 --- a/esphome/components/esp32/__init__.py +++ b/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 diff --git a/esphome/components/host/__init__.py b/esphome/components/host/__init__.py index 6f0fcb9d52..1717870238 100644 --- a/esphome/components/host/__init__.py +++ b/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, ) diff --git a/esphome/components/nrf52/__init__.py b/esphome/components/nrf52/__init__.py index 2328d444f7..fe0c1a12de 100644 --- a/esphome/components/nrf52/__init__.py +++ b/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 diff --git a/esphome/components/rp2/__init__.py b/esphome/components/rp2/__init__.py index 14806b26ce..dae7df26c3 100644 --- a/esphome/components/rp2/__init__.py +++ b/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, ) diff --git a/esphome/config_validation.py b/esphome/config_validation.py index c3ee760761..009b844986 100644 --- a/esphome/config_validation.py +++ b/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 diff --git a/esphome/core/config.py b/esphome/core/config.py index be577cccbb..9fef173b48 100644 --- a/esphome/core/config.py +++ b/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. diff --git a/tests/script/test_determine_jobs.py b/tests/script/test_determine_jobs.py index 80f572d9fe..297752b3ac 100644 --- a/tests/script/test_determine_jobs.py +++ b/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), diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index eca74ded35..e65802dd74 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/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`` (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")