From 337ecfb8f98185d72453bd977cde693822127520 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 25 Aug 2026 13:58:34 -0500 Subject: [PATCH] Select the pch TU by source path, expand response files, and key the failure marker on the command --- esphome/build_gen/espidf.py | 54 +++++++++++++------ esphome/build_helpers/idedata.py | 6 +-- esphome/espidf/framework.py | 2 + tests/unit_tests/build_gen/test_espidf.py | 30 +++++++++-- .../unit_tests/build_helpers/test_idedata.py | 4 +- 5 files changed, 71 insertions(+), 25 deletions(-) diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 1c21d138e7..1d4ecda45c 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -1,11 +1,16 @@ """ESP-IDF direct build generator for ESPHome.""" +import hashlib import json import logging from pathlib import Path import subprocess -from esphome.build_helpers.idedata import is_launcher, split_command +from esphome.build_helpers.idedata import ( + expand_response_files, + is_launcher, + split_command, +) from esphome.build_helpers.pch import ( PCH_CORE_HEADER, PCH_HEADER_NAME, @@ -49,6 +54,13 @@ _PCH_HEADERS = ( # _pch_cmake() and prepare_pch() for the layout rationale _PCH_BUILD_HEADER = f"build/{PCH_HEADER_NAME}" +# Compile-command tokens dropped when retargeting a TU's flags at the +# prefix header: source/output/depfile flags with an argument, and the +# argument-less depfile flags (the pch compile must not touch depfiles) +_PCH_STRIP_FLAGS_WITH_ARG = frozenset({"-include", "-o", "-c", "-MT", "-MF", "-MQ"}) +_PCH_STRIP_FLAGS = frozenset({"-MD", "-MMD", "-MP", "-MM", "-M"}) +_CXX_SOURCE_SUFFIXES = (".cpp", ".cc", ".cxx") + # Replaces the IDF default C++ standard (-std=gnu++2b appended to # CXX_COMPILE_OPTIONS by project.cmake's __build_init) with the one set via # cg.set_cpp_standard(). Emitted between include(project.cmake) and project(), @@ -329,25 +341,30 @@ target_compile_options(${{COMPONENT_LIB}} PRIVATE def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str] | None: """The exact src C++ flags from compile_commands.json, retargeted at - the header; None when no configured C++ TU is available yet.""" + the header; None (logged) when no configured C++ TU is available yet.""" try: entries = json.loads( (build_dir / "compile_commands.json").read_text(encoding="utf-8") ) - except (OSError, json.JSONDecodeError): + except (OSError, json.JSONDecodeError) as err: + _LOGGER.debug("No usable compile database, skipping pch: %s", err) return None + src_prefix = str(CORE.relative_src_path()) entry = next( ( e for e in entries - if "__idf_src.dir" in e.get("command", "") - and e.get("file", "").endswith((".cpp", ".cc", ".cxx")) + if e.get("file", "").startswith(src_prefix) + and e.get("file", "").endswith(_CXX_SOURCE_SUFFIXES) ), None, ) if entry is None: + _LOGGER.debug("No src C++ entry in the compile database, skipping pch") return None - tokens = split_command(entry["command"]) + tokens = expand_response_files( + split_command(entry["command"]), Path(entry.get("directory", build_dir)) + ) # A DB recorded with ccache enabled prefixes the compiler with the # launcher; the .gch must be compiled directly if tokens and is_launcher(tokens[0]): @@ -355,10 +372,11 @@ def _pch_compile_command(build_dir: Path, header: Path, gch: Path) -> list[str] args: list[str] = [] arg_it = iter(tokens) for tok in arg_it: - # Strip the source/output and any -include (the header holds it) - if tok in ("-include", "-o", "-c"): + if tok in _PCH_STRIP_FLAGS_WITH_ARG: next(arg_it, None) continue + if tok in _PCH_STRIP_FLAGS: + continue args.append(tok) return [*args, "-x", "c++-header", "-c", str(header), "-o", str(gch)] @@ -400,20 +418,26 @@ def prepare_pch() -> None: and sum_path.read_text(encoding="utf-8").strip() == checksum ): return + cmd = _pch_compile_command(build_dir, header, gch) + if cmd is None: + return + # Keyed on the checksum and the compile command: a failure caused by + # the command alone must retry when the command changes + marker_key = f"{checksum} {hashlib.sha256(' '.join(cmd).encode()).hexdigest()}" failed_marker = Path(f"{gch}.failed") if ( failed_marker.is_file() - and failed_marker.read_text(encoding="utf-8").strip() == checksum + and failed_marker.read_text(encoding="utf-8").strip() == marker_key ): - return - cmd = _pch_compile_command(build_dir, header, gch) - if cmd is None: + _LOGGER.debug("Pch previously failed for these inputs; skipping") return try: result = subprocess.run( cmd, cwd=build_dir, capture_output=True, text=True, check=False ) - error = result.stderr.strip() if result.returncode != 0 else None + error = None + if result.returncode != 0: + error = result.stderr.strip() or f"exit code {result.returncode}" except OSError as err: error = str(err) if error is not None: @@ -422,8 +446,8 @@ def prepare_pch() -> None: ) gch.unlink(missing_ok=True) sum_path.unlink(missing_ok=True) - # Skip retries until a flag/header/sdkconfig change alters the checksum - failed_marker.write_text(checksum + "\n", encoding="utf-8") + # Skip retries until a header/flag/sdkconfig/command change + failed_marker.write_text(marker_key + "\n", encoding="utf-8") return failed_marker.unlink(missing_ok=True) sum_path.write_text(checksum + "\n", encoding="utf-8") diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 8c5aeffdac..97e04407fc 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -112,7 +112,7 @@ def split_command(command: str) -> list[str]: ctypes.windll.kernel32.LocalFree(argv) -def _expand_response_files(tokens: list[str], directory: Path) -> list[str]: +def expand_response_files(tokens: list[str], directory: Path) -> list[str]: """Inline any ``@response-file`` arguments (paths relative to ``directory``). GCC response files embed flags that must be expanded so GCC-only flags @@ -127,7 +127,7 @@ def _expand_response_files(tokens: list[str], directory: Path) -> list[str]: rf = directory / rf try: out.extend( - _expand_response_files( + expand_response_files( split_command(rf.read_text(encoding="utf-8")), directory ) ) @@ -166,7 +166,7 @@ def parse_entry( ) -> tuple[str, list[str], list[str], list[str]]: """Parse one compile_commands entry -> (cxx_path, defines, includes, cxx_flags).""" directory = Path(entry["directory"]) - tokens = _expand_response_files(split_command(entry["command"]), directory) + tokens = expand_response_files(split_command(entry["command"]), directory) def _include(raw: str) -> str: # Resolve against the entry's ``directory`` so cached idedata works diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 55a493def8..1420c88879 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -1200,6 +1200,8 @@ def _ccache_env() -> dict[str, str]: Only values the user has not already set in the environment are returned, so a custom ``CCACHE_DIR`` / ``CCACHE_MAXSIZE`` / etc. is respected. + The pch settings add ``time_macros`` sloppiness process-wide; the visible + effect is a cached TU can keep an older ``esp_app_desc`` build timestamp. """ if not _ccache_enabled(): # The raw knob value (e.g. "disable") is still inherited by idf.py diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index 4bf394db39..016881c4ba 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -541,7 +541,6 @@ def test_prepare_pch_writes_header_and_sum(tmp_path: Path) -> None: patch("esphome.build_gen.espidf.subprocess.run", side_effect=fake_compile), ): prepare_pch() - assert (dev / "build" / "esphome_pch.h").read_text().startswith("#include") checksum = (dev / "build" / "esphome_pch.h.gch.sum").read_text().strip() assert len(checksum) == 64 # Unchanged inputs: the second call must not recompile @@ -607,20 +606,23 @@ def test_pch_compile_command_variants(tmp_path: Path) -> None: ) assert _pch_compile_command(build, header, gch) is None + src_file = str(tmp_path / "src" / "esphome" / "a.cpp") (build / "compile_commands.json").write_text( json.dumps( [ { "command": ( - "/usr/bin/ccache g++ -DX=1 -include esphome_pch.h " - "-o esp-idf/src/CMakeFiles/__idf_src.dir/a.cpp.obj -c a.cpp" + "/usr/bin/ccache g++ -DX=1 -include esphome_pch.h -MMD " + "-MT a.cpp.obj -MF a.cpp.obj.d " + "-o esp-idf/src/CMakeFiles/__idf_src.dir/a.cpp.obj " + f"-c {src_file}" ), - "file": "/dev/src/esphome/a.cpp", + "file": src_file, }, ] ) ) - # Launcher stripped, -include/-o/-c pairs removed, header targeted + # Launcher stripped; -include/-o/-c and depfile flags removed assert _pch_compile_command(build, header, gch) == [ "g++", "-DX=1", @@ -713,3 +715,21 @@ def test_write_project_pch_disabled_writes_no_header( ): write_project() assert not (tmp_path / "build" / "esphome_pch.h").exists() + + +def test_write_project_writes_pch_header(tmp_path: Path) -> None: + """The header write_project emits is what _pch_cmake() force-includes; + this pairing is the one non-fail-safe path in the design.""" + from esphome.build_gen.espidf import _PCH_HEADERS, write_project + from esphome.build_helpers.pch import pch_header_text + + _write_project_description(tmp_path, {}) + CORE.build_path = tmp_path + with ( + patch("esphome.build_gen.espidf.get_esp32_variant", return_value="ESP32"), + patch.object(CORE, "name", "test"), + ): + write_project() + assert (tmp_path / "build" / "esphome_pch.h").read_text() == pch_header_text( + _PCH_HEADERS + ) diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index 615d3dfe37..f21e28cd13 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -104,7 +104,7 @@ def test_expand_response_files(tmp_path: Path) -> None: rsp = tmp_path / "flags.rsp" rsp.write_text("-DFROM_RSP -I/rsp/inc") - tokens = idedata._expand_response_files( + tokens = idedata.expand_response_files( ["g++", f"@{rsp.name}", "-c", "x.cpp"], tmp_path ) @@ -115,7 +115,7 @@ def test_expand_response_files(tmp_path: Path) -> None: def test_expand_response_files_keeps_literal_when_missing(tmp_path: Path) -> None: """An unreadable ``@file`` token is kept verbatim rather than dropped.""" - tokens = idedata._expand_response_files(["g++", "@nope.rsp"], tmp_path) + tokens = idedata.expand_response_files(["g++", "@nope.rsp"], tmp_path) assert "@nope.rsp" in tokens