From 67544598c5c420111627abbad919aced1da66af4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 11:42:55 -0500 Subject: [PATCH] Trim comment essays --- esphome/build_gen/arduino8266.py | 57 ++++++------------- .../unit_tests/build_gen/test_arduino8266.py | 18 ++---- 2 files changed, 21 insertions(+), 54 deletions(-) diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 8c71556d6f..1405cb8e36 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -174,14 +174,10 @@ class _BuildConfig: def _lexed_build_flags() -> list[str]: - """Shell-lex every ``CORE.build_flags`` entry the way PlatformIO's - ``ParseFlags`` does, so a knob in ``"-DKNOB -DOTHER"``, a spaced - ``"-D KNOB"``, and quoted bodies all read identically everywhere. + """Shell-lex ``CORE.build_flags`` as PlatformIO's ``ParseFlags`` does, + sorted so duplicate defines resolve deterministically. - Sorted so duplicate defines resolve the same way every run (the winner - feeds the linker-script preprocessor line, which is also the cache - stamp). Lex once per build and pass the result to ``_flag_defines`` and - ``_project_flags`` so a malformed entry warns once, not per consumer. + Lex once per build; consumers share the tokens. """ return lex_build_flags(sorted(CORE.build_flags), "esphome") @@ -242,9 +238,8 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: # build flags, whose iteration order varies between processes. vtables_knobs = sorted(name for name in defines if name.startswith("VTABLES_IN_")) known_vtables = {"VTABLES_IN_FLASH", "VTABLES_IN_DRAM", "VTABLES_IN_IRAM"} - # A typo would otherwise win the sorted pick and end in the SDK header's - # #error, and a conflicting pair would resolve arbitrarily; both are - # config errors, not build-time surprises + # A typo'd or conflicting knob would otherwise fail obscurely in the + # SDK header's #error if unknown := [k for k in vtables_knobs if k not in known_vtables]: raise EsphomeError(f"Unknown VTABLES_IN_* define(s): {', '.join(unknown)}") if len(vtables_knobs) > 1: @@ -277,11 +272,8 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: mmu = sorted(body for name, body in defines.items() if name.startswith("MMU_")) else: if "MMU_IRAM_SIZE" in defines or "MMU_ICACHE_SIZE" in defines: - # PlatformIO only warns here and appends its defaults last so - # they win the compile line; in this generator the user's - # tokens would come last instead, compiling against a memory - # layout the linker script does not implement. Refuse rather - # than reproduce the upstream footgun with worse odds. + # Unlike PlatformIO (whose defaults win the compile line), user + # MMU_* here would win the compile but not the linker script; refuse. raise EsphomeError( "Custom MMU_IRAM_SIZE/MMU_ICACHE_SIZE build flags require " "-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" @@ -324,10 +316,7 @@ def _defines_flags( return [ f"-D{d}" for d in ( - # Upstream reads this from the board manifest (build.f_cpu), - # where all 45 supported boards ship 80000000L, overridable via - # board_build.f_cpu; published configs pin 160000000L for - # timing-sensitive integrations, so the override is honored + # Every supported board ships 80 MHz; board_build.f_cpu overrides f"F_CPU={_pio_option('board_build.f_cpu', '80000000L')}", "__ets__", "ICACHE_FLASH", @@ -360,15 +349,9 @@ def _project_flags( ) -> tuple[list[str], list[str], list[Path], list[str]]: """Split the ESPHome build flags into compile, linker, -L, and -l lists. - Every entry is shell-lexed the way PlatformIO's ``ParseFlags`` does, so a - linker flag anywhere in an entry reaches the link line and - ``build_unflags`` matches individual tokens (``-Os`` inside ``-Os -g3``). - Only the flag forms ESPHome emits are classified (``-Wl,``/``-L``/``-l`` - and compile flags); plain-form ``-T``/``-u``/``-Xlinker`` raise (inert on - a ``-c`` compile line), unlike full ParseFlags which routes them to the - link line. The returned ``compile_flags`` and ``link_flags`` are already - ``_shell_token``-quoted; ``lib_dirs`` and ``libs`` are raw and the caller - must quote them at emission. + Plain-form linker flags (``-T``/``-u``/``-Xlinker``) raise: they would be + inert on the ``-c`` compile line. ``compile_flags``/``link_flags`` come + back shell-quoted; ``lib_dirs``/``libs`` are raw, quote at emission. """ compile_flags: list[str] = [] link_flags: list[str] = [] @@ -457,25 +440,19 @@ def generate_ld_scripts( # incremental builds when nothing changed. output = ld_dir / "local.eagle.app.v6.common.ld" stamp = ld_dir / ".local.eagle.app.v6.common.ld.stamp" - # The surgery constants are inputs too: an edit to build_surgery.py must - # invalidate existing build dirs, not wait for an esphome clean. - # The header's and compiler's size and mtime cover an in-place - # framework or toolchain re-extraction at the same versioned path, - # which the command line alone would not notice + # Stamp includes the header/gcc stat (catches in-place re-extraction) + # and the surgery fingerprint (a build_surgery edit invalidates old + # build dirs) stamp_content = ( " ".join(cmd) + f" testing={CORE.testing_mode}" + f" header={_stat_sig(header)}" + f" gcc={_stat_sig(gcc)}" - # One fingerprint instead of enumerating surgery internals here, so - # any behavioral edit in build_surgery self-invalidates the cache + f" {build_surgery.surgery_fingerprint()}" ) def _cached_ld_is_valid() -> bool: - # A damaged cache (unreadable, non-UTF-8, truncated, externally - # edited) must regenerate, not abort the build or be reused on - # existence alone (the SECTIONS check below only guards generation) + # Any damaged cache regenerates; never abort the build over it try: if not ( output.is_file() @@ -530,9 +507,7 @@ def generate_ld_scripts( write_file_if_changed(output, content) stamp.write_text(stamp_content, encoding="utf-8") elif stderr_note.is_file(): - # The diagnostic must not vanish for the life of the build dir just - # because the script is cached; best-effort like every other cache - # read here (a damaged note must not abort an incremental build) + # Re-emit cached preprocessor warnings on cache hits; best-effort with contextlib.suppress(OSError, UnicodeDecodeError): _LOGGER.warning( "Linker-script preprocessor: %s", diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index 42441aff0a..147a7bdf32 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -467,10 +467,7 @@ def test_flag_defines_joins_spaced_define() -> None: def test_build_config_custom_mmu_without_knob_raises() -> None: - """Custom MMU sizes without the CUSTOM knob would compile against a - layout the linker script does not implement; refuse instead of warning - (PlatformIO warns, but its defaults win the compile line; ours would - not).""" + """Custom MMU sizes without the CUSTOM knob are refused.""" with pytest.raises(EsphomeError, match="PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM"): _resolve("-DMMU_IRAM_SIZE=0xC000") @@ -519,8 +516,7 @@ def test_flag_tables_match_platformio_builder() -> None: "-free", "-fipa-pta", ] - # The -u block is where the deliberate -u _scanf_float omission lives; - # pinned in full so "restoring" it fails here first + # Pins the deliberate -u _scanf_float omission assert arduino8266._LINKFLAGS == [ "-Os", "-nostdlib", @@ -569,8 +565,7 @@ def test_generate_ld_scripts_missing_compiler_is_clean(tmp_path: Path) -> None: def test_unflag_tokens_join_spaced_entries() -> None: - """A spaced build_unflags entry ("-D FOO") removes -DFOO, and no bare half leaks into the - unflag set to collaterally drop unrelated tokens.""" + """Spaced build_unflags entries ("-D FOO") match the joined token.""" CORE.build_unflags = {"-D FOO", "-l bar"} tokens = arduino8266._unflag_tokens() assert tokens == {"-DFOO", "-lbar"} @@ -592,8 +587,7 @@ def test_flag_defines_respects_unflags() -> None: def test_vtables_unknown_raises() -> None: - """A typo'd knob would win the sorted pick and die in the SDK header's - #error; fail by name at generation instead.""" + """An unknown VTABLES_IN_* knob fails by name.""" with pytest.raises(EsphomeError, match="Unknown VTABLES_IN_.*BANANA"): _resolve("-DVTABLES_IN_BANANA") @@ -686,9 +680,7 @@ def test_lexed_build_flags_shared_between_consumers( "tok", ["-Tcustom.ld", "-Xlinker", "-u", "-e", "-s", "-static", "-nostartfiles"] ) def test_project_flags_rejects_plain_linker_forms(tok: str) -> None: - """A plain-form linker flag would land on the -c compile line where it - is inert; refuse naming the -Wl, form instead of shipping firmware that - silently lacks the requested link behavior.""" + """Plain-form linker flags are refused, naming the -Wl, form.""" _set_flags(tok) with pytest.raises(EsphomeError, match="use the -Wl, form"): arduino8266._project_flags(set(), arduino8266._lexed_build_flags())