From 0aa6c6cc4d662e821092cf01b8bfcd5d0bc92b99 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 28 Aug 2026 09:51:52 -0500 Subject: [PATCH] [espidf] Skip ldgen when only app code changes --- esphome/build_gen/espidf.py | 13 ++++ esphome/espidf/framework.py | 62 ++++++++++++++++++ tests/unit_tests/build_gen/test_espidf.py | 15 +++++ tests/unit_tests/test_espidf_framework.py | 76 +++++++++++++++++++++++ 4 files changed, 166 insertions(+) diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 2ef89cf595..8bdddfa868 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -2,6 +2,7 @@ import json import logging +import os from pathlib import Path from esphome.components.esp32 import ( @@ -122,6 +123,16 @@ 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. + ldgen_dep_exclude = ( + "" + if os.environ.get("ESPHOME_LDGEN_FULL_DEPS") == "1" + else "set(ESPHOME_LDGEN_DEP_EXCLUDE idf::src __idf_src)" + ) + # CMake variables registered via cg.add_cmake_arg(). Emitted before # include(project.cmake) so values like EXCLUDE_COMPONENTS are already # set when project.cmake seeds the component list, and on minimal @@ -195,6 +206,8 @@ set(CMAKE_NINJA_FORCE_RESPONSE_FILE 1) set(IDF_TARGET {idf_target}) set(EXTRA_COMPONENT_DIRS ${{CMAKE_SOURCE_DIR}}/src) +{ldgen_dep_exclude} + {cmake_args} include($ENV{{IDF_PATH}}/tools/cmake/project.cmake) diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 6c2a285360..ce4a769268 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -686,6 +686,63 @@ 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; + # the app component never appears there, so rebuilding it cannot change + # the generated linker script. Set by the ESPHome project CMakeLists. + if(ESPHOME_LDGEN_DEP_EXCLUDE) + list(REMOVE_ITEM ldgen_deps ${ESPHOME_LDGEN_DEP_EXCLUDE}) + endif() + add_custom_command( +""" + + +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. + """ + 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.", + ldgen_cmake, + e, + ) + return + _LOGGER.info("Patched %s to honor ESPHOME_LDGEN_DEP_EXCLUDE.", ldgen_cmake) + + def _prefetch_idf_tool_archives( framework_path: Path, targets_str: str, @@ -916,6 +973,11 @@ 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. + _patch_ldgen_cmake(framework_path) + # 3. Check if the framework tools are the same and correctly installed stored_stamp = None if fresh_extract else _read_stamp(env_stamp_file) install = fresh_extract diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index 079f10ddb9..7f94fb1de9 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -163,6 +163,21 @@ 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) + + +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") + assert "ESPHOME_LDGEN_DEP_EXCLUDE" not in _render() + + def test_get_project_cmakelists_uses_supplied_builtin_components() -> None: """A cached list replaces project_description.json and is still filtered by EXCLUDE_COMPONENTS.""" diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index afa4433aa1..dadf8717a4 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -33,6 +33,7 @@ from esphome.espidf.framework import ( _get_python_env_path, _get_python_version, _parse_git_source, + _patch_ldgen_cmake, _patch_tools_json_demote_unused_tools, _patch_tools_json_for_linux_arm64, _prefetch_idf_tool_archives, @@ -401,6 +402,7 @@ def espidf_mocks(setup_core: Path): patch("esphome.espidf.framework._write_idf_version_txt"), patch("esphome.espidf.framework._patch_tools_json_for_linux_arm64"), patch("esphome.espidf.framework._patch_tools_json_demote_unused_tools"), + patch("esphome.espidf.framework._patch_ldgen_cmake"), patch("esphome.espidf.framework._prefetch_idf_tool_archives"), patch("esphome.espidf.framework._write_stamp"), patch("esphome.espidf.framework._check_stamp", return_value=True), @@ -864,6 +866,80 @@ def test_patch_tools_json_already_patched_is_noop(tmp_path: Path) -> None: assert tools_json.read_text(encoding="utf-8") == before +# --------------------------------------------------------------------------- +# _patch_ldgen_cmake +# --------------------------------------------------------------------------- + + +_LDGEN_CMAKE_STOCK = """\ +function(__ldgen_create_target exe_target) + idf_build_get_property(python PYTHON) + + add_custom_command( + OUTPUT ${output} + COMMAND ${python} "${idf_path}/tools/ldgen/ldgen.py" + DEPENDS ${template} ${ldgen_fragment_files} ${ldgen_deps} ${SDKCONFIG} + VERBATIM + ) +endfunction() +""" + + +def _write_ldgen_cmake(framework_path: Path, content: str) -> Path: + cmake_dir = framework_path / "tools" / "cmake" + cmake_dir.mkdir(parents=True, exist_ok=True) + ldgen_cmake = cmake_dir / "ldgen.cmake" + ldgen_cmake.write_text(content, encoding="utf-8") + return ldgen_cmake + + +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 "DEPENDS ${template}" in content + + +def test_patch_ldgen_cmake_is_idempotent(tmp_path: Path) -> None: + ldgen_cmake = _write_ldgen_cmake(tmp_path, _LDGEN_CMAKE_STOCK) + _patch_ldgen_cmake(tmp_path) + patched = ldgen_cmake.read_text(encoding="utf-8") + _patch_ldgen_cmake(tmp_path) + assert ldgen_cmake.read_text(encoding="utf-8") == patched + + +def test_patch_ldgen_cmake_missing_file_is_noop(tmp_path: Path) -> None: + _patch_ldgen_cmake(tmp_path) # no tools/cmake/ldgen.cmake present + + +def test_patch_ldgen_cmake_unexpected_depends_skips( + 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" + 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 + + # --------------------------------------------------------------------------- # _prefetch_idf_tool_archives # ---------------------------------------------------------------------------