Close the remaining silent degradation paths

esphome/build_gen/espidf.py

@@ -39,6 +39,7 @@ idf_build_set_property(CXX_COMPILE_OPTIONS "${{esphome_cxx_compile_options}}")""
 # the prior definition stays reachable with an underscore prefix.
 _LDGEN_OVERRIDE = """\
 if(COMMAND __ldgen_get_lib_deps_of_target)
+    set_property(GLOBAL PROPERTY ESPHOME_LDGEN_ARMED 1)
     function(__ldgen_get_lib_deps_of_target target out_list_var)
         if(NOT COMMAND ___ldgen_get_lib_deps_of_target)
             message(FATAL_ERROR "ESPHome ldgen override lost the original "
@@ -46,6 +47,7 @@ if(COMMAND __ldgen_get_lib_deps_of_target)
         endif()
         ___ldgen_get_lib_deps_of_target(${target} ${out_list_var})
         if(out_list_var STREQUAL "ldgen_libraries")
+            set_property(GLOBAL PROPERTY ESPHOME_LDGEN_FILTERED 1)
             list(LENGTH ${out_list_var} esphome_ldgen_before)
             list(REMOVE_ITEM ${out_list_var} idf::src __idf_src)
             list(LENGTH ${out_list_var} esphome_ldgen_after)
@@ -61,6 +63,16 @@ else()
         "app edits will regenerate sections.ld.")
 endif()"""
 
+# Runs after project() so the walk has happened; catches the remaining
+# silent path where the top-level out-var was renamed.
+_LDGEN_OVERRIDE_CHECK = """\
+get_property(esphome_ldgen_armed GLOBAL PROPERTY ESPHOME_LDGEN_ARMED)
+get_property(esphome_ldgen_filtered GLOBAL PROPERTY ESPHOME_LDGEN_FILTERED)
+if(esphome_ldgen_armed AND NOT esphome_ldgen_filtered)
+    message(@SEVERITY@ "ESPHome ldgen override never filtered the app "
+        "archive; app edits will regenerate sections.ld.")
+endif()"""
+
 
 def get_available_components() -> list[str] | None:
     """List the built-in ESP-IDF components from ``project_description.json``.
@@ -156,11 +168,15 @@ def get_project_cmakelists(
     # breaks the override instead of degrading to stock deps.
     if get_bool_env("ESPHOME_LDGEN_FULL_DEPS"):
         ldgen_override = ""
+        ldgen_override_check = ""
     else:
         strict = get_bool_env("ESPHOME_LDGEN_STRICT")
-        ldgen_override = _LDGEN_OVERRIDE.replace(
-            "@SEVERITY@", "FATAL_ERROR" if strict else "WARNING"
-        ).replace("@MISSING@", "FATAL_ERROR" if strict else "STATUS")
+        severity = "FATAL_ERROR" if strict else "WARNING"
+        missing = "FATAL_ERROR" if strict else "STATUS"
+        ldgen_override = _LDGEN_OVERRIDE.replace("@SEVERITY@", severity).replace(
+            "@MISSING@", missing
+        )
+        ldgen_override_check = _LDGEN_OVERRIDE_CHECK.replace("@SEVERITY@", severity)
 
     # CMake variables registered via cg.add_cmake_arg(). Emitted before
     # include(project.cmake) so values like EXCLUDE_COMPONENTS are already
@@ -253,6 +269,8 @@ include($ENV{{IDF_PATH}}/tools/cmake/project.cmake)
 
 project({CORE.name})
 
+{ldgen_override_check}
+
 # Emit raw JSON size data for ESPHome to read post-build.
 add_custom_command(
     TARGET ${{CMAKE_PROJECT_NAME}}.elf POST_BUILD

esphome/espidf/toolchain.py

@@ -480,7 +480,14 @@ def _patch_memory_segments():
 
 
 _LDGEN_FRAGMENTS_RE = re.compile(r'--fragments-list\s+"([^"]+)"')
-_APP_ARCHIVE_MAPPED_RE = re.compile(r"^archive:\s*libsrc\.a\s*$", re.MULTILINE)
+_APP_ARCHIVE_MAPPED_RE = re.compile(r"^\s*archive:\s*libsrc\.a\b", re.MULTILINE)
+
+
+def _ldgen_check_skip(msg: str, strict: bool) -> None:
+    """A skipped fragment check is debug for users, fatal under strict."""
+    if strict:
+        raise EsphomeError(f"ldgen fragment check: {msg} (ESPHOME_LDGEN_STRICT)")
+    _LOGGER.debug("Skipping ldgen fragment check: %s", msg)
 
 
 def _warn_if_app_archive_mapped() -> None:
@@ -488,26 +495,33 @@ def _warn_if_app_archive_mapped() -> None:
     linker fragment names the app archive, since ldgen would silently skip
     remapping it rather than fail.
     """
+    strict = get_bool_env("ESPHOME_LDGEN_STRICT")
     build_ninja = CORE.relative_build_path("build", "build.ninja")
     try:
-        match = _LDGEN_FRAGMENTS_RE.search(build_ninja.read_text(encoding="utf-8"))
-        if match is None:
-            return
-        for fragment in match.group(1).split(";"):
-            if _APP_ARCHIVE_MAPPED_RE.search(
-                Path(fragment).read_text(encoding="utf-8")
-            ):
-                msg = (
-                    f"Linker fragment {fragment} maps the app archive; its "
-                    "entries may be skipped. Set ESPHOME_LDGEN_FULL_DEPS=1 "
-                    "and rebuild."
-                )
-                if get_bool_env("ESPHOME_LDGEN_STRICT"):
-                    raise EsphomeError(msg)
-                _LOGGER.warning("%s", msg)
-                return
+        ninja_text = build_ninja.read_text(encoding="utf-8", errors="replace")
     except OSError as e:
-        _LOGGER.debug("Skipping ldgen fragment check: %s", e)
+        _ldgen_check_skip(f"could not read {build_ninja}: {e}", strict)
+        return
+    match = _LDGEN_FRAGMENTS_RE.search(ninja_text)
+    if match is None:
+        _ldgen_check_skip(f"no --fragments-list in {build_ninja}", strict)
+        return
+    for fragment in match.group(1).split(";"):
+        try:
+            text = Path(fragment).read_text(encoding="utf-8", errors="replace")
+        except OSError as e:
+            _ldgen_check_skip(f"could not read {fragment}: {e}", strict)
+            continue
+        if _APP_ARCHIVE_MAPPED_RE.search(text):
+            msg = (
+                f"Linker fragment {fragment} maps the app archive; its "
+                "entries may be skipped. Set ESPHOME_LDGEN_FULL_DEPS=1 "
+                "and rebuild."
+            )
+            if strict:
+                raise EsphomeError(msg)
+            _LOGGER.warning("%s", msg)
+            return
 
 
 def run_compile(config, verbose: bool) -> int:

tests/unit_tests/build_gen/test_espidf.py

@@ -175,9 +175,12 @@ def test_get_project_cmakelists_emits_ldgen_override(
     assert "REMOVE_ITEM ${out_list_var} idf::src __idf_src" in content
     assert 'message(WARNING "ESPHome ldgen app archive exclusion' in content
     assert 'message(STATUS "ESPHome ldgen override target not found' in content
+    assert 'message(WARNING "ESPHome ldgen override never filtered' in content
     assert content.index("tools/cmake/project.cmake") < content.index(
         "function(__ldgen_get_lib_deps_of_target"
     )
+    # The never-filtered check must run after project() has walked the deps
+    assert content.index("project(test)") < content.index("esphome_ldgen_armed GLOBAL")
 
 
 def test_get_project_cmakelists_ldgen_strict_fails_closed(
@@ -190,6 +193,7 @@ def test_get_project_cmakelists_ldgen_strict_fails_closed(
     content = _render()
     assert 'message(FATAL_ERROR "ESPHome ldgen app archive exclusion' in content
     assert 'message(FATAL_ERROR "ESPHome ldgen override target not found' in content
+    assert 'message(FATAL_ERROR "ESPHome ldgen override never filtered' in content
     assert "@SEVERITY@" not in content
     assert "@MISSING@" not in content
 
@@ -199,7 +203,9 @@ def test_get_project_cmakelists_ldgen_full_deps_escape_hatch(
 ) -> None:
     """ESPHOME_LDGEN_FULL_DEPS restores stock ldgen behavior."""
     monkeypatch.setenv("ESPHOME_LDGEN_FULL_DEPS", "true")
-    assert "__ldgen_get_lib_deps_of_target" not in _render()
+    content = _render()
+    assert "__ldgen_get_lib_deps_of_target" not in content
+    assert "esphome_ldgen_armed" not in content
 
 
 def test_get_project_cmakelists_uses_supplied_builtin_components() -> None:

tests/unit_tests/test_espidf_toolchain.py

@@ -623,6 +623,13 @@ def test_component_cache_ignores_corrupt_file(setup_core: Path, tmp_path: Path)
         assert toolchain.load_cached_builtin_components() is None
 
 
+@pytest.fixture(autouse=True)
+def _clear_ldgen_env(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Isolate tests from ambient ldgen escape hatch and strict knobs."""
+    monkeypatch.delenv("ESPHOME_LDGEN_STRICT", raising=False)
+    monkeypatch.delenv("ESPHOME_LDGEN_FULL_DEPS", raising=False)
+
+
 def _write_fragments_build_ninja(tmp_path: Path, fragments: list[Path]) -> None:
     build_dir = CORE.relative_build_path("build")
     build_dir.mkdir(parents=True, exist_ok=True)
@@ -633,21 +640,43 @@ def _write_fragments_build_ninja(tmp_path: Path, fragments: list[Path]) -> None:
 
 
 def test_warn_if_app_archive_mapped_warns(
-    setup_core: Path,
-    tmp_path: Path,
-    caplog: pytest.LogCaptureFixture,
-    monkeypatch: pytest.MonkeyPatch,
+    setup_core: Path, tmp_path: Path, caplog: pytest.LogCaptureFixture
 ) -> None:
-    """A fragment naming the app archive triggers the loud warning."""
-    monkeypatch.delenv("ESPHOME_LDGEN_STRICT", raising=False)
+    """A fragment naming the app archive, even with trailing text or leading
+    whitespace, triggers the loud warning."""
     _setup_build(setup_core)
     frag = tmp_path / "linker.lf"
-    frag.write_text("[mapping:evil]\narchive: libsrc.a\nentries:\n    * (noflash)\n")
+    frag.write_text("[mapping:evil]\n archive: libsrc.a # app\nentries:\n")
     _write_fragments_build_ninja(tmp_path, [frag])
     toolchain._warn_if_app_archive_mapped()
     assert "maps the app archive" in caplog.text
 
 
+def test_warn_if_app_archive_mapped_scans_past_unreadable(
+    setup_core: Path, tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """An unreadable fragment doesn't stop later fragments being checked."""
+    _setup_build(setup_core)
+    frag = tmp_path / "linker.lf"
+    frag.write_text("[mapping:evil]\narchive: libsrc.a\n")
+    _write_fragments_build_ninja(tmp_path, [tmp_path / "missing.lf", frag])
+    toolchain._warn_if_app_archive_mapped()
+    assert "maps the app archive" in caplog.text
+
+
+def test_warn_if_app_archive_mapped_strict_no_fragments_list(
+    setup_core: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Under strict, a build.ninja the check can't parse fails the build."""
+    monkeypatch.setenv("ESPHOME_LDGEN_STRICT", "1")
+    _setup_build(setup_core)
+    build_dir = CORE.relative_build_path("build")
+    build_dir.mkdir(parents=True, exist_ok=True)
+    (build_dir / "build.ninja").write_text("rule CXX\n  command = gcc\n")
+    with pytest.raises(EsphomeError, match="no --fragments-list"):
+        toolchain._warn_if_app_archive_mapped()
+
+
 def test_warn_if_app_archive_mapped_strict_raises(
     setup_core: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
 ) -> None:
@@ -698,6 +727,22 @@ def test_warn_if_app_archive_mapped_no_fragments_list(setup_core: Path) -> None:
     toolchain._warn_if_app_archive_mapped()
 
 
+def test_run_compile_runs_fragment_check(setup_core: Path) -> None:
+    """The fragment belt runs by default on every compile."""
+    _setup_build(setup_core)
+    config = {CONF_ESPHOME: {}}
+
+    with (
+        patch.object(toolchain, "need_reconfigure", return_value=False),
+        patch.object(toolchain, "run_idf_py", return_value=0),
+        patch.object(toolchain, "print_summary"),
+        patch.object(toolchain, "_warn_if_app_archive_mapped") as mock_check,
+    ):
+        assert toolchain.run_compile(config, verbose=False) == 0
+
+    mock_check.assert_called_once()
+
+
 def test_run_compile_full_deps_skips_fragment_check(
     setup_core: Path, monkeypatch: pytest.MonkeyPatch
 ) -> None: