Trim comment essays

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",

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())