diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index be1f7fd25c..88c70aacab 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -258,6 +258,22 @@ def _cc_path_from_cxx(cxx_path: str) -> str: return f"{stem}{suffix}" +def _cache_usable(cached: object) -> bool: + """Check a cached idedata dict against the guarantees of the write path. + + Caches written by older versions predate the launcher rejection and the + include-union shape; serving one would bypass both. The dict check also + keeps "in" from substring-matching a bare JSON string. + """ + if not isinstance(cached, dict) or "cc_path" not in cached: + return False + cxx_path = cached.get("cxx_path") + if not isinstance(cxx_path, str) or _is_launcher(cxx_path): + return False + includes = cached.get("includes") + return isinstance(includes, dict) and isinstance(includes.get("build"), list) + + def load_or_build_idedata( compile_commands: Path, elf_path: Path, @@ -283,10 +299,11 @@ def load_or_build_idedata( # look like unexplained slow builds _LOGGER.warning("Discarding unreadable idedata cache %s: %s", cache, err) else: - # Rebuild pre-cc_path caches on the field, not the timestamp; - # the type check keeps "in" from substring-matching a string - if isinstance(cached, dict) and "cc_path" in cached: + if _cache_usable(cached): + # Re-stamp so a relocated build dir cannot serve a stale ELF path + cached["prog_path"] = str(elf_path) return cached + _LOGGER.debug("Regenerating idedata: cache %s fails validation", cache) data = idedata_from_build(compile_commands, launcher) data["prog_path"] = str(elf_path) @@ -320,6 +337,9 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d project-wide superset (as PlatformIO's idedata provides). """ entries = json.loads(Path(compile_commands).read_text(encoding="utf-8")) + if not isinstance(entries, list): + # A TypeError here would escape IDEDATA_BEST_EFFORT_ERRORS + raise EsphomeError(f"{compile_commands} is not a compile-command list") representative = _pick_entry(entries) cxx_path, defines, rep_includes, cxx_flags = parse_entry(representative, launcher) @@ -339,7 +359,7 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d # may hold different include sets. command = entry["command"] if "@" in command: - return f"unique:{entry['file']}" + return f"unique:{entry.get('output') or command}" return command.replace(entry.get("file", ""), "").replace( entry.get("output", ""), "" ) diff --git a/esphome/espidf/size_summary.py b/esphome/espidf/size_summary.py index 5237495843..5338cd2135 100644 --- a/esphome/espidf/size_summary.py +++ b/esphome/espidf/size_summary.py @@ -37,7 +37,7 @@ _SIZE_SUFFIXES = {"K": 1024, "M": 1024 * 1024} def _parse_size(token: str) -> int: token = token.strip() if not token: - return 0 + raise ValueError("blank partition size cell") if token.startswith(("0x", "0X")): return int(token, 16) suffix = token[-1].upper() @@ -76,6 +76,14 @@ def print_summary(size_json: Path, partitions_csv: Path | None) -> None: summarize. Logs the cause at warning level, so a missing RAM/Flash line (which CI's memory-impact extraction greps for) is diagnosable. """ + try: + _print_summary(size_json, partitions_csv) + except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught + # Backstop for nested shapes the named guards below miss + _LOGGER.warning("Skipping size summary: %s", e) + + +def _print_summary(size_json: Path, partitions_csv: Path | None) -> None: if not size_json.is_file(): _LOGGER.warning("Skipping size summary: %s not found", size_json) return diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index dd2494a093..fec276a92c 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -574,11 +574,101 @@ def test_load_or_build_idedata_never_caches_a_launcher(tmp_path: Path) -> None: assert not cache.exists() +@pytest.mark.parametrize( + "cached", + ( + {"cc_path": "/x/gcc", "cxx_path": "/opt/homebrew/bin/ccache"}, + {"cc_path": "/x/gcc", "cxx_path": "/tools/g++"}, + {"cc_path": "/x/gcc", "cxx_path": "/tools/g++", "includes": {}}, + ), + ids=("launcher-cxx", "no-includes", "no-build-list"), +) +def test_load_or_build_idedata_regenerates_invalid_cache( + tmp_path: Path, cached: dict +) -> None: + """A cache written by an older version fails validation and regenerates.""" + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "c.json" + cache.write_text(json.dumps(cached)) + os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 2) + with patch.object(idedata, "get_toolchain_includes", return_value=[]): + data = idedata.load_or_build_idedata( + compile_commands, tmp_path / "f.elf", cache + ) + assert data["cxx_path"] == "/tools/g++" + assert "includes" in data + + +def test_load_or_build_idedata_cache_hit_restamps_prog_path(tmp_path: Path) -> None: + """A served cache carries the current ELF path, not the one it was written with.""" + compile_commands = _write_compile_commands(tmp_path) + cache = tmp_path / "c.json" + cache.write_text( + json.dumps( + { + "cc_path": "/tools/gcc", + "cxx_path": "/tools/g++", + "includes": {"build": [], "toolchain": []}, + "prog_path": "/old/location/firmware.elf", + } + ) + ) + os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 2) + data = idedata.load_or_build_idedata( + compile_commands, tmp_path / "firmware.elf", cache + ) + assert data["prog_path"] == str(tmp_path / "firmware.elf") + + +def test_idedata_from_build_non_list_compile_db_raises(tmp_path: Path) -> None: + """Valid JSON that is not a list raises by name, inside the best-effort tuple.""" + compile_commands = tmp_path / "compile_commands.json" + for bad in ("{}", "null", '"text"'): + compile_commands.write_text(bad) + with pytest.raises(EsphomeError, match="not a compile-command list"): + idedata.idedata_from_build(compile_commands) + + +def test_idedata_from_build_same_file_rsp_commands_never_dedupe( + tmp_path: Path, +) -> None: + """Two objects built from one source with different .rsp files keep both + include sets; the rsp sentinel keys on the output, not the source.""" + file = f"{ABS}build/src/esphome/core/shared.cpp" + entries = [] + for name in ("a", "b"): + rsp = tmp_path / f"{name}.o.rsp" + rsp.write_text(f"-I{ABS}inc/{name}") + entries.append( + { + "directory": str(tmp_path), + "file": file, + "command": f"/tools/g++ @{rsp.name} -c {file} -o {name}.o", + "output": f"{name}.o", + } + ) + compile_commands = tmp_path / "compile_commands.json" + compile_commands.write_text(json.dumps(entries)) + with patch.object(idedata, "get_toolchain_includes", return_value=[]): + data = idedata.idedata_from_build(compile_commands) + joined = " ".join(data["includes"]["build"]) + assert "inc/a" in joined and "inc/b" in joined + + def test_load_or_build_idedata_cache_hit_skips_rebuild(tmp_path: Path) -> None: """A valid cache newer than the compile DB is served without re-parsing.""" compile_commands = _write_compile_commands(tmp_path) cache = tmp_path / "c.json" - cache.write_text(json.dumps({"cc_path": "/tools/gcc", "cached": True})) + cache.write_text( + json.dumps( + { + "cc_path": "/tools/gcc", + "cxx_path": "/tools/g++", + "includes": {"build": ["/inc"], "toolchain": []}, + "cached": True, + } + ) + ) os.utime(cache, (compile_commands.stat().st_mtime + 5,) * 2) with patch.object(idedata, "idedata_from_build") as mock_build: data = idedata.load_or_build_idedata( diff --git a/tests/unit_tests/test_size_summary.py b/tests/unit_tests/test_size_summary.py index c5d117b953..8647eec67f 100644 --- a/tests/unit_tests/test_size_summary.py +++ b/tests/unit_tests/test_size_summary.py @@ -205,8 +205,19 @@ def test_print_summary_non_dict_json_warns(tmp_path, caplog) -> None: def test_print_summary_zero_app_partition_warns(tmp_path, caplog) -> None: - """A malformed partition row parsing to 0 must not render a 0% bar for - CI's memory-impact extraction to ingest.""" + """A partition row with size 0 drops the Flash bar instead of rendering 0%.""" + size_json = tmp_path / "size.json" + size_json.write_text( + '{"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": 100}' + ) + partitions = tmp_path / "partitions.csv" + partitions.write_text("app0, app, ota_0, 0x10000, 0,\n") + print_summary(size_json, partitions) + assert "app partition size is" in caplog.text + + +def test_print_summary_blank_partition_size_warns(tmp_path, caplog) -> None: + """A blank size cell raises ValueError by name instead of parsing to 0.""" size_json = tmp_path / "size.json" size_json.write_text( '{"memory_types": {"DRAM": {"used": 1, "size": 2}}, "image_size": 100}' @@ -214,4 +225,21 @@ def test_print_summary_zero_app_partition_warns(tmp_path, caplog) -> None: partitions = tmp_path / "partitions.csv" partitions.write_text("app0, app, ota_0, 0x10000, ,\n") print_summary(size_json, partitions) - assert "app partition size is" in caplog.text + assert "blank partition size cell" in caplog.text + + +@pytest.mark.parametrize( + "payload", + ( + '{"memory_types": []}', + '{"memory_types": {"DRAM": 5}}', + '{"memory_types": {"DRAM": {"used": "x", "size": "y"}}}', + ), + ids=("non-dict-memory-types", "scalar-region", "non-numeric-sizes"), +) +def test_print_summary_nested_shapes_never_raise(tmp_path, caplog, payload) -> None: + """The blanket guard keeps unexpected nested shapes from raising.""" + size_json = tmp_path / "size.json" + size_json.write_text(payload) + print_summary(size_json, tmp_path / "partitions.csv") + assert "Skipping size summary" in caplog.text