diff --git a/esphome/arduino8266/component.py b/esphome/arduino8266/component.py index 6365a26dda..0898d4722b 100644 --- a/esphome/arduino8266/component.py +++ b/esphome/arduino8266/component.py @@ -17,7 +17,7 @@ from dataclasses import dataclass, field import logging from pathlib import Path -from esphome.core import CORE, Library +from esphome.core import CORE, EsphomeError, Library from esphome.espidf.extra_script import apply_extra_script from esphome.platformio.library import ( DEFAULT_BUILD_INCLUDE_DIR, @@ -68,9 +68,11 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: (d for d in ("src", "Src") if (read_path / d).is_dir()), "." ) if "srcDir" in build and not (read_path / src_dir).is_dir(): - # A silently empty source set would surface as link errors instead - _LOGGER.warning( - "Library %s declares srcDir %s which does not exist", name, src_dir + # Unlike the default probes, an explicitly declared srcDir that does + # not resolve is unambiguously a manifest/tree error; a silently + # empty source set would surface as link errors far from the cause + raise EsphomeError( + f"Library {name} declares srcDir {src_dir} which does not exist" ) src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index 843e3a2022..b07b135f2d 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -17,6 +17,7 @@ toolchain has always used, so the bits are identical); the from __future__ import annotations +from collections.abc import Collection import functools import logging import os @@ -178,6 +179,7 @@ def _install_package( version: str, dest: Path, mirrors: list[str], + expect: Collection[str] = (), ) -> None: """Download, verify, and extract one package if not already installed. @@ -207,6 +209,14 @@ def _install_package( download_with_resume(url, archive, sha256=sha256, size=size) _LOGGER.info("Extracting %s ...", name) archive_extract_all(archive, dest, progress_header="Extracting") + # Validate the layout before recording success, so an unexpected package + # is never cached as a working install. + for rel in expect: + if not (dest / rel).is_dir(): + raise EsphomeError( + f"{name} {version} extracted without the expected {rel} " + "directory; run 'esphome clean-all' and retry" + ) marker.touch() archive.unlink(missing_ok=True) @@ -245,6 +255,7 @@ def check_and_install(framework_version: cv.Version) -> dict[str, Path]: package_version, framework_path, ESPHOME_ARDUINO8266_FRAMEWORK_MIRRORS, + expect=("cores/esp8266", "tools/sdk"), ) toolchain_path = get_toolchain_path() _install_package( @@ -252,6 +263,7 @@ def check_and_install(framework_version: cv.Version) -> dict[str, Path]: TOOLCHAIN_VERSION, toolchain_path, ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS, + expect=("bin",), ) return { "framework_path": framework_path, diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index e5af8cc1c4..b45b121f91 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -481,7 +481,8 @@ def write_project(paths: dict[str, Path]) -> bool: for required in include_dirs: if not required.is_dir(): raise EsphomeError( - f"Arduino toolchain install is incomplete: missing {required}" + f"Arduino toolchain install is incomplete: missing {required}; " + "run 'esphome clean-all' and retry" ) for lib in libraries: include_dirs += lib.include_dirs diff --git a/esphome/components/esp8266/build_surgery.py b/esphome/components/esp8266/build_surgery.py index d8ca64f669..039ecaf0c6 100644 --- a/esphome/components/esp8266/build_surgery.py +++ b/esphome/components/esp8266/build_surgery.py @@ -60,7 +60,7 @@ def _patch_segment_size(content: str, segment_name: str, new_size: str) -> str: return re.sub(pattern, rf"\g<1>{new_size}", content) -def apply_testing_memory_patches(content: str, require: Collection[str] = ()) -> str: +def apply_testing_memory_patches(content: str, require: Collection[str]) -> str: """Enlarge IRAM/DRAM/flash segments so grouped CI test builds can link. ``require`` names the segments this file must define; a silently diff --git a/tests/unit_tests/components/esp8266/test_build_surgery.py b/tests/unit_tests/components/esp8266/test_build_surgery.py index 9e4640418d..200954494b 100644 --- a/tests/unit_tests/components/esp8266/test_build_surgery.py +++ b/tests/unit_tests/components/esp8266/test_build_surgery.py @@ -49,7 +49,7 @@ def test_relocate_ratetable_requires_anchor() -> None: def test_testing_memory_patches_enlarge_segments() -> None: - patched = apply_testing_memory_patches(_FLASH_LD_SNIPPET) + patched = apply_testing_memory_patches(_FLASH_LD_SNIPPET, require=()) assert ( "iram1_0_seg : org = 0x40100000, len = 0x200000" in patched @@ -83,4 +83,5 @@ def test_testing_memory_patches_require() -> None: "MEMORY { }", require=("dram0_0_seg", "irom0_0_seg") ) # Segments a file does not require are patched opportunistically only - assert apply_testing_memory_patches("MEMORY { }") == "MEMORY { }" + # With nothing required, unmatched content passes through unchanged + assert apply_testing_memory_patches("MEMORY { }", require=()) == "MEMORY { }" diff --git a/tests/unit_tests/test_arduino8266_component.py b/tests/unit_tests/test_arduino8266_component.py index 0a271215dc..efe38a68dc 100644 --- a/tests/unit_tests/test_arduino8266_component.py +++ b/tests/unit_tests/test_arduino8266_component.py @@ -9,7 +9,7 @@ import pytest from esphome.arduino8266 import component from esphome.const import KEY_CORE, KEY_TARGET_PLATFORM, PLATFORM_ESP8266 -from esphome.core import CORE, Library +from esphome.core import CORE, EsphomeError, Library from esphome.platformio.library import ConvertedLibrary, LibraryBackend @@ -257,16 +257,20 @@ def test_library_info_missing_explicit_include_warns( assert "include dir nope which does not exist" in caplog.text -def test_library_info_missing_declared_dirs_warn( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: - """Explicitly declared srcDir/includeDir that do not exist warn by name.""" +def test_library_info_missing_declared_src_dir_raises(tmp_path: Path) -> None: + """An explicitly declared srcDir that does not exist is a manifest error.""" read_path = tmp_path / "lib" read_path.mkdir() - component._library_info( - "x", read_path, {"build": {"srcDir": "nosrc", "includeDir": "noinc"}} - ) - assert "srcDir nosrc which does not exist" in caplog.text + with pytest.raises(EsphomeError, match="srcDir nosrc which does not exist"): + component._library_info("x", read_path, {"build": {"srcDir": "nosrc"}}) + + +def test_library_info_missing_declared_include_dir_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + read_path = tmp_path / "lib" + read_path.mkdir() + component._library_info("x", read_path, {"build": {"includeDir": "noinc"}}) assert "include dir noinc which does not exist" in caplog.text diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index d17250a49b..aba3c514d6 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -376,3 +376,29 @@ def test_ccache_env(tmp_path: Path) -> None: assert env["CCACHE_DEPEND"] == "1" assert env["CCACHE_BASEDIR"] == str(Path(CORE.build_path).resolve()) assert env["CCACHE_DIR"].endswith("ccache") + + +def test_install_package_validates_expected_layout(tmp_path: Path) -> None: + """The success marker is only written when the extracted tree is usable.""" + dest = tmp_path / "pkg" + with ( + patch.object(framework, "download_from_mirrors"), + patch.object(framework, "archive_extract_all") as mock_extract, + patch.object(framework, "_pio_system", return_value="linux_x86_64"), + ): + mock_extract.side_effect = lambda *_a, **_kw: (dest / "bin").mkdir(parents=True) + framework._install_package("pkg", "1.0.0", dest, ["http://m"], expect=("bin",)) + assert (dest / ".esphome_extracted").is_file() + + +def test_install_package_unexpected_layout_raises(tmp_path: Path) -> None: + dest = tmp_path / "pkg" + with ( + patch.object(framework, "download_from_mirrors"), + patch.object(framework, "archive_extract_all") as mock_extract, + patch.object(framework, "_pio_system", return_value="linux_x86_64"), + pytest.raises(EsphomeError, match="without the expected bin"), + ): + mock_extract.side_effect = lambda *_a, **_kw: dest.mkdir() + framework._install_package("pkg", "1.0.0", dest, ["http://m"], expect=("bin",)) + assert not (dest / ".esphome_extracted").exists()