diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 8bdddfa868..65a392d45f 100644 --- a/esphome/build_gen/espidf.py +++ b/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 diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index ce4a769268..cca32b52fb 100644 --- a/esphome/espidf/framework.py +++ b/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 diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index 7f94fb1de9..a39d7add56 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/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() diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index 5afb07cb2f..3411eb6584 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/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