diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 50c0abf1b2..42a1de5a4b 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -66,6 +66,43 @@ else() "app edits will regenerate sections.ld.") endif()""" +# lwip sources that compile to empty objects with the option off (their own +# #if guard). (option, regex valid for both Python and CMake); a source is +# only dropped when its option is defined and off, so a renamed option +# keeps it. +LWIP_EMPTY_SOURCES: tuple[tuple[str, str], ...] = ( + ("CONFIG_LWIP_PPP_SUPPORT", "/netif/ppp/"), + ("CONFIG_LWIP_IPV6", "/core/ipv6/"), + ("CONFIG_LWIP_AUTOIP", "/core/ipv4/autoip[.]c$"), + ("CONFIG_LWIP_STATS", "/core/stats[.]c$"), +) +# Drift guard only: keep every lwip source. +LWIP_FULL_SOURCES_ENV = "ESPHOME_LWIP_FULL_SOURCES" + +# Drops the empty objects after project(), once the lwip target exists. +_LWIP_EMPTY_SOURCES_FILTER = f"""\ +idf_build_get_property(esphome_build_components BUILD_COMPONENTS) +if(lwip IN_LIST esphome_build_components AND NOT DEFINED ENV{{{LWIP_FULL_SOURCES_ENV}}}) + idf_component_get_property(esphome_lwip_lib lwip COMPONENT_LIB) + get_target_property(esphome_lwip_srcs ${{esphome_lwip_lib}} SOURCES) +@FILTERS@ + set_property(TARGET ${{esphome_lwip_lib}} PROPERTY SOURCES ${{esphome_lwip_srcs}}) +endif()""" + + +def lwip_empty_source_gate(option: str, regex: str) -> str: + return ( + f" if(DEFINED {option} AND NOT {option})\n" + f' list(FILTER esphome_lwip_srcs EXCLUDE REGEX "{regex}")\n' + " endif()" + ) + + +def _lwip_empty_sources_filter() -> str: + gates = "\n".join(lwip_empty_source_gate(*entry) for entry in LWIP_EMPTY_SOURCES) + return _LWIP_EMPTY_SOURCES_FILTER.replace("@FILTERS@", gates) + + # Runs after project() so the walk has happened; catches the remaining # silent path where the top-level out-var was renamed. _LDGEN_OVERRIDE_CHECK = """\ @@ -348,6 +385,8 @@ project({CORE.name}) {ldgen_override_check} +{_lwip_empty_sources_filter()} + # Emit per-memory-type JSON size data for ESPHome to read post-build. # json2 stays small; raw dumps every symbol (~2s on a large map) and # this command runs inside the link edge, blocking everything downstream. diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 4202ee71ce..effb817f35 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -366,7 +366,9 @@ def _tool_env() -> dict[str, str]: return env -def run_reconfigure(verbose: bool = False) -> int: +def run_reconfigure( + verbose: bool = False, extra_env: dict[str, str] | None = None +) -> int: """Run the CMake configure, with the arguments idf.py uses.""" build_dir = _build_dir() build_dir.mkdir(parents=True, exist_ok=True) @@ -389,7 +391,7 @@ def run_reconfigure(verbose: bool = False) -> int: rc = run_build_tool( cmd, cwd=build_dir, - env=_tool_env(), + env={**_tool_env(), **(extra_env or {})}, filter_lines=None if verbose else FILTER_IDF_LINES, log_path=log_path, ) diff --git a/script/check_idf_py_equivalence.py b/script/check_idf_py_equivalence.py index 8ef3577eed..cbd55d57cc 100755 --- a/script/check_idf_py_equivalence.py +++ b/script/check_idf_py_equivalence.py @@ -59,6 +59,15 @@ VERSION_DRIFT = ( "ESPHome reads ESP-IDF version {ours!r} from {source} but idf_tools reports " "{theirs!r}; update read_idf_version_{source} in esphome/espidf/framework.py" ) +LWIP_NOT_EMPTY = ( + "lwip source {source} compiles to a non-empty object with {option} off; " + "drop it from LWIP_EMPTY_SOURCES in esphome/build_gen/espidf.py" +) +LWIP_NOTHING_MATCHED = ( + "no lwip object matched {regex!r} for {option}; the lwip layout or the " + "pattern in esphome/build_gen/espidf.py changed" +) +LWIP_NM_FAILED = "nm failed on lwip object {source}: {error}" WORK_SUFFIXES = (".obj", ".o", ".a", ".elf", ".map", ".bin", ".ld") DEFAULT_GLOB = "tests/test_build_components/build/.esphome/build/*" @@ -108,6 +117,51 @@ def _log_problems( return problems +def _lwip_empty_source_problems(build_path: Path) -> list[str]: + """Compile the lwip sources the generated CMakeLists drops; any with + symbols is a problem. Leaves the tree configured with every source.""" + # pylint: disable=protected-access + from esphome.build_gen.espidf import LWIP_EMPTY_SOURCES, LWIP_FULL_SOURCES_ENV + from esphome.espidf import toolchain + + if (rc := toolchain.run_reconfigure(extra_env={LWIP_FULL_SOURCES_ENV: "1"})) != 0: + return [f"CMake configure with every lwip source failed with exit code {rc}"] + if rc := toolchain._run_ninja("esp-idf/lwip/liblwip.a", verbose=False, jobs=None): + return [f"building every lwip source failed with exit code {rc}"] + build = build_path / "build" + config = json.loads( + (build / "config" / "sdkconfig.json").read_text(encoding="utf-8") + ) + objects = [ + obj.as_posix().removesuffix(".obj") + for obj in (build / "esp-idf" / "lwip").rglob("*.obj") + ] + nm = toolchain._parse_cmakecache(build / "CMakeCache.txt")["CMAKE_NM"] + problems = [] + for option, regex in LWIP_EMPTY_SOURCES: + # Absent means the option is invisible here; the filter keeps those. + if config.get(option.removeprefix("CONFIG_"), True): + continue + matched = [source for source in objects if re.search(regex, source)] + if not matched: + problems.append(LWIP_NOTHING_MATCHED.format(regex=regex, option=option)) + for source in matched: + name = Path(source).name + result = subprocess.run( + [nm, "--defined-only", f"{source}.obj"], + capture_output=True, + text=True, + check=False, + ) + if result.returncode: + problems.append( + LWIP_NM_FAILED.format(source=name, error=result.stderr.strip()) + ) + elif result.stdout.strip(): + problems.append(LWIP_NOT_EMPTY.format(source=name, option=option)) + return problems + + def _setup_core(build_path: Path, description: dict) -> tuple[str, str]: """Point CORE at the tree so ESPHome resolves the same IDF env as the build.""" from esphome.components.esp32.const import KEY_ESP32, KEY_IDF_VERSION, KEY_VARIANT @@ -202,7 +256,8 @@ def check(build_path: Path) -> list[str]: problems.append(f"idf.py dropped {out} from {log}") elif mtimes_before.get(key) != mtimes_after[key]: problems.append(f"idf.py rebuilt {out}") - return problems + # Last: it reconfigures the tree, which would otherwise relink above. + return problems or _lwip_empty_source_problems(build_path) def main() -> int: diff --git a/tests/script/test_check_idf_py_equivalence.py b/tests/script/test_check_idf_py_equivalence.py index 9ce52a6df5..f2b798ff21 100644 --- a/tests/script/test_check_idf_py_equivalence.py +++ b/tests/script/test_check_idf_py_equivalence.py @@ -103,6 +103,7 @@ def _run_check( patch.object(framework, "read_idf_version_txt", return_value=versions[0]), patch.object(framework, "read_idf_version_header", return_value=versions[1]), patch.object(framework, "idf_tools_version", return_value=versions[2]), + patch.object(guard, "_lwip_empty_source_problems", return_value=[]), patch.object(guard.subprocess, "run", side_effect=run), patch.dict(os.environ), ): @@ -319,6 +320,99 @@ def test_main_without_build_trees( assert "No native ESP-IDF build tree found" in capsys.readouterr().out +def _make_lwip_tree(tmp_path: Path, objects: list[str], config: dict) -> Path: + """A tree with lwip objects, their sdkconfig.json and an nm in the cache.""" + tree = _make_tree(tmp_path) + objdir = tree / "build" / "esp-idf" / "lwip" / "CMakeFiles" / "__idf_lwip.dir" + for name in objects: + (objdir / name).parent.mkdir(parents=True, exist_ok=True) + (objdir / name).write_bytes(b"x") + (tree / "build" / "config").mkdir(parents=True, exist_ok=True) + (tree / "build" / "config" / "sdkconfig.json").write_text(json.dumps(config)) + with (tree / "build" / "CMakeCache.txt").open("a") as cache: + cache.write("CMAKE_NM:FILEPATH=/tools/nm\n") + return tree + + +def _run_lwip_check( + tree: Path, + non_empty: set[str] = frozenset(), + failing: set[str] = frozenset(), + calls: list[list[str]] | None = None, +) -> list[str]: + """Run the lwip check with nm faked; ``calls`` collects the nm commands.""" + + def run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess: + if calls is not None: + calls.append(cmd) + name = Path(cmd[-1]).name + if name in failing: + return subprocess.CompletedProcess(cmd, 1, "", "bad object") + return subprocess.CompletedProcess( + cmd, 0, "symbol\n" if name in non_empty else "", "" + ) + + with ( + patch.object(toolchain, "run_reconfigure", return_value=0) as reconfigure, + patch.object(toolchain, "_run_ninja", return_value=0), + patch.object(guard.subprocess, "run", side_effect=run), + ): + problems = guard._lwip_empty_source_problems(tree) + reconfigure.assert_called_once_with( + extra_env={build_gen.LWIP_FULL_SOURCES_ENV: "1"} + ) + return problems + + +def test_lwip_check_inspects_only_the_dropped_sources(tmp_path: Path) -> None: + """An option that is on, or absent (invisible), keeps its sources unchecked.""" + tree = _make_lwip_tree( + tmp_path, + [ + "lwip/src/netif/ppp/auth.c.obj", + "lwip/src/core/ipv6/ip6.c.obj", + "lwip/src/core/ipv4/autoip.c.obj", + ], + {"LWIP_PPP_SUPPORT": False, "LWIP_IPV6": True}, + ) + calls: list[list[str]] = [] + assert _run_lwip_check(tree, calls=calls) == [] + assert [Path(c[-1]).name for c in calls] == ["auth.c.obj"] + + +def test_lwip_check_flags_a_dropped_source_with_symbols(tmp_path: Path) -> None: + tree = _make_lwip_tree( + tmp_path, ["lwip/src/netif/ppp/auth.c.obj"], {"LWIP_PPP_SUPPORT": False} + ) + assert _run_lwip_check(tree, non_empty={"auth.c.obj"}) == [ + guard.LWIP_NOT_EMPTY.format(source="auth.c", option="CONFIG_LWIP_PPP_SUPPORT") + ] + + +def test_lwip_check_flags_a_failed_nm(tmp_path: Path) -> None: + """A broken nm must not pass as an empty object.""" + tree = _make_lwip_tree( + tmp_path, ["lwip/src/netif/ppp/auth.c.obj"], {"LWIP_PPP_SUPPORT": False} + ) + assert _run_lwip_check(tree, failing={"auth.c.obj"}) == [ + guard.LWIP_NM_FAILED.format(source="auth.c", error="bad object") + ] + + +def test_lwip_check_fails_per_pattern_that_matched_nothing(tmp_path: Path) -> None: + """A stale pattern is reported even while the others still match.""" + tree = _make_lwip_tree( + tmp_path, + ["lwip/src/netif/ppp/auth.c.obj"], + {"LWIP_PPP_SUPPORT": False, "LWIP_STATS": False}, + ) + assert _run_lwip_check(tree) == [ + guard.LWIP_NOTHING_MATCHED.format( + regex="/core/stats[.]c$", option="CONFIG_LWIP_STATS" + ) + ] + + @pytest.mark.parametrize(("problems", "rc"), [([], 0), (["idf.py changed x"], 1)]) def test_main_reports_each_tree( tmp_path: Path, diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index f8e834a78d..2c95c61b37 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -222,6 +222,24 @@ def test_get_project_cmakelists_size_command_uses_json2() -> None: assert "--format=json2" in content +def test_get_project_cmakelists_drops_empty_lwip_sources() -> None: + """The filter comes after project(), where the lwip target exists.""" + from esphome.build_gen.espidf import ( + LWIP_EMPTY_SOURCES, + LWIP_FULL_SOURCES_ENV, + lwip_empty_source_gate, + ) + + content = _render() + filter_at = content.index( + "set_property(TARGET ${esphome_lwip_lib} PROPERTY SOURCES" + ) + assert filter_at > content.index("project(") + assert f"NOT DEFINED ENV{{{LWIP_FULL_SOURCES_ENV}}}" in content + for entry in LWIP_EMPTY_SOURCES: + assert lwip_empty_source_gate(*entry) in content + + def test_get_project_cmakelists_declares_map_as_link_byproduct() -> None: """The link declares the map so size can build in the same ninja run.""" content = _render()