diff --git a/esphome/__main__.py b/esphome/__main__.py index e5f8110bc6..25ff986b0b 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -865,11 +865,8 @@ def compile_program(args: ArgsProtocol, config: ConfigType) -> int: RuntimeError, ValueError, ) as err: - # The firmware already built; idedata is a bonus artifact here. - # Broad on purpose: a vanished compiler (OSError), a failed - # include probe (RuntimeError), or a truncated compile DB - # (ValueError/LookupError) must not fail a successful build - # either. + # Broad on purpose: the firmware already built; an idedata + # failure must not fail a successful build. _LOGGER.warning("Could not generate idedata: %s", err) else: from esphome.platformio import toolchain diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 31b62df528..00a890e087 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -123,10 +123,8 @@ def _pick_entry(entries: list[dict]) -> dict: raise ValueError("no C++ translation unit found in compile_commands.json") -# Compiler launchers that may prefix a compile command. A closed denylist is -# sturdier than trying to enumerate compiler names: launchers are few and -# stable, while compilers (cross prefixes, versioned names, icx, armcc, ...) -# are an open set. +# Compiler launchers that may prefix a compile command; a closed launcher +# denylist beats enumerating compiler names, an open set. _LAUNCHER_STEMS = frozenset({"ccache", "sccache", "distcc", "icecc", "buildcache"}) @@ -300,17 +298,14 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d representative = _pick_entry(entries) cxx_path, defines, rep_includes, cxx_flags = parse_entry(representative, launcher) if _is_launcher(cxx_path): - # Checked before the toolchain probe (which would fail opaquely on - # a launcher) so the unusable compile DB is named, and never - # cached or conflated with "nothing built yet" + # Reject before the toolchain probe, which would fail opaquely on + # a launcher; never cache the unusable compile DB raise EsphomeError( f"compile_commands.json names the launcher {cxx_path} as the " "compiler; the compile database is unusable" ) # Seed with the representative's includes so it is not parsed twice - # (per-file -c/-o arguments make every command distinct, so memoizing - # whole commands would never hit) build_includes: dict[str, None] = dict.fromkeys( rep_includes if _is_esphome_src(representative["file"]) else () ) diff --git a/esphome/build_helpers/size_summary.py b/esphome/build_helpers/size_summary.py index 72c429d095..b888111044 100644 --- a/esphome/build_helpers/size_summary.py +++ b/esphome/build_helpers/size_summary.py @@ -4,11 +4,7 @@ from __future__ import annotations def format_bar(used: int, total: int) -> str: - """Match PlatformIO's ``_format_availale_bytes`` (pioupload.py) exactly. - - The upstream helper's name really is spelled that way; keep the citation - verbatim so it stays greppable in the PlatformIO source. - """ + """Match PlatformIO's ``_format_availale_bytes`` (sic, pioupload.py) exactly.""" pct_raw = used / total if total else 0 blocks = 10 filled = min(int(round(blocks * pct_raw)), blocks) diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index 07c3dc9dde..567cde65e2 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -92,10 +92,8 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: build_flags = ensure_list( component.data.get("build", {}).get("flags", DEFAULT_BUILD_FLAGS) ) - # PlatformIO shell-lexes each build.flags entry, so one entry can carry a - # flag and its argument (e.g. "-include cp_custom_alloc.h"); bare - # -I/-L/-l/-D tokens re-glue to their argument ("-I foo" -> "-Ifoo") so - # prefix classifiers below still route them. + # PlatformIO shell-lexes each build.flags entry; bare -I/-L/-l/-D tokens + # re-glue to their argument so the prefix classifiers below route them. # Joined per entry, as SCons's ParseFlags lexes each string # independently: a dangling -I ending one entry must warn, not absorb # the next entry's first token. diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py index 4a0a0dc7c5..e311e8729b 100644 --- a/esphome/platformio/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -141,9 +141,7 @@ class _FakeSConsEnv: def Append(self, **kwargs) -> None: # noqa: N802 (SCons API name) for key, value in kwargs.items(): if key not in _CAPTURED_KEYS: - # Diagnosable from the build log when a script configures - # something this shim does not translate; once per key so a - # loop of Appends cannot spam + # Warn once per key so a loop of Appends cannot spam if key not in self._warned_keys: self._warned_keys.add(key) _LOGGER.warning( @@ -239,9 +237,8 @@ def run_extra_script( ) return ExtraScriptResult() except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught - # Discard any partial capture: folding half a script's flags into the - # build could produce wrong-output firmware that links cleanly. The - # warning plus the resulting loud link error point back here. + # Discard any partial capture: half-applied flags could build wrong + # firmware that links cleanly. _LOGGER.warning( "PIO extra-script %s (in %s) raised %r; ignoring its output", script_path, diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index e93d9c499f..7ce45a7fca 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -47,11 +47,8 @@ DEFAULT_BUILD_SRC_FILTER = ( DEFAULT_BUILD_SRC_DIRS = "src" DEFAULT_BUILD_INCLUDE_DIR = "include" DEFAULT_BUILD_FLAGS = [] -# Source suffix -> compiler kind, PlatformIO's CSUFFIXES/CXXSUFFIXES/ASSUFFIXES -# split. Native build generators map the kind to their compile rules. "asm" -# deliberately merges SCons's AS (.s/.asm) and ASPP (.S/.spp/.sx) sets: the -# ninja rules compile all of them as assembler-with-cpp, whose asm-mode -# preprocessor passes non-directive text through unchanged. +# Suffix -> compiler kind (PlatformIO's CSUFFIXES/CXXSUFFIXES/ASSUFFIXES). +# "asm" merges SCons's AS and ASPP sets: all compile as assembler-with-cpp. SOURCE_KIND_FOR_SUFFIX: dict[str, str] = { ".c": "c", ".cpp": "cxx", diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index 7297912955..52ffc88224 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -429,8 +429,7 @@ def test_load_or_build_idedata_corrupted_cache_is_logged( def test_load_or_build_idedata_never_caches_a_launcher(tmp_path: Path) -> None: - """A compile DB naming a launcher as the compiler is rejected by name, - before the toolchain probe could fail opaquely, and never cached.""" + """A compile DB naming a launcher as the compiler is rejected, never cached.""" compile_commands = tmp_path / "compile_commands.json" compile_commands.write_text( json.dumps( diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index 1baa22777c..e2884454e5 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -6,6 +6,7 @@ from unittest.mock import MagicMock import pytest +from esphome.components import esp32 as esp32_module from esphome.const import ( KEY_CORE, KEY_TARGET_FRAMEWORK, @@ -15,6 +16,7 @@ from esphome.const import ( ) from esphome.core import CORE, Library from esphome.espidf.component import ( + _emit_idf_component, generate_cmakelists_txt, generate_idf_component_yml, generate_idf_components, @@ -1072,8 +1074,6 @@ def test_idf_component_download_passes_salt() -> None: 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 _emit_idf_component monkeypatch.setattr(esp32_module, "get_esp32_variant", lambda: "ESP32") (tmp_path / "src").mkdir() diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index b0cb5f6a0a..d20832af6b 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -101,6 +101,7 @@ from esphome.const import ( CONF_WEB_SERVER, CONF_WIFI, KEY_CORE, + KEY_TARGET_FRAMEWORK, KEY_TARGET_PLATFORM, PLATFORM_BK72XX, PLATFORM_ESP32, @@ -7139,7 +7140,7 @@ def test_warn_source_tree_mismatch_falls_back_when_stat_fails( RuntimeError("Could not query builtin include dirs"), ValueError("no C++ translation unit found"), KeyError("command"), - None, # replaced with EsphomeError inside (import is function-local) + None, # replaced with EsphomeError inside ], ) def test_compile_program_espidf_idedata_failure_does_not_fail_build( @@ -7147,14 +7148,6 @@ def test_compile_program_espidf_idedata_failure_does_not_fail_build( caplog: pytest.LogCaptureFixture, ) -> None: """A post-compile idedata error is a warning: the firmware already built.""" - from esphome.const import ( - KEY_CORE, - KEY_TARGET_FRAMEWORK, - KEY_TARGET_PLATFORM, - Toolchain, - ) - from esphome.core import CORE, EsphomeError - if error is None: error = EsphomeError("compile database is unusable") CORE.toolchain = Toolchain.ESP_IDF @@ -7178,14 +7171,6 @@ def test_compile_program_espidf_idedata_success_is_silent( caplog: pytest.LogCaptureFixture, ) -> None: """The healthy path: idedata generated, nothing to warn about.""" - from esphome.const import ( - KEY_CORE, - KEY_TARGET_FRAMEWORK, - KEY_TARGET_PLATFORM, - Toolchain, - ) - from esphome.core import CORE - CORE.toolchain = Toolchain.ESP_IDF CORE.data[KEY_CORE] = { KEY_TARGET_PLATFORM: "esp32", @@ -7207,14 +7192,6 @@ def test_compile_program_espidf_idedata_none_warns( caplog: pytest.LogCaptureFixture, ) -> None: """A silent None from the post-compile idedata refresh is made visible.""" - from esphome.const import ( - KEY_CORE, - KEY_TARGET_FRAMEWORK, - KEY_TARGET_PLATFORM, - Toolchain, - ) - from esphome.core import CORE - CORE.toolchain = Toolchain.ESP_IDF CORE.data[KEY_CORE] = { KEY_TARGET_PLATFORM: "esp32", diff --git a/tests/unit_tests/test_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py index 7e904183f1..d09f02d95f 100644 --- a/tests/unit_tests/test_platformio_extra_script.py +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -2,19 +2,25 @@ from __future__ import annotations +import logging import os from pathlib import Path +from unittest.mock import patch import pytest +from esphome.core import EsphomeError +from esphome.platformio.extra_script import ( + ExtraScriptResult, + _FakeSConsEnv, + apply_extra_script, + captured_as_build_flags, + run_extra_script, +) from esphome.platformio.library import ConvertedLibrary as IDFComponent, URLSource def test_extra_script_captures_libpath_libs_and_defines(tmp_path): - from esphome.platformio.extra_script import ( - captured_as_build_flags, - run_extra_script, - ) (tmp_path / "src" / "esp32").mkdir(parents=True) script = tmp_path / "extra_script.py" @@ -58,10 +64,6 @@ def test_extra_script_libpath_relative_resolves_against_library_dir( """Relative LIBPATH entries must resolve against ``library_dir``, not the caller's CWD (the shim restores CWD before ``captured_as_build_flags`` runs).""" - from esphome.platformio.extra_script import ( - ExtraScriptResult, - captured_as_build_flags, - ) (tmp_path / "lib" / "esp32").mkdir(parents=True) elsewhere = tmp_path.parent / "not_the_library_dir" @@ -76,10 +78,6 @@ def test_extra_script_libpath_relative_resolves_against_library_dir( def test_extra_script_libpath_absolute_outside_library_dir(tmp_path): - from esphome.platformio.extra_script import ( - ExtraScriptResult, - captured_as_build_flags, - ) outside = tmp_path.parent / "system_lib" outside.mkdir(exist_ok=True) @@ -90,7 +88,6 @@ def test_extra_script_libpath_absolute_outside_library_dir(tmp_path): def test_extra_script_failure_returns_empty_result(tmp_path, caplog): - from esphome.platformio.extra_script import run_extra_script script = tmp_path / "broken.py" script.write_text("raise RuntimeError('boom')\n") @@ -106,7 +103,6 @@ def test_extra_script_failure_returns_empty_result(tmp_path, caplog): def test_apply_extra_script_path_traversal_is_rejected(tmp_path): - from esphome.platformio.extra_script import apply_extra_script library_dir = tmp_path / "lib" library_dir.mkdir() @@ -117,8 +113,6 @@ def test_apply_extra_script_path_traversal_is_rejected(tmp_path): c.path = library_dir c.data = {"build": {"extraScript": "../evil.py"}} - from esphome.core import EsphomeError - with pytest.raises(EsphomeError, match="escapes the library directory"): apply_extra_script(c, board_mcu=lambda: "esp32", pio_platform="espressif32") # Nothing was folded into flags: the traversal was rejected before @@ -127,7 +121,6 @@ def test_apply_extra_script_path_traversal_is_rejected(tmp_path): def test_apply_extra_script_merges_into_existing_flags(tmp_path): - from esphome.platformio.extra_script import apply_extra_script (tmp_path / "src").mkdir() script = tmp_path / "extra.py" @@ -146,8 +139,6 @@ def test_apply_extra_script_merges_into_existing_flags(tmp_path): def test_apply_extra_script_malformed_flags_raises(tmp_path) -> None: """A null/dict build.flags fails naming the library instead of injecting a non-string into the compiler command line.""" - from esphome.core import EsphomeError - from esphome.platformio.extra_script import apply_extra_script (tmp_path / "src").mkdir() script = tmp_path / "extra.py" @@ -164,7 +155,6 @@ def test_apply_extra_script_malformed_flags_raises(tmp_path) -> None: def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None: """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 (tmp_path / "src").mkdir() script = tmp_path / "extra.py" @@ -180,7 +170,6 @@ def test_apply_extra_script_callable_target_and_str_flags(tmp_path) -> None: def test_apply_extra_script_no_script_and_no_flags(tmp_path) -> None: - from esphome.platformio.extra_script import apply_extra_script # No extraScript declared: nothing happens, the target is never resolved c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) @@ -203,9 +192,6 @@ def test_apply_extra_script_no_script_and_no_flags(tmp_path) -> None: def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path, caplog) -> None: """Un-captured env vars and unsupported env methods are skipped but diagnosable from the build log.""" - import logging - - from esphome.platformio.extra_script import apply_extra_script caplog.set_level(logging.DEBUG) script = tmp_path / "extra.py" @@ -223,7 +209,6 @@ def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path, caplog) -> No def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None: """A raising extra-script is best-effort: logged and skipped.""" - from esphome.platformio.extra_script import apply_extra_script script = tmp_path / "extra.py" script.write_text("raise RuntimeError('boom')\n") @@ -237,7 +222,6 @@ def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None: def test_apply_extra_script_pio_platform(tmp_path) -> None: """The backend's platform token is exposed to the script as PIOPLATFORM.""" - from esphome.platformio.extra_script import apply_extra_script script = tmp_path / "extra.py" script.write_text("env.Append(LIBS=[env.get('PIOPLATFORM')])\n") @@ -251,8 +235,6 @@ def test_apply_extra_script_pio_platform(tmp_path) -> None: def test_apply_extra_script_missing_script_raises(tmp_path) -> None: """A declared but absent extraScript is a broken package and fails by name, as it would under PlatformIO.""" - from esphome.core import EsphomeError - from esphome.platformio.extra_script import apply_extra_script c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) c.path = tmp_path @@ -264,7 +246,6 @@ def test_apply_extra_script_missing_script_raises(tmp_path) -> None: def test_run_extra_script_failure_discards_partial_capture(tmp_path, caplog) -> None: """A crashed script yields an empty result: half-applied flags could build wrong-output firmware that links cleanly.""" - from esphome.platformio.extra_script import run_extra_script script = tmp_path / "extra.py" script.write_text("env.Append(LIBS=['algobsec'])\nraise RuntimeError('boom')\n") @@ -278,7 +259,6 @@ def test_run_extra_script_failure_discards_partial_capture(tmp_path, caplog) -> def test_run_extra_script_syntax_error_is_best_effort(tmp_path, caplog) -> None: """A vendored script that does not even compile warns and skips instead of aborting the build.""" - from esphome.platformio.extra_script import run_extra_script script = tmp_path / "extra.py" script.write_text("def broken(:\n") @@ -291,7 +271,6 @@ def test_run_extra_script_syntax_error_is_best_effort(tmp_path, caplog) -> None: def test_unsupported_env_method_warns_once(caplog) -> None: """Repeated calls to the same unsupported method warn only once.""" - from esphome.platformio.extra_script import _FakeSConsEnv env = _FakeSConsEnv( board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" @@ -304,7 +283,6 @@ def test_unsupported_env_method_warns_once(caplog) -> None: def test_run_extra_script_sys_exit_is_best_effort(tmp_path, caplog) -> None: """A nonzero sys.exit() in a vendored script must not kill the esphome run, and its output is discarded.""" - from esphome.platformio.extra_script import run_extra_script script = tmp_path / "extra.py" script.write_text("import sys\nenv.Append(LIBS=['x'])\nsys.exit(3)\n") @@ -317,7 +295,6 @@ def test_run_extra_script_sys_exit_is_best_effort(tmp_path, caplog) -> None: def test_run_extra_script_sys_exit_zero_is_success(tmp_path, caplog) -> None: """sys.exit(0) is a normal PlatformIO script ending: the capture is kept.""" - from esphome.platformio.extra_script import run_extra_script script = tmp_path / "extra.py" script.write_text("import sys\nenv.Append(LIBS=['algobsec'])\nsys.exit(0)\n") @@ -330,10 +307,6 @@ def test_run_extra_script_sys_exit_zero_is_success(tmp_path, caplog) -> None: def test_run_extra_script_unreadable_raises(tmp_path) -> None: """An unreadable declared script is a broken package, like a missing one.""" - from unittest.mock import patch - - from esphome.core import EsphomeError - from esphome.platformio.extra_script import run_extra_script script = tmp_path / "extra.py" script.write_text("") @@ -348,7 +321,6 @@ def test_run_extra_script_unreadable_raises(tmp_path) -> None: def test_run_extra_script_bad_encoding_is_best_effort(tmp_path, caplog) -> None: """Undecodable content warns and skips, like a SyntaxError.""" - from esphome.platformio.extra_script import run_extra_script script = tmp_path / "extra.py" script.write_bytes(b"\xff\xfe\x00bad") @@ -361,7 +333,6 @@ def test_run_extra_script_bad_encoding_is_best_effort(tmp_path, caplog) -> None: def test_uncaptured_append_key_warns_once(caplog) -> None: """A loop of Appends to the same uncaptured key warns once.""" - from esphome.platformio.extra_script import _FakeSConsEnv env = _FakeSConsEnv( board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 9567ab985f..d050acad9e 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -13,6 +13,7 @@ import pytest from esphome.core import EsphomeError, Library import esphome.platformio.library as lib from esphome.platformio.library import ( + SOURCE_KIND_FOR_SUFFIX, ConvertedLibrary, GitSource, InvalidLibrary, @@ -23,6 +24,8 @@ from esphome.platformio.library import ( _resolve_registry_version, check_library_data, convert_libraries, + join_flag_args, + split_flag_entry, ) @@ -535,7 +538,6 @@ def test_convert_libraries_skips_incompatible_dependency(tmp_path, monkeypatch): def test_split_flag_entry_unbalanced_quote_is_clean() -> None: """A malformed flags entry raises EsphomeError, not a raw ValueError.""" - from esphome.platformio.library import split_flag_entry assert split_flag_entry('-DX="a b"', "library x") == ["-DX=a b"] with pytest.raises(EsphomeError, match=r"Malformed build flag.*library x"): @@ -544,7 +546,6 @@ def test_split_flag_entry_unbalanced_quote_is_clean() -> None: def test_join_flag_args_reglues_spaced_define() -> None: """A spaced -D re-glues to its argument, as ParseFlags does.""" - from esphome.platformio.library import join_flag_args assert join_flag_args(["-D", "FOO=1", "-Os"], "x") == ["-DFOO=1", "-Os"] @@ -552,7 +553,6 @@ def test_join_flag_args_reglues_spaced_define() -> None: def test_join_flag_args_trailing_bare_flag_warns( caplog: pytest.LogCaptureFixture, ) -> None: - from esphome.platformio.library import join_flag_args assert join_flag_args(["-Os", "-l"], "library x") == ["-Os"] assert "Ignoring trailing '-l'" in caplog.text @@ -561,7 +561,6 @@ def test_join_flag_args_trailing_bare_flag_warns( def test_split_flag_entry_non_string_is_clean() -> None: """A dict or number from a third-party manifest fails naming the entry, not with an opaque shlex traceback.""" - from esphome.platformio.library import split_flag_entry with pytest.raises(EsphomeError, match="Malformed build flag"): split_flag_entry({"esp32": ["-DX"]}, "lib x") @@ -572,7 +571,6 @@ def test_split_flag_entry_non_string_is_clean() -> None: def test_source_kind_map_shape() -> None: """The kind values the native compile rules key on, and the deliberate AS/ASPP merge (.s and .S both map to asm).""" - from esphome.platformio.library import SOURCE_KIND_FOR_SUFFIX assert set(SOURCE_KIND_FOR_SUFFIX.values()) == {"c", "cxx", "asm"} assert SOURCE_KIND_FOR_SUFFIX[".s"] == "asm"