Drop the app archive from ldgen_libraries too and add a fragment check

This commit is contained in:
J. Nick Koston
2026-08-28 10:24:26 -05:00
parent 17defaf361
commit c290a508f6
4 changed files with 95 additions and 11 deletions
+12 -8
View File
@@ -686,21 +686,24 @@ def _patch_tools_json_demote_unused_tools(framework_path: Path) -> None:
)
_LDGEN_COMMAND_ANCHOR = " add_custom_command(\n"
_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 DEPENDS.
# 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 rebuilding it cannot change
# the generated linker script. Set by the ESPHome project CMakeLists.
# 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()
add_custom_command(
"""
def _patch_ldgen_cmake(framework_path: Path) -> None:
"""Let projects drop their app archive from the sections.ld DEPENDS.
"""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
@@ -720,7 +723,7 @@ def _patch_ldgen_cmake(framework_path: Path) -> None:
return
if "ESPHOME_LDGEN_DEP_EXCLUDE" in content:
return
if "${ldgen_deps}" not in content or content.count(_LDGEN_COMMAND_ANCHOR) != 1:
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).",
@@ -728,7 +731,8 @@ def _patch_ldgen_cmake(framework_path: Path) -> None:
)
return
write_file_if_changed(
ldgen_cmake, content.replace(_LDGEN_COMMAND_ANCHOR, _LDGEN_DEP_FILTER)
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)
+34 -1
View File
@@ -24,7 +24,7 @@ from esphome.core import CORE, EsphomeError
from esphome.espidf import variant_to_idf_target
from esphome.espidf.framework import check_esp_idf_install, get_framework_env
from esphome.espidf.size_summary import print_summary
from esphome.helpers import add_git_ceiling_directory, write_file
from esphome.helpers import add_git_ceiling_directory, get_bool_env, write_file
_LOGGER = logging.getLogger(__name__)
@@ -479,6 +479,36 @@ def _patch_memory_segments():
_LOGGER.warning("Could not patch memory segments in %s", memory_ld)
_LDGEN_FRAGMENTS_RE = re.compile(r'--fragments-list\s+"([^"]+)"')
_APP_ARCHIVE_MAPPED_RE = re.compile(r"^archive:\s*libsrc\.a\s*$", re.MULTILINE)
def _warn_if_app_archive_mapped() -> None:
"""Belt for the ldgen dependency exclusion (see build_gen/espidf.py).
The exclusion is safe only while no linker fragment names the app
archive; if one ever did, ldgen would silently skip remapping it, so
surface that loudly instead of relying on the invariant forever.
"""
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")
):
_LOGGER.warning(
"Linker fragment %s maps the app archive; its entries may be "
"skipped. Set ESPHOME_LDGEN_FULL_DEPS=1 and rebuild.",
fragment,
)
return
except OSError as e:
_LOGGER.debug("Skipping ldgen fragment check: %s", e)
def run_compile(config, verbose: bool) -> int:
"""Compile the ESP-IDF project.
@@ -505,6 +535,9 @@ def run_compile(config, verbose: bool) -> int:
if path.is_file():
os.utime(path)
if not get_bool_env("ESPHOME_LDGEN_FULL_DEPS"):
_warn_if_app_archive_mapped()
# In testing mode, generate the linker script first, patch DRAM/IRAM sizes,
# then build. memory.ld is regenerated by ninja during the build phase,
# so we must patch after it's generated but before linking (same timing
+10 -2
View File
@@ -876,6 +876,9 @@ _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"
@@ -899,6 +902,10 @@ def test_patch_ldgen_cmake_inserts_guarded_filter(tmp_path: Path) -> None:
_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
@@ -931,8 +938,9 @@ def test_patch_ldgen_cmake_unreadable_file_warns_and_skips(
id="no_ldgen_deps",
),
pytest.param(
_LDGEN_CMAKE_STOCK + "\n add_custom_command(\n OUTPUT x)\n",
id="duplicate_command",
_LDGEN_CMAKE_STOCK
+ '\n list(JOIN ldgen_libraries_expr "\\n" ldgen_libraries_str)\n',
id="duplicate_join",
),
],
)
+39
View File
@@ -623,6 +623,45 @@ def test_component_cache_ignores_corrupt_file(setup_core: Path, tmp_path: Path)
assert toolchain.load_cached_builtin_components() is None
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)
frag_list = ";".join(str(f) for f in fragments)
(build_dir / "build.ninja").write_text(
f' COMMAND = python ldgen.py --fragments-list "{frag_list}" --input x\n'
)
def test_warn_if_app_archive_mapped_warns(
setup_core: Path, tmp_path: Path, caplog: pytest.LogCaptureFixture
) -> None:
"""A fragment naming the app archive 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")
_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_clean(
setup_core: Path, tmp_path: Path, caplog: pytest.LogCaptureFixture
) -> None:
"""Normal fragments produce no warning; missing files are non-fatal."""
_setup_build(setup_core)
frag = tmp_path / "linker.lf"
frag.write_text("[mapping:freertos]\narchive: libfreertos.a\n")
_write_fragments_build_ninja(tmp_path, [frag, tmp_path / "missing.lf"])
toolchain._warn_if_app_archive_mapped()
assert "maps the app archive" not in caplog.text
def test_warn_if_app_archive_mapped_no_build_ninja(setup_core: Path) -> None:
"""No build.ninja yet is a quiet no-op."""
_setup_build(setup_core)
toolchain._warn_if_app_archive_mapped()
def test_run_compile_passes_compile_process_limit(setup_core: Path) -> None:
"""compile_process_limit is forwarded to run_idf_py as the job limit."""
_setup_build(setup_core)