From 1e1b591a7087d8d942963f1a799c9cdd6da9793c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 28 Aug 2026 10:37:09 -0500 Subject: [PATCH] Fail closed in CI with ESPHOME_LDGEN_STRICT --- .github/workflows/ci-docker.yml | 1 + esphome/build_gen/espidf.py | 17 ++++++++++++++--- esphome/espidf/toolchain.py | 11 +++++++---- tests/unit_tests/build_gen/test_espidf.py | 20 ++++++++++++++++++-- tests/unit_tests/test_espidf_toolchain.py | 19 ++++++++++++++++++- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci-docker.yml b/.github/workflows/ci-docker.yml index 829bdd5f98..d0aef53452 100644 --- a/.github/workflows/ci-docker.yml +++ b/.github/workflows/ci-docker.yml @@ -219,5 +219,6 @@ jobs: run: | docker run --rm \ -v "${{ github.workspace }}/docker/test_configs:/config" \ + -e ESPHOME_LDGEN_STRICT=1 \ "ghcr.io/esphome/esphome-amd64:${{ needs.check-docker.outputs.tag }}" \ compile "${{ matrix.id }}.yaml" diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 376f389d29..b87a60e2f4 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -50,12 +50,15 @@ if(COMMAND __ldgen_get_lib_deps_of_target) 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 " + message(@SEVERITY@ "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() +else() + message(@MISSING@ "ESPHome ldgen override target not found; " + "app edits will regenerate sections.ld.") endif()""" @@ -148,8 +151,16 @@ def get_project_cmakelists( ) # 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 + # _LDGEN_OVERRIDE. ESPHOME_LDGEN_FULL_DEPS=1 restores stock behavior; + # ESPHOME_LDGEN_STRICT=1 (CI) fails the configure when an IDF bump + # breaks the override instead of degrading to stock deps. + if get_bool_env("ESPHOME_LDGEN_FULL_DEPS"): + ldgen_override = "" + else: + strict = get_bool_env("ESPHOME_LDGEN_STRICT") + ldgen_override = _LDGEN_OVERRIDE.replace( + "@SEVERITY@", "FATAL_ERROR" if strict else "WARNING" + ).replace("@MISSING@", "FATAL_ERROR" if strict else "STATUS") # CMake variables registered via cg.add_cmake_arg(). Emitted before # include(project.cmake) so values like EXCLUDE_COMPONENTS are already diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 0ad563bf52..23e1d454ba 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -497,11 +497,14 @@ def _warn_if_app_archive_mapped() -> None: 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, + msg = ( + f"Linker fragment {fragment} maps the app archive; its " + "entries may be skipped. Set ESPHOME_LDGEN_FULL_DEPS=1 " + "and rebuild." ) + if get_bool_env("ESPHOME_LDGEN_STRICT"): + raise EsphomeError(msg) + _LOGGER.warning("%s", msg) return except OSError as e: _LOGGER.debug("Skipping ldgen fragment check: %s", e) diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index e7a4980a10..952c8323b9 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -9,7 +9,6 @@ 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, @@ -171,13 +170,30 @@ def test_get_project_cmakelists_emits_ldgen_override( """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) + monkeypatch.delenv("ESPHOME_LDGEN_STRICT", raising=False) content = _render(minimal=minimal) - assert build_gen_espidf._LDGEN_OVERRIDE in content + assert "REMOVE_ITEM ${out_list_var} idf::src __idf_src" in content + assert 'message(WARNING "ESPHome ldgen app archive exclusion' in content + assert 'message(STATUS "ESPHome ldgen override target not found' in content assert content.index("tools/cmake/project.cmake") < content.index( "function(__ldgen_get_lib_deps_of_target" ) +def test_get_project_cmakelists_ldgen_strict_fails_closed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """ESPHOME_LDGEN_STRICT turns both degradation paths into hard errors so + CI fails right away when an IDF bump breaks the override.""" + monkeypatch.delenv("ESPHOME_LDGEN_FULL_DEPS", raising=False) + monkeypatch.setenv("ESPHOME_LDGEN_STRICT", "1") + content = _render() + assert 'message(FATAL_ERROR "ESPHome ldgen app archive exclusion' in content + assert 'message(FATAL_ERROR "ESPHome ldgen override target not found' in content + assert "@SEVERITY@" not in content + assert "@MISSING@" not in content + + def test_get_project_cmakelists_ldgen_full_deps_escape_hatch( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/unit_tests/test_espidf_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index 622327bb3d..707d26d52d 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -633,9 +633,13 @@ def _write_fragments_build_ninja(tmp_path: Path, fragments: list[Path]) -> None: def test_warn_if_app_archive_mapped_warns( - setup_core: Path, tmp_path: Path, caplog: pytest.LogCaptureFixture + setup_core: Path, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, ) -> None: """A fragment naming the app archive triggers the loud warning.""" + monkeypatch.delenv("ESPHOME_LDGEN_STRICT", raising=False) _setup_build(setup_core) frag = tmp_path / "linker.lf" frag.write_text("[mapping:evil]\narchive: libsrc.a\nentries:\n * (noflash)\n") @@ -644,6 +648,19 @@ def test_warn_if_app_archive_mapped_warns( assert "maps the app archive" in caplog.text +def test_warn_if_app_archive_mapped_strict_raises( + setup_core: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Under ESPHOME_LDGEN_STRICT a mapped app archive fails the build.""" + monkeypatch.setenv("ESPHOME_LDGEN_STRICT", "1") + _setup_build(setup_core) + frag = tmp_path / "linker.lf" + frag.write_text("[mapping:evil]\narchive: libsrc.a\n") + _write_fragments_build_ninja(tmp_path, [frag]) + with pytest.raises(EsphomeError, match="maps the app archive"): + toolchain._warn_if_app_archive_mapped() + + def test_warn_if_app_archive_mapped_clean( setup_core: Path, tmp_path: Path, caplog: pytest.LogCaptureFixture ) -> None: