From 6479e079d2331077c2eafb4aff0ac9a2b8ceeff7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 1 Oct 2026 19:39:18 -0500 Subject: [PATCH] [espidf] Build all and size in one ninja run (#19997) --- esphome/build_gen/espidf.py | 4 ++ esphome/espidf/size_summary.py | 2 +- esphome/espidf/toolchain.py | 21 ++++----- tests/unit_tests/build_gen/test_espidf.py | 6 +++ tests/unit_tests/test_espidf_toolchain.py | 52 ++++++++++++----------- 5 files changed, 50 insertions(+), 35 deletions(-) diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 3838d07602..50c0abf1b2 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -351,11 +351,15 @@ project({CORE.name}) # 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. +# The map is a BYPRODUCT so ninja knows the link writes it; IDF's size +# target depends on the map and can then be built in the same run as all. +# IDF's cmakev2 declares the map itself, so drop this line on that switch. add_custom_command( TARGET ${{CMAKE_PROJECT_NAME}}.elf POST_BUILD COMMAND ${{PYTHON}} -m esp_idf_size {size_ng_flag} --format=json2 -o ${{CMAKE_BINARY_DIR}}/esp_idf_size.json ${{CMAKE_PROJECT_NAME}}.map + BYPRODUCTS ${{CMAKE_BINARY_DIR}}/${{CMAKE_PROJECT_NAME}}.map WORKING_DIRECTORY ${{CMAKE_BINARY_DIR}} VERBATIM ) diff --git a/esphome/espidf/size_summary.py b/esphome/espidf/size_summary.py index ffe97ba618..9fc1a6b3a9 100644 --- a/esphome/espidf/size_summary.py +++ b/esphome/espidf/size_summary.py @@ -1,6 +1,6 @@ """PlatformIO-format RAM/Flash one-liners after a native ESP-IDF build. -The ninja ``size`` target (run after ``all`` in +The ninja ``size`` target (built together with ``all`` in ``toolchain.run_compile``) prints the per-region table inline as part of the build. This module adds two summary lines underneath, byte-identical to PlatformIO's output: diff --git a/esphome/espidf/toolchain.py b/esphome/espidf/toolchain.py index 2e8cc0c632..4202ee71ce 100644 --- a/esphome/espidf/toolchain.py +++ b/esphome/espidf/toolchain.py @@ -428,21 +428,21 @@ def _build_jobs(config) -> int | None: def _run_ninja( - target: str, - *, + *targets: str, verbose: bool, jobs: int | None, progress: bool = False, extra_env: dict[str, str] | None = None, ) -> int: - """Build one ninja target, with the flags and env idf.py uses.""" + """Build ninja targets in one run, with the flags and env idf.py uses.""" cmd = [_get_idf_tool("ninja")] if jobs is not None: cmd += ["-j", str(jobs)] if verbose: cmd.append("-v") - cmd.append(target) - log_path = _build_dir() / "log" / f"ninja_{Path(target).name}_output.log" + cmd += targets + log_name = "_".join(Path(t).name for t in targets) + log_path = _build_dir() / "log" / f"ninja_{log_name}_output.log" rc = run_build_tool( cmd, cwd=_build_dir(), @@ -452,7 +452,7 @@ def _run_ninja( log_path=log_path, ) if rc != 0: - _LOGGER.error("ninja %s failed with exit code %d", target, rc) + _LOGGER.error("ninja %s failed with exit code %d", " ".join(targets), rc) _print_hints(log_path) return rc @@ -845,10 +845,11 @@ def run_compile(config, verbose: bool) -> int: write_pch_checksum() - # idf.py's ``build size``, minus the second ``ninja all`` it runs first. - rc = _run_ninja("all", verbose=verbose, jobs=jobs, progress=True) - if rc == 0: - rc = _run_ninja("size", verbose=verbose, jobs=jobs, extra_env=_size_env()) + # idf.py's ``build size`` in one ninja run; size needs the map, so it + # runs after the link. + rc = _run_ninja( + "all", "size", verbose=verbose, jobs=jobs, progress=True, extra_env=_size_env() + ) if rc == 0: size_json = CORE.relative_build_path("build", "esp_idf_size.json") partitions = CORE.relative_build_path("partitions.csv") diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index 7ed6201b6a..f8e834a78d 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -222,6 +222,12 @@ def test_get_project_cmakelists_size_command_uses_json2() -> None: assert "--format=json2" 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() + assert "BYPRODUCTS ${CMAKE_BINARY_DIR}/${CMAKE_PROJECT_NAME}.map" in content + + 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_toolchain.py b/tests/unit_tests/test_espidf_toolchain.py index 1aaff7e9ce..620d08fe3b 100644 --- a/tests/unit_tests/test_espidf_toolchain.py +++ b/tests/unit_tests/test_espidf_toolchain.py @@ -534,8 +534,8 @@ def _record_compile_calls( def record_save(components: list[str]) -> None: calls.append(("save", components)) - def record_ninja(target: str, **kwargs: object) -> int: - if target == "all": + def record_ninja(*targets: str, **kwargs: object) -> int: + if "all" in targets: calls.append(("build",)) return 0 @@ -886,7 +886,7 @@ def test_run_compile_full_deps_skips_fragment_check( def test_run_compile_passes_compile_process_limit( setup_core: Path, limit: int | None ) -> None: - """compile_process_limit is the job limit for both ninja runs.""" + """compile_process_limit is the job limit of the one ninja run.""" _setup_build(setup_core) esphome = {} if limit is None else {CONF_COMPILE_PROCESS_LIMIT: limit} @@ -894,8 +894,14 @@ def test_run_compile_passes_compile_process_limit( assert toolchain.run_compile({CONF_ESPHOME: esphome}, verbose=False) == 0 assert mock_run.call_args_list == [ - call("all", verbose=False, jobs=limit, progress=True), - call("size", verbose=False, jobs=limit, extra_env=toolchain._size_env()), + call( + "all", + "size", + verbose=False, + jobs=limit, + progress=True, + extra_env=toolchain._size_env(), + ), ] @@ -1185,14 +1191,17 @@ def test_run_ninja_filters_and_reports_failure( patch.object(toolchain, "_print_hints") as mock_hints, ): mock_run.return_value = 1 - assert toolchain._run_ninja("all", verbose=False, jobs=None, progress=True) == 1 + assert ( + toolchain._run_ninja("all", "size", verbose=False, jobs=None, progress=True) + == 1 + ) log_path = mock_run.call_args.kwargs["log_path"] - assert log_path.name == "ninja_all_output.log" + assert log_path.name == "ninja_all_size_output.log" mock_hints.assert_called_once_with(log_path) - assert mock_run.call_args.args[0] == ["/tools/ninja", "all"] + assert mock_run.call_args.args[0] == ["/tools/ninja", "all", "size"] assert mock_run.call_args.kwargs["filter_lines"] is toolchain.FILTER_IDF_LINES assert mock_run.call_args.kwargs["progress"] is True - assert "ninja all failed with exit code 1" in caplog.text + assert "ninja all size failed with exit code 1" in caplog.text @pytest.mark.parametrize("reconfigure_rc", [0, 5]) @@ -1215,17 +1224,12 @@ def test_run_compile_reconfigures_when_cache_entries_change( assert mock_ninja.called is (reconfigure_rc == 0) -@pytest.mark.parametrize("failing", ["all", "size"]) -def test_run_compile_stops_on_ninja_failure(setup_core: Path, failing: str) -> None: - """A failed build skips size; either failure skips the summary.""" +def test_run_compile_stops_on_ninja_failure(setup_core: Path) -> None: + """A failed ninja run skips the summary.""" _setup_build(setup_core) - with _up_to_date_compile(lambda target, **kw: 7 if target == failing else 0) as ( - mock_ninja, - mock_summary, - ): + with _up_to_date_compile(lambda *targets, **kw: 7) as (mock_ninja, mock_summary): assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == 7 - targets = [c.args[0] for c in mock_ninja.call_args_list] - assert targets == (["all"] if failing == "all" else ["all", "size"]) + assert [c.args for c in mock_ninja.call_args_list] == [("all", "size")] mock_summary.assert_not_called() @@ -1236,11 +1240,11 @@ def test_run_compile_testing_mode_builds_memory_ld_first( """Testing mode builds and patches memory.ld before the main build.""" _setup_build(setup_core) CORE.testing_mode = True - targets: list[str] = [] + targets: list[tuple[str, ...]] = [] - def record(target: str, **kwargs: object) -> int: - targets.append(target) - return memory_ld_rc if target.endswith("memory.ld") else 0 + def record(*run_targets: str, **kwargs: object) -> int: + targets.append(run_targets) + return memory_ld_rc if run_targets[0].endswith("memory.ld") else 0 with ( _up_to_date_compile(record), @@ -1249,10 +1253,10 @@ def test_run_compile_testing_mode_builds_memory_ld_first( assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == memory_ld_rc memory_ld = str(Path("esp-idf", "esp_system", "ld", "memory.ld")) if memory_ld_rc: - assert targets == [memory_ld] + assert targets == [(memory_ld,)] mock_patch.assert_not_called() else: - assert targets == [memory_ld, "all", "size"] + assert targets == [(memory_ld,), ("all", "size")] mock_patch.assert_called_once()