From 4510f104917fbd5f9a5ae4847a74afc3989ab44e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 23 Aug 2026 18:13:57 -0500 Subject: [PATCH] [core] Treat a malformed build_info.json as stale instead of crashing (#18669) --- esphome/writer.py | 21 ++- tests/unit_tests/test_writer.py | 243 +++++++++++++++----------------- 2 files changed, 130 insertions(+), 134 deletions(-) diff --git a/esphome/writer.py b/esphome/writer.py index 866377d2f5..d614204603 100644 --- a/esphome/writer.py +++ b/esphome/writer.py @@ -337,12 +337,29 @@ def copy_src_tree(): else: try: existing = json.loads(build_info_json_path.read_text(encoding="utf-8")) - if ( + if not isinstance(existing, dict) or ( existing.get("config_hash") != config_hash or existing.get("esphome_version") != __version__ ): + # Non-object JSON is stale like every other damage case sources_changed = True - except (json.JSONDecodeError, KeyError, OSError): + except FileNotFoundError: + # An absent build_info.json is stale, not damaged; rebuild quietly + sources_changed = True + except (ValueError, OSError) as err: + # ValueError covers both JSONDecodeError and UnicodeDecodeError; + # unlink so the regenerating write never re-reads the bad copy. + # "Unreadable" not "damaged": EACCES/EISDIR land here too + _LOGGER.warning("Regenerating unreadable build_info.json: %s", err) + try: + # missing_ok: a concurrent clean may have removed it already + build_info_json_path.unlink(missing_ok=True) + except OSError as unlink_err: + # The later write re-reads the file, so a kept unreadable copy + # fails again with a misattributed error; name the real cause + _LOGGER.warning( + "Could not remove unreadable build_info.json: %s", unlink_err + ) sources_changed = True # Write build_info header and JSON metadata diff --git a/tests/unit_tests/test_writer.py b/tests/unit_tests/test_writer.py index 46e60ebd8e..c73a5c5789 100644 --- a/tests/unit_tests/test_writer.py +++ b/tests/unit_tests/test_writer.py @@ -2041,6 +2041,38 @@ def test_copy_src_tree_writes_build_info_files( assert build_info_json["esphome_version"] == "2025.1.0-dev" +def _setup_build_info_mocks( + mock_core: MagicMock, + mock_iter_components: MagicMock, + mock_walk_files: MagicMock, + tmp_path: Path, +) -> Path: + """Point CORE at tmp_path and return the build_info.json path.""" + src_path = tmp_path / "src" + (src_path / "esphome" / "core").mkdir(parents=True) + build_path = tmp_path / "build" + build_path.mkdir() + mock_core.relative_src_path.side_effect = src_path.joinpath + mock_core.relative_build_path.side_effect = build_path.joinpath + mock_core.defines = [] + mock_core.config_hash = 0xDEADBEEF + mock_core.comment = "" + mock_core.target_platform = "test_platform" + mock_core.config = {} + mock_iter_components.return_value = [] + mock_walk_files.return_value = [] + return build_path / "build_info.json" + + +def _run_copy_src_tree(version: str = "2025.1.0-dev") -> None: + with ( + patch("esphome.writer.__version__", version), + patch("esphome.writer.importlib.import_module") as mock_import, + ): + mock_import.side_effect = AttributeError + copy_src_tree() + + @patch("esphome.writer.CORE") @patch("esphome.writer.iter_components") @patch("esphome.writer.walk_files") @@ -2050,59 +2082,17 @@ def test_copy_src_tree_detects_config_hash_change( mock_core: MagicMock, tmp_path: Path, ) -> None: - """Test copy_src_tree detects when config_hash changes.""" - # Setup directory structure - src_path = tmp_path / "src" - src_path.mkdir() - esphome_core_path = src_path / "esphome" / "core" - esphome_core_path.mkdir(parents=True) - build_path = tmp_path / "build" - build_path.mkdir() - - # Create existing build_info.json with different config_hash - build_info_json_path = build_path / "build_info.json" - build_info_json_path.write_text( - json.dumps( - { - "config_hash": 0x12345678, # Different from current - "build_time": 1700000000, - "build_time_str": "2023-11-14 22:13:20 +0000", - "esphome_version": "2025.1.0-dev", - } - ) + """A changed config_hash regenerates build_info after a steady-state run.""" + build_info_json_path = _setup_build_info_mocks( + mock_core, mock_iter_components, mock_walk_files, tmp_path ) + _run_copy_src_tree() + assert json.loads(build_info_json_path.read_text())["config_hash"] == 0xDEADBEEF - # Create existing build_info_data.h - build_info_h_path = esphome_core_path / "build_info_data.h" - build_info_h_path.write_text("// old build_info_data.h") - - # Setup mocks - mock_core.relative_src_path.side_effect = src_path.joinpath - mock_core.relative_build_path.side_effect = build_path.joinpath - mock_core.defines = [] - mock_core.config_hash = 0xDEADBEEF # Different from existing - mock_core.comment = "" - mock_core.target_platform = "test_platform" - mock_core.config = {} - mock_iter_components.return_value = [] - mock_walk_files.return_value = [] - - with ( - patch("esphome.writer.__version__", "2025.1.0-dev"), - patch("esphome.writer.importlib.import_module") as mock_import, - ): - mock_import.side_effect = AttributeError - copy_src_tree() - - # Verify build_info files were updated due to config_hash change - assert build_info_h_path.exists() - build_info_cpp_path = esphome_core_path / "build_info_data.cpp" - assert build_info_cpp_path.exists() - new_content = build_info_cpp_path.read_text() - assert "0xdeadbeef" in new_content.lower() - - new_json = json.loads(build_info_json_path.read_text()) - assert new_json["config_hash"] == 0xDEADBEEF + # Second run only regenerates if the hash comparison detects the change + mock_core.config_hash = 0xC0FFEE + _run_copy_src_tree() + assert json.loads(build_info_json_path.read_text())["config_hash"] == 0xC0FFEE @patch("esphome.writer.CORE") @@ -2114,104 +2104,93 @@ def test_copy_src_tree_detects_version_change( mock_core: MagicMock, tmp_path: Path, ) -> None: - """Test copy_src_tree detects when esphome_version changes.""" - # Setup directory structure - src_path = tmp_path / "src" - src_path.mkdir() - esphome_core_path = src_path / "esphome" / "core" - esphome_core_path.mkdir(parents=True) - build_path = tmp_path / "build" - build_path.mkdir() - - # Create existing build_info.json with different version - build_info_json_path = build_path / "build_info.json" - build_info_json_path.write_text( - json.dumps( - { - "config_hash": 0xDEADBEEF, - "build_time": 1700000000, - "build_time_str": "2023-11-14 22:13:20 +0000", - "esphome_version": "2024.12.0", # Old version - } - ) + """A changed esphome_version regenerates build_info after a steady-state run.""" + build_info_json_path = _setup_build_info_mocks( + mock_core, mock_iter_components, mock_walk_files, tmp_path ) - - # Create existing build_info_data.h - build_info_h_path = esphome_core_path / "build_info_data.h" - build_info_h_path.write_text("// old build_info_data.h") - - # Setup mocks - mock_core.relative_src_path.side_effect = src_path.joinpath - mock_core.relative_build_path.side_effect = build_path.joinpath - mock_core.defines = [] - mock_core.config_hash = 0xDEADBEEF - mock_core.comment = "" - mock_core.target_platform = "test_platform" - mock_core.config = {} - mock_iter_components.return_value = [] - mock_walk_files.return_value = [] - - with ( - patch("esphome.writer.__version__", "2025.1.0-dev"), # New version - patch("esphome.writer.importlib.import_module") as mock_import, - ): - mock_import.side_effect = AttributeError - copy_src_tree() - - # Verify build_info files were updated due to version change - assert build_info_h_path.exists() + # Pin version.h so only the build_info comparison can see the bump + with patch("esphome.writer.generate_version_h", return_value="// version.h\n"): + _run_copy_src_tree(version="2024.12.0") + _run_copy_src_tree(version="2025.1.0-dev") new_json = json.loads(build_info_json_path.read_text()) assert new_json["esphome_version"] == "2025.1.0-dev" +@pytest.mark.parametrize( + "damage", + (b"invalid json {{{", b"[]", b'\xff{"config_hash": 1}'), + ids=("invalid-json", "non-object", "non-utf8"), +) +@patch("esphome.writer.CORE") +@patch("esphome.writer.iter_components") +@patch("esphome.writer.walk_files") +def test_copy_src_tree_regenerates_damaged_build_info( + mock_walk_files: MagicMock, + mock_iter_components: MagicMock, + mock_core: MagicMock, + tmp_path: Path, + damage: bytes, +) -> None: + """A damaged build_info.json reads as stale and is regenerated, not left in place.""" + build_info_json_path = _setup_build_info_mocks( + mock_core, mock_iter_components, mock_walk_files, tmp_path + ) + _run_copy_src_tree() + build_info_json_path.write_bytes(damage) + # Second run only rewrites the file if the damage branch fires + _run_copy_src_tree() + new_json = json.loads(build_info_json_path.read_text()) + assert new_json["config_hash"] == 0xDEADBEEF + + @patch("esphome.writer.CORE") @patch("esphome.writer.iter_components") @patch("esphome.writer.walk_files") -def test_copy_src_tree_handles_invalid_build_info_json( +def test_copy_src_tree_missing_build_info_rebuilds_quietly( mock_walk_files: MagicMock, mock_iter_components: MagicMock, mock_core: MagicMock, tmp_path: Path, + caplog: pytest.LogCaptureFixture, ) -> None: - """Test copy_src_tree handles invalid build_info.json gracefully.""" - # Setup directory structure - src_path = tmp_path / "src" - src_path.mkdir() - esphome_core_path = src_path / "esphome" / "core" - esphome_core_path.mkdir(parents=True) - build_path = tmp_path / "build" - build_path.mkdir() + """An absent build_info.json regenerates without claiming damage.""" + build_info_json_path = _setup_build_info_mocks( + mock_core, mock_iter_components, mock_walk_files, tmp_path + ) + _run_copy_src_tree() + build_info_json_path.unlink() + _run_copy_src_tree() + assert json.loads(build_info_json_path.read_text())["config_hash"] == 0xDEADBEEF + assert "unreadable" not in caplog.text - # Create invalid build_info.json - build_info_json_path = build_path / "build_info.json" + +@patch("esphome.writer.CORE") +@patch("esphome.writer.iter_components") +@patch("esphome.writer.walk_files") +def test_copy_src_tree_unremovable_damaged_build_info_is_logged( + mock_walk_files: MagicMock, + mock_iter_components: MagicMock, + mock_core: MagicMock, + tmp_path: Path, + caplog: pytest.LogCaptureFixture, +) -> None: + """A failed unlink of the damaged file names the real cause.""" + build_info_json_path = _setup_build_info_mocks( + mock_core, mock_iter_components, mock_walk_files, tmp_path + ) + _run_copy_src_tree() build_info_json_path.write_text("invalid json {{{") + real_unlink = Path.unlink - # Create existing build_info_data.h - build_info_h_path = esphome_core_path / "build_info_data.h" - build_info_h_path.write_text("// old build_info_data.h") + def fail_on_build_info(self: Path, missing_ok: bool = False) -> None: + if self.name == "build_info.json": + raise OSError("simulated EACCES") + real_unlink(self, missing_ok=missing_ok) - # Setup mocks - mock_core.relative_src_path.side_effect = src_path.joinpath - mock_core.relative_build_path.side_effect = build_path.joinpath - mock_core.defines = [] - mock_core.config_hash = 0xDEADBEEF - mock_core.comment = "" - mock_core.target_platform = "test_platform" - mock_core.config = {} - mock_iter_components.return_value = [] - mock_walk_files.return_value = [] - - with ( - patch("esphome.writer.__version__", "2025.1.0-dev"), - patch("esphome.writer.importlib.import_module") as mock_import, - ): - mock_import.side_effect = AttributeError - copy_src_tree() - - # Verify build_info files were created despite invalid JSON - assert build_info_h_path.exists() - new_json = json.loads(build_info_json_path.read_text()) - assert new_json["config_hash"] == 0xDEADBEEF + with patch.object(Path, "unlink", fail_on_build_info): + _run_copy_src_tree() + assert "Could not remove unreadable build_info.json" in caplog.text + assert json.loads(build_info_json_path.read_text())["config_hash"] == 0xDEADBEEF @patch("esphome.writer.CORE")