Address review: include surgery constants in the ld stamp, require size sections, warn on missing extraScript

esphome/arduino8266/toolchain.py

@@ -149,8 +149,15 @@ def _print_size_summary(build_dir: Path, toolchain_path: Path) -> None:
                     # A confident total built on a dropped section would feed
                     # a wrong number to CI's memory-impact metric
                     return
-    ram = sum(sections.get(s, 0) for s in _RAM_SECTIONS)
-    flash = sum(sections.get(s, 0) for s in _FLASH_SECTIONS)
+    if missing := set(_RAM_SECTIONS + _FLASH_SECTIONS) - set(sections):
+        # A defaulted 0 would print a confidently wrong total for CI's metric
+        _LOGGER.warning(
+            "Size output is missing section(s) %s; skipping the size summary",
+            ", ".join(sorted(missing)),
+        )
+        return
+    ram = sum(sections[s] for s in _RAM_SECTIONS)
+    flash = sum(sections[s] for s in _FLASH_SECTIONS)
     print(f"RAM:   {format_bar(ram, _MAX_RAM_SIZE)}")
     if app_size := _parse_app_size(build_dir):
         print(f"Flash: {format_bar(flash, app_size)}")

esphome/build_gen/arduino8266.py

@@ -21,15 +21,12 @@ import re
 import subprocess
 import sys
 
+from esphome.components.esp8266 import build_surgery
 from esphome.components.esp8266.boards import (
     BOARDS,
     ESP8266_BOARD_BUILD,
     ESP8266_LD_SCRIPTS,
 )
-from esphome.components.esp8266.build_surgery import (
-    apply_testing_memory_patches,
-    relocate_ratetable,
-)
 from esphome.components.esp8266.const import (
     KEY_BOARD,
     KEY_ESP8266,
@@ -357,7 +354,16 @@ 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"
-    stamp_content = " ".join(cmd) + f" testing={CORE.testing_mode}"
+    # The surgery constants are inputs too: an edit to build_surgery.py must
+    # invalidate existing build dirs, not wait for an esphome clean.
+    stamp_content = (
+        " ".join(cmd)
+        + f" testing={CORE.testing_mode}"
+        + f" {build_surgery.RATETABLE_RULE}"
+        + f" {build_surgery.TESTING_IRAM_SIZE}"
+        + f" {build_surgery.TESTING_DRAM_SIZE}"
+        + f" {build_surgery.TESTING_FLASH_SIZE}"
+    )
     if not (
         output.is_file()
         and stamp.is_file()
@@ -368,9 +374,11 @@ def generate_ld_scripts(
         )
         if result.returncode != 0:
             raise EsphomeError(f"Generating the linker script failed:\n{result.stderr}")
-        content = relocate_ratetable(result.stdout)
+        content = build_surgery.relocate_ratetable(result.stdout)
         if CORE.testing_mode:
-            content = apply_testing_memory_patches(content, require=("iram1_0_seg",))
+            content = build_surgery.apply_testing_memory_patches(
+                content, require=("iram1_0_seg",)
+            )
         write_file_if_changed(output, content)
         stamp.write_text(stamp_content, encoding="utf-8")
 
@@ -380,7 +388,7 @@ def generate_ld_scripts(
         flash_ld = framework / "tools" / "sdk" / "ld" / flash_ld_name
         write_file_if_changed(
             ld_dir / f"testing_{flash_ld_name}",
-            apply_testing_memory_patches(
+            build_surgery.apply_testing_memory_patches(
                 flash_ld.read_text(encoding="utf-8"),
                 require=("dram0_0_seg", "irom0_0_seg"),
             ),

esphome/espidf/extra_script.py

@@ -70,7 +70,9 @@ def apply_extra_script(
         )
         return
     if not script_path.is_file():
-        _LOGGER.debug(
+        # The script's captured -L/-l/-D flags are lost; surface that here
+        # instead of as undefined references at link time
+        _LOGGER.warning(
             "extraScript %s of library %s not found; skipping",
             extra_script,
             component.name,

tests/unit_tests/build_gen/test_arduino8266.py

@@ -398,6 +398,15 @@ def test_generate_ld_scripts(tmp_path: Path) -> None:
         _run_generate_ld_scripts(paths)
     mock_run.assert_not_called()
 
+    # An edit to the surgery constants invalidates the stamp (a stale linker
+    # script would otherwise persist until an esphome clean)
+    with (
+        patch.object(arduino8266.build_surgery, "TESTING_FLASH_SIZE", "0x3000000"),
+        patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run,
+    ):
+        _run_generate_ld_scripts(paths)
+    mock_run.assert_called_once()
+
 
 def test_generate_ld_scripts_failure(tmp_path: Path) -> None:
 

tests/unit_tests/test_arduino8266_toolchain.py

@@ -253,3 +253,22 @@ def test_print_size_summary_unparsable_section(
         toolchain._print_size_summary(tmp_path, tmp_path / "toolchain")
     assert "RAM:" in capsys.readouterr().out
     assert "Unparsable size output" in caplog.text
+
+
+def test_print_size_summary_missing_section_skips_summary(
+    tmp_path: Path,
+    capsys: pytest.CaptureFixture[str],
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """A totals section absent from the output must not default to zero."""
+    without_bss = "\n".join(
+        line for line in _SIZE_OUTPUT.splitlines() if ".bss" not in line
+    )
+    with patch.object(
+        toolchain.subprocess,
+        "run",
+        return_value=MagicMock(returncode=0, stdout=without_bss),
+    ):
+        toolchain._print_size_summary(tmp_path, tmp_path / "toolchain")
+    assert capsys.readouterr().out == ""
+    assert "missing section(s) .bss" in caplog.text

tests/unit_tests/test_espidf_component.py

@@ -1270,14 +1270,12 @@ def test_apply_extra_script_pio_platform(tmp_path) -> None:
 
 
 def test_apply_extra_script_missing_script_logged(tmp_path, caplog) -> None:
-    """A declared but absent extraScript is skipped with a diagnostic."""
-    import logging
+    """A declared but absent extraScript is skipped with a visible warning:
+    its captured link flags are lost."""
+    from esphome.espidf.extra_script import apply_extra_script
 
     c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
     c.path = tmp_path
     c.data = {"build": {"extraScript": "nope.py"}}
-    with caplog.at_level(logging.DEBUG, logger="esphome.espidf.extra_script"):
-        from esphome.espidf.extra_script import apply_extra_script
-
-        apply_extra_script(c, "esp8266")
+    apply_extra_script(c, "esp8266")
     assert "not found" in caplog.text