mirror of
https://github.com/esphome/esphome.git
synced 2026-09-11 15:27:33 +00:00
Replace the framework patch with a CMake function override
This commit is contained in:
+35
-10
@@ -33,6 +33,36 @@ list(FILTER esphome_cxx_compile_options EXCLUDE REGEX "^-std=")
|
||||
list(APPEND esphome_cxx_compile_options "-std={standard}")
|
||||
idf_build_set_property(CXX_COMPILE_OPTIONS "${{esphome_cxx_compile_options}}")"""
|
||||
|
||||
# Drops the app archive from ldgen's inputs (the sections.ld DEPENDS and the
|
||||
# ldgen_libraries file) by overriding the IDF helper that collects them, so
|
||||
# app-only edits skip the sections.ld regeneration and ldgen never opens the
|
||||
# archive. Safe because no linker mapping fragment references the app archive
|
||||
# (run_compile re-checks that each build); the override filters only the top
|
||||
# level call, after the recursive walk finished, so the walker's cycle guard
|
||||
# is untouched. CMake keeps the prior definition reachable with an underscore
|
||||
# prefix; a future IDF that renames the helper skips the override and keeps
|
||||
# stock behavior. Emitted after include(project.cmake), before project().
|
||||
_LDGEN_OVERRIDE = """\
|
||||
if(COMMAND __ldgen_get_lib_deps_of_target)
|
||||
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 "
|
||||
"implementation; set ESPHOME_LDGEN_FULL_DEPS=1 and rebuild.")
|
||||
endif()
|
||||
___ldgen_get_lib_deps_of_target(${target} ${out_list_var})
|
||||
if(out_list_var STREQUAL "ldgen_libraries")
|
||||
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)
|
||||
if(esphome_ldgen_before EQUAL esphome_ldgen_after)
|
||||
message(WARNING "ESPHome ldgen app archive exclusion matched "
|
||||
"nothing; app edits will regenerate sections.ld.")
|
||||
endif()
|
||||
endif()
|
||||
set(${out_list_var} ${${out_list_var}} PARENT_SCOPE)
|
||||
endfunction()
|
||||
endif()"""
|
||||
|
||||
|
||||
def get_available_components() -> list[str] | None:
|
||||
"""List the built-in ESP-IDF components from ``project_description.json``.
|
||||
@@ -122,14 +152,9 @@ def get_project_cmakelists(
|
||||
else ""
|
||||
)
|
||||
|
||||
# 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 = (
|
||||
"set(ESPHOME_LDGEN_DEP_EXCLUDE idf::src __idf_src)"
|
||||
if not get_bool_env("ESPHOME_LDGEN_FULL_DEPS")
|
||||
else ""
|
||||
)
|
||||
# Stops the ~3s sections.ld regeneration on app-only edits; see
|
||||
# _LDGEN_OVERRIDE. ESPHOME_LDGEN_FULL_DEPS=1 restores stock behavior.
|
||||
ldgen_override = "" if get_bool_env("ESPHOME_LDGEN_FULL_DEPS") else _LDGEN_OVERRIDE
|
||||
|
||||
# CMake variables registered via cg.add_cmake_arg(). Emitted before
|
||||
# include(project.cmake) so values like EXCLUDE_COMPONENTS are already
|
||||
@@ -204,12 +229,12 @@ 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)
|
||||
|
||||
{ldgen_override}
|
||||
|
||||
{cpp_standard_options}
|
||||
|
||||
{cxx_compile_options}
|
||||
|
||||
@@ -686,57 +686,6 @@ def _patch_tools_json_demote_unused_tools(framework_path: Path) -> None:
|
||||
)
|
||||
|
||||
|
||||
_LDGEN_JOIN_ANCHOR = ' list(JOIN ldgen_libraries_expr "\\n" ldgen_libraries_str)\n'
|
||||
_LDGEN_DEP_FILTER = """\
|
||||
# Patched by ESPHome: drop app-only archives from ldgen's inputs.
|
||||
# ldgen only reads archives named in mapping fragments' archive: lines;
|
||||
# the app component never appears there, so it cannot change the output,
|
||||
# and keeping it listed would race ldgen's objdump against the archive
|
||||
# being written. Set by the ESPHome project CMakeLists.
|
||||
if(ESPHOME_LDGEN_DEP_EXCLUDE)
|
||||
list(REMOVE_ITEM ldgen_deps ${ESPHOME_LDGEN_DEP_EXCLUDE})
|
||||
foreach(esphome_ldgen_excl ${ESPHOME_LDGEN_DEP_EXCLUDE})
|
||||
list(REMOVE_ITEM ldgen_libraries_expr "$<TARGET_FILE:${esphome_ldgen_excl}>")
|
||||
endforeach()
|
||||
endif()
|
||||
"""
|
||||
|
||||
|
||||
def _patch_ldgen_cmake(framework_path: Path) -> None:
|
||||
"""Let projects drop their app archive from ldgen's inputs.
|
||||
|
||||
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")
|
||||
except OSError as e:
|
||||
_LOGGER.warning(
|
||||
"Could not apply the ldgen dependency patch to %s (%s); skipping.",
|
||||
ldgen_cmake,
|
||||
e,
|
||||
)
|
||||
return
|
||||
if "ESPHOME_LDGEN_DEP_EXCLUDE" in content:
|
||||
return
|
||||
if "${ldgen_deps}" not in content or content.count(_LDGEN_JOIN_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_JOIN_ANCHOR, _LDGEN_DEP_FILTER + _LDGEN_JOIN_ANCHOR),
|
||||
)
|
||||
_LOGGER.info("Patched %s to honor ESPHOME_LDGEN_DEP_EXCLUDE.", ldgen_cmake)
|
||||
|
||||
|
||||
def _prefetch_idf_tool_archives(
|
||||
framework_path: Path,
|
||||
targets_str: str,
|
||||
@@ -967,9 +916,6 @@ def _check_esphome_idf_framework_install(
|
||||
# check recovers on the next build.
|
||||
_patch_tools_json_demote_unused_tools(framework_path)
|
||||
|
||||
# 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
|
||||
stored_stamp = None if fresh_extract else _read_stamp(env_stamp_file)
|
||||
install = fresh_extract
|
||||
|
||||
@@ -9,6 +9,7 @@ from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from esphome.build_gen import espidf as build_gen_espidf
|
||||
from esphome.components.esp32 import (
|
||||
KEY_COMPONENTS,
|
||||
KEY_ESP32,
|
||||
@@ -164,19 +165,25 @@ def test_has_discovered_components_after_configure(tmp_path: Path) -> None:
|
||||
|
||||
|
||||
@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_emits_ldgen_override(
|
||||
minimal: bool, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""Both renders override the ldgen dep walker to drop the app archive,
|
||||
after include(project.cmake) which defines the original."""
|
||||
monkeypatch.delenv("ESPHOME_LDGEN_FULL_DEPS", raising=False)
|
||||
content = _render(minimal=minimal)
|
||||
assert build_gen_espidf._LDGEN_OVERRIDE in content
|
||||
assert content.index("tools/cmake/project.cmake") < content.index(
|
||||
"function(__ldgen_get_lib_deps_of_target"
|
||||
)
|
||||
|
||||
|
||||
def test_get_project_cmakelists_ldgen_full_deps_escape_hatch(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""ESPHOME_LDGEN_FULL_DEPS restores stock ldgen dependencies."""
|
||||
"""ESPHOME_LDGEN_FULL_DEPS restores stock ldgen behavior."""
|
||||
monkeypatch.setenv("ESPHOME_LDGEN_FULL_DEPS", "true")
|
||||
assert "ESPHOME_LDGEN_DEP_EXCLUDE" not in _render()
|
||||
assert "__ldgen_get_lib_deps_of_target" not in _render()
|
||||
|
||||
|
||||
def test_get_project_cmakelists_uses_supplied_builtin_components() -> None:
|
||||
|
||||
@@ -20,7 +20,6 @@ from unittest.mock import MagicMock, patch
|
||||
import pytest
|
||||
|
||||
from esphome.espidf.framework import (
|
||||
_LDGEN_DEP_FILTER,
|
||||
ESPHOME_STAMP_FILE,
|
||||
STAMP_SCHEMA_VERSION,
|
||||
_ccache_env,
|
||||
@@ -34,7 +33,6 @@ 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,
|
||||
@@ -403,7 +401,6 @@ 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),
|
||||
@@ -867,93 +864,6 @@ 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)
|
||||
|
||||
list(JOIN ldgen_libraries_expr "\\n" ldgen_libraries_str)
|
||||
file(WRITE ${build_dir}/ldgen_libraries.in "${ldgen_libraries_str}")
|
||||
|
||||
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 _LDGEN_DEP_FILTER in content
|
||||
# The libraries list must be filtered before it is serialized to disk
|
||||
assert content.index("REMOVE_ITEM ldgen_libraries_expr") < content.index(
|
||||
"list(JOIN ldgen_libraries_expr"
|
||||
)
|
||||
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_unreadable_file_warns_and_skips(
|
||||
tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
_write_ldgen_cmake(tmp_path, _LDGEN_CMAKE_STOCK)
|
||||
with patch.object(Path, "read_text", side_effect=OSError("boom")):
|
||||
_patch_ldgen_cmake(tmp_path)
|
||||
assert "Could not apply the ldgen dependency patch" in caplog.text
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"content",
|
||||
[
|
||||
pytest.param(
|
||||
_LDGEN_CMAKE_STOCK.replace("${ldgen_deps}", "${other_deps}"),
|
||||
id="no_ldgen_deps",
|
||||
),
|
||||
pytest.param(
|
||||
_LDGEN_CMAKE_STOCK
|
||||
+ '\n list(JOIN ldgen_libraries_expr "\\n" ldgen_libraries_str)\n',
|
||||
id="duplicate_join",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_patch_ldgen_cmake_unexpected_layout_skips(
|
||||
content: str, tmp_path: Path, caplog: pytest.LogCaptureFixture
|
||||
) -> None:
|
||||
"""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
|
||||
assert "does not match the expected layout" in caplog.text
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _prefetch_idf_tool_archives
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user