Simplify the ldgen patch guards

esphome/build_gen/espidf.py

@@ -2,7 +2,6 @@
 
 import json
 import logging
-import os
 from pathlib import Path
 
 from esphome.components.esp32 import (
@@ -19,7 +18,7 @@ from esphome.framework_helpers import (
     get_project_cxx_compile_flags,
     get_project_link_flags,
 )
-from esphome.helpers import mkdir_p, write_file_if_changed
+from esphome.helpers import get_bool_env, mkdir_p, write_file_if_changed
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -123,14 +122,13 @@ def get_project_cmakelists(
         else ""
     )
 
-    # The app component is never referenced by linker mapping fragments, so
-    # its rebuilds cannot change ldgen output; excluding it stops the ~3s
-    # sections.ld regeneration on app-only edits. Honored by the
-    # ESPHome-patched ldgen.cmake, a no-op on a stock IDF.
+    # Honored by the ESPHome-patched ldgen.cmake, a no-op on a stock IDF;
+    # stops the ~3s sections.ld regeneration on app-only edits.
+    # ESPHOME_LDGEN_FULL_DEPS=1 restores stock dependencies.
     ldgen_dep_exclude = (
-        ""
-        if os.environ.get("ESPHOME_LDGEN_FULL_DEPS") == "1"
-        else "set(ESPHOME_LDGEN_DEP_EXCLUDE idf::src __idf_src)"
+        "set(ESPHOME_LDGEN_DEP_EXCLUDE idf::src __idf_src)"
+        if not get_bool_env("ESPHOME_LDGEN_FULL_DEPS")
+        else ""
     )
 
     # CMake variables registered via cg.add_cmake_arg(). Emitted before

esphome/espidf/framework.py

@@ -686,12 +686,7 @@ def _patch_tools_json_demote_unused_tools(framework_path: Path) -> None:
     )
 
 
-_LDGEN_DEPENDS_ANCHOR = (
-    "\n        DEPENDS     ${template} ${ldgen_fragment_files}"
-    " ${ldgen_deps} ${SDKCONFIG}\n"
-)
 _LDGEN_COMMAND_ANCHOR = "    add_custom_command(\n"
-_LDGEN_PATCH_MARKER = "# Patched by ESPHome:"
 _LDGEN_DEP_FILTER = """\
     # Patched by ESPHome: drop app-only archives from ldgen DEPENDS.
     # ldgen only reads archives named in mapping fragments' archive: lines;
@@ -707,32 +702,15 @@ _LDGEN_DEP_FILTER = """\
 def _patch_ldgen_cmake(framework_path: Path) -> None:
     """Let projects drop their app archive from the sections.ld DEPENDS.
 
-    The filter is guarded by ESPHOME_LDGEN_DEP_EXCLUDE, which only the
-    ESPHome project CMakeLists sets, so stock IDF users of the shared
-    framework copy are unaffected. Idempotent and anchor checked; an
-    unexpected ldgen.cmake keeps stock behavior with a warning.
+    Guarded by ESPHOME_LDGEN_DEP_EXCLUDE, which only the ESPHome project
+    CMakeLists sets, so stock IDF builds using the shared framework copy
+    are unaffected. An unexpected ldgen.cmake keeps stock behavior.
     """
     ldgen_cmake = framework_path / "tools" / "cmake" / "ldgen.cmake"
     if not ldgen_cmake.is_file():
         return
-
     try:
         content = ldgen_cmake.read_text(encoding="utf-8")
-        if _LDGEN_PATCH_MARKER in content:
-            return
-        if (
-            _LDGEN_DEPENDS_ANCHOR not in content
-            or content.count(_LDGEN_COMMAND_ANCHOR) != 1
-        ):
-            _LOGGER.warning(
-                "ldgen.cmake at %s does not match the expected layout; "
-                "skipping the ldgen dependency patch (builds stay correct).",
-                ldgen_cmake,
-            )
-            return
-        write_file_if_changed(
-            ldgen_cmake, content.replace(_LDGEN_COMMAND_ANCHOR, _LDGEN_DEP_FILTER)
-        )
     except OSError as e:
         _LOGGER.warning(
             "Could not apply the ldgen dependency patch to %s (%s); skipping.",
@@ -740,6 +718,18 @@ def _patch_ldgen_cmake(framework_path: Path) -> None:
             e,
         )
         return
+    if "ESPHOME_LDGEN_DEP_EXCLUDE" in content:
+        return
+    if "${ldgen_deps}" not in content or content.count(_LDGEN_COMMAND_ANCHOR) != 1:
+        _LOGGER.warning(
+            "ldgen.cmake at %s does not match the expected layout; "
+            "skipping the ldgen dependency patch (builds stay correct).",
+            ldgen_cmake,
+        )
+        return
+    write_file_if_changed(
+        ldgen_cmake, content.replace(_LDGEN_COMMAND_ANCHOR, _LDGEN_DEP_FILTER)
+    )
     _LOGGER.info("Patched %s to honor ESPHOME_LDGEN_DEP_EXCLUDE.", ldgen_cmake)
 
 
@@ -973,9 +963,7 @@ def _check_esphome_idf_framework_install(
     # check recovers on the next build.
     _patch_tools_json_demote_unused_tools(framework_path)
 
-    # Let the project CMakeLists drop the app archive from the sections.ld
-    # DEPENDS so app-only edits skip the ~3s ldgen re-run. Idempotent, heals
-    # pre-patch trees on the next build.
+    # Applied every invocation so pre-patch trees heal without a clean.
     _patch_ldgen_cmake(framework_path)
 
     # 3. Check if the framework tools are the same and correctly installed

tests/unit_tests/build_gen/test_espidf.py

@@ -163,18 +163,19 @@ def test_has_discovered_components_after_configure(tmp_path: Path) -> None:
     assert has_discovered_components()
 
 
-def test_get_project_cmakelists_sets_ldgen_dep_exclude() -> None:
-    """Both renders exclude the app archive from ldgen's DEPENDS; app code
-    never appears in linker mapping fragments so it cannot affect ldgen."""
-    assert "set(ESPHOME_LDGEN_DEP_EXCLUDE idf::src __idf_src)" in _render()
-    assert "set(ESPHOME_LDGEN_DEP_EXCLUDE idf::src __idf_src)" in _render(minimal=True)
+@pytest.mark.parametrize("minimal", [False, True])
+def test_get_project_cmakelists_sets_ldgen_dep_exclude(minimal: bool) -> None:
+    """Both renders exclude the app archive from ldgen's DEPENDS."""
+    assert "set(ESPHOME_LDGEN_DEP_EXCLUDE idf::src __idf_src)" in _render(
+        minimal=minimal
+    )
 
 
 def test_get_project_cmakelists_ldgen_full_deps_escape_hatch(
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
-    """ESPHOME_LDGEN_FULL_DEPS=1 restores stock ldgen dependencies."""
-    monkeypatch.setenv("ESPHOME_LDGEN_FULL_DEPS", "1")
+    """ESPHOME_LDGEN_FULL_DEPS restores stock ldgen dependencies."""
+    monkeypatch.setenv("ESPHOME_LDGEN_FULL_DEPS", "true")
     assert "ESPHOME_LDGEN_DEP_EXCLUDE" not in _render()
 
 

tests/unit_tests/test_espidf_framework.py

@@ -20,6 +20,7 @@ from unittest.mock import MagicMock, patch
 import pytest
 
 from esphome.espidf.framework import (
+    _LDGEN_DEP_FILTER,
     ESPHOME_STAMP_FILE,
     STAMP_SCHEMA_VERSION,
     _ccache_env,
@@ -897,12 +898,7 @@ def test_patch_ldgen_cmake_inserts_guarded_filter(tmp_path: Path) -> None:
     ldgen_cmake = _write_ldgen_cmake(tmp_path, _LDGEN_CMAKE_STOCK)
     _patch_ldgen_cmake(tmp_path)
     content = ldgen_cmake.read_text(encoding="utf-8")
-    assert "if(ESPHOME_LDGEN_DEP_EXCLUDE)" in content
-    assert "list(REMOVE_ITEM ldgen_deps ${ESPHOME_LDGEN_DEP_EXCLUDE})" in content
-    # The filter must run before the command that consumes ldgen_deps
-    assert content.index("REMOVE_ITEM ldgen_deps") < content.index(
-        "add_custom_command("
-    )
+    assert _LDGEN_DEP_FILTER in content
     assert "DEPENDS     ${template}" in content
 
 
@@ -927,22 +923,23 @@ def test_patch_ldgen_cmake_unreadable_file_warns_and_skips(
     assert "Could not apply the ldgen dependency patch" in caplog.text
 
 
-def test_patch_ldgen_cmake_unexpected_depends_skips(
-    tmp_path: Path, caplog: pytest.LogCaptureFixture
+@pytest.mark.parametrize(
+    "content",
+    [
+        pytest.param(
+            _LDGEN_CMAKE_STOCK.replace("${ldgen_deps}", "${other_deps}"),
+            id="no_ldgen_deps",
+        ),
+        pytest.param(
+            _LDGEN_CMAKE_STOCK + "\n    add_custom_command(\n        OUTPUT x)\n",
+            id="duplicate_command",
+        ),
+    ],
+)
+def test_patch_ldgen_cmake_unexpected_layout_skips(
+    content: str, tmp_path: Path, caplog: pytest.LogCaptureFixture
 ) -> None:
-    """A future IDF with a different DEPENDS line keeps stock behavior."""
-    content = _LDGEN_CMAKE_STOCK.replace("${SDKCONFIG}", "${SDKCONFIG} ${EXTRA}")
-    ldgen_cmake = _write_ldgen_cmake(tmp_path, content)
-    _patch_ldgen_cmake(tmp_path)
-    assert ldgen_cmake.read_text(encoding="utf-8") == content
-    assert "does not match the expected layout" in caplog.text
-
-
-def test_patch_ldgen_cmake_duplicate_command_skips(
-    tmp_path: Path, caplog: pytest.LogCaptureFixture
-) -> None:
-    """Two add_custom_command blocks make the insert ambiguous; keep stock."""
-    content = _LDGEN_CMAKE_STOCK + "\n    add_custom_command(\n        OUTPUT x)\n"
+    """A future IDF that restructures ldgen.cmake keeps stock behavior."""
     ldgen_cmake = _write_ldgen_cmake(tmp_path, content)
     _patch_ldgen_cmake(tmp_path)
     assert ldgen_cmake.read_text(encoding="utf-8") == content