From 18a2aa20970bc1ccb6b142b04480a0156bdd3d6a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 23:53:40 -0500 Subject: [PATCH 1/3] Probe the PATH ninja, chain import errors, harden registry payload checks, honest ccache docstring --- esphome/build_helpers/ccache.py | 10 ++-- esphome/build_helpers/ninja.py | 48 +++++++++++++++++--- esphome/platformio/registry.py | 15 +++++- tests/unit_tests/build_helpers/test_ninja.py | 33 +++++++++++++- tests/unit_tests/test_platformio_registry.py | 18 ++++++++ 5 files changed, 111 insertions(+), 13 deletions(-) diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 87dd337396..70005f5d40 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -1,9 +1,11 @@ """Shared ccache policy for build backends. -One place for the ``CCACHE_*`` defaults (every backend) and for the probe -and enable rules (backends that call ``resolve_ccache_path``: PlatformIO -and the native Arduino build). The ESP-IDF backend keeps its own -``IDF_CCACHE_ENABLE`` gate and does not probe. +``ccache_defaults_env`` serves the backends that export ``CCACHE_*`` into a +build subprocess (native ESP-IDF and Arduino); ``resolve_ccache_path`` +carries the probe and enable rules (PlatformIO and the native Arduino +build). The ESP-IDF backend keeps its own ``IDF_CCACHE_ENABLE`` gate and +does not probe; PlatformIO feeds its SCons wrapper script through env +channels instead of ``CCACHE_*`` defaults. """ from __future__ import annotations diff --git a/esphome/build_helpers/ninja.py b/esphome/build_helpers/ninja.py index 3b16a9d57d..171462feb6 100644 --- a/esphome/build_helpers/ninja.py +++ b/esphome/build_helpers/ninja.py @@ -2,25 +2,61 @@ from __future__ import annotations +import logging import os from pathlib import Path import re import shutil +import subprocess from esphome.core import EsphomeError +from esphome.framework_helpers import strip_win_long_path_prefix + +_LOGGER = logging.getLogger(__name__) + + +def _ninja_runs(binary: str) -> bool: + """Whether the ninja found on PATH actually runs. + + Same rationale as the ccache probe: ``shutil.which`` proves existence, + not runnability (stale shims, broken wrappers). + """ + try: + subprocess.run( + [binary, "--version"], + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + timeout=15, + # Repo-wide convention (posix_spawn fast path) + close_fds=False, + ) + except (OSError, subprocess.SubprocessError): + _LOGGER.warning( + "Ignoring ninja at %s because it failed to run; " + "falling back to the bundled wheel", + binary, + ) + return False + return True def find_ninja() -> Path: - """Locate the ninja binary: PATH first, else the ninja PyPI wheel. + """Locate the ninja binary: a runnable PATH hit first, else the ninja + PyPI wheel. The wheel is a requirements.txt dependency, so pip has already integrity-checked it; no download logic is needed here. """ if binary := shutil.which("ninja"): - return Path(binary) + binary = strip_win_long_path_prefix(binary) + if _ninja_runs(binary): + return Path(binary) + import_error: ImportError | None = None try: import ninja - except ImportError: + except ImportError as err: + import_error = err wheel_binary = None else: wheel_binary = Path(ninja.BIN_DIR) / ( @@ -30,11 +66,11 @@ def find_ninja() -> Path: raise EsphomeError( "ninja not found on PATH or in the ninja package; reinstall the " "esphome Python environment" - ) + ) from import_error return wheel_binary -def escape(value) -> str: +def escape(value: Path | str) -> str: """Escape a path or token for a ninja file.""" return str(value).replace("$", "$$").replace(":", "$:").replace(" ", "$ ") @@ -76,6 +112,6 @@ def shell_token(tok: str, force: bool = False) -> str: return tok -def quote_path(value) -> str: +def quote_path(value: Path | str) -> str: """Force-quote a path for the ninja command line (shell/CreateProcess).""" return shell_token(str(value), force=True) diff --git a/esphome/platformio/registry.py b/esphome/platformio/registry.py index e6db487c83..89cf76de10 100644 --- a/esphome/platformio/registry.py +++ b/esphome/platformio/registry.py @@ -81,7 +81,12 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None] for ver in versions: if ver.get("name") != version: continue - for file in ver.get("files", []): + files = ver.get("files") + if not isinstance(files, list): + raise EsphomeError( + f"Unexpected package registry response for {package}: {str(ver)[:200]}" + ) + for file in files: # Only a MISSING key means "any system"; an explicitly empty # list must not match (a wrong-architecture download would be # cached as a good install). A bare string would make ``in`` a @@ -100,7 +105,13 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None] f"The package registry returned no sha256 for " f"{package} {version}; refusing the unverified download" ) - return (file["download_url"], sha256, file.get("size")) + url = file.get("download_url") + if not url: + raise EsphomeError( + f"The package registry returned no download URL for " + f"{package} {version}" + ) + return (url, sha256, file.get("size")) raise EsphomeError( f"No {package} {version} build for this platform ({systype})" ) diff --git a/tests/unit_tests/build_helpers/test_ninja.py b/tests/unit_tests/build_helpers/test_ninja.py index ea4998e107..981c740cde 100644 --- a/tests/unit_tests/build_helpers/test_ninja.py +++ b/tests/unit_tests/build_helpers/test_ninja.py @@ -14,7 +14,10 @@ from esphome.core import EsphomeError def test_find_ninja_prefers_path(tmp_path: Path) -> None: - with patch("shutil.which", return_value=str(tmp_path / "ninja")): + with ( + patch("shutil.which", return_value=str(tmp_path / "ninja")), + patch.object(ninja_helper, "_ninja_runs", return_value=True), + ): assert ninja_helper.find_ninja() == tmp_path / "ninja" @@ -81,3 +84,31 @@ def test_quote_path_force_quotes() -> None: def test_shell_token_empty_token_is_quoted() -> None: """An empty argv element must survive as an explicit pair of quotes.""" assert ninja_helper.shell_token("") == '""' + + +def test_find_ninja_probes_path_hit(tmp_path: Path) -> None: + """A broken PATH shim falls back to the wheel instead of failing every + build later.""" + binary_name = "ninja.exe" if os.name == "nt" else "ninja" + (tmp_path / binary_name).touch() + wheel = MagicMock(BIN_DIR=str(tmp_path)) + with ( + patch("shutil.which", return_value="/broken/ninja"), + patch.object(ninja_helper, "_ninja_runs", return_value=False), + patch.dict(sys.modules, {"ninja": wheel}), + ): + assert ninja_helper.find_ninja() == tmp_path / binary_name + + +def test_ninja_probe_failure_warns(caplog: pytest.LogCaptureFixture) -> None: + with patch( + "esphome.build_helpers.ninja.subprocess.run", side_effect=OSError("boom") + ): + assert ninja_helper._ninja_runs("/broken/ninja") is False + assert "failed to run" in caplog.text + + +def test_ninja_probe_success() -> None: + with patch("esphome.build_helpers.ninja.subprocess.run") as mock_run: + assert ninja_helper._ninja_runs("/usr/bin/ninja") is True + assert mock_run.call_args.kwargs["close_fds"] is False diff --git a/tests/unit_tests/test_platformio_registry.py b/tests/unit_tests/test_platformio_registry.py index 6c4ad83c4c..a8f895ad43 100644 --- a/tests/unit_tests/test_platformio_registry.py +++ b/tests/unit_tests/test_platformio_registry.py @@ -346,3 +346,21 @@ def test_registry_download_missing_system_key_matches_any() -> None: [{"download_url": "http://x/any", "checksum": {"sha256": "abc"}, "size": 1}] ): assert registry.registry_download("pkg", "1.0.0") == ("http://x/any", "abc", 1) + + +def test_registry_download_missing_files_list_is_named() -> None: + """A version entry without a files list is an unexpected payload, not a + missing platform build.""" + with ( + _registry_response(None), + pytest.raises(EsphomeError, match="Unexpected package registry response"), + ): + registry.registry_download("pkg", "1.0.0") + + +def test_registry_download_missing_download_url_is_named() -> None: + with ( + _registry_response([{"system": "*", "checksum": {"sha256": "abc"}, "size": 1}]), + pytest.raises(EsphomeError, match="no download URL"), + ): + registry.registry_download("pkg", "1.0.0") From 078bcba4735898a1bc97e5d63ded703dba45dd14 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 23:55:15 -0500 Subject: [PATCH 2/3] Backend-neutral unsupported-core message, pin the PIO source formatter --- esphome/arduino8266/framework.py | 7 +++++-- tests/unit_tests/test_arduino8266_framework.py | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index e7b55e1b7c..cded1438c4 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -68,9 +68,12 @@ def framework_package_version(ver: Version) -> str: package that cannot exist. """ if ver.major > 3: + # Backend-neutral: this also fires on the PlatformIO validation path + # (via _format_framework_arduino_version), where switching toolchains + # would not help raise EsphomeError( - f"Arduino core {ver} has no known package encoding; " - "use 'toolchain: platformio'" + f"Arduino core {ver} is not supported yet; " + "the newest known core series is 3.x" ) return f"3.{ver.major}{ver.minor:02d}{ver.patch:02d}.0" diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index 334f081bcb..c69f802c9e 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -25,10 +25,23 @@ def test_framework_package_version() -> None: # 2.6.3+ cores use the same package-major-3 encoding (PlatformIO path) assert framework.framework_package_version(cv.Version(2, 7, 4)) == "3.20704.0" # A future major bump needs its own encoding, not a doomed registry lookup - with pytest.raises(EsphomeError, match="no known package encoding"): + with pytest.raises(EsphomeError, match="not supported yet"): framework.framework_package_version(cv.Version(4, 0, 0)) +def test_format_framework_arduino_version_pins_all_series() -> None: + """The esp8266 component's PIO source formatter across every encoding + era, including the 4.x rejection it now shares with the installer.""" + from esphome.components.esp8266 import _format_framework_arduino_version as fmt + + assert fmt(cv.Version(2, 4, 1)) == "~1.20401.0" + assert fmt(cv.Version(2, 6, 2)) == "~2.20602.0" + assert fmt(cv.Version(2, 7, 4)) == "~3.20704.0" + assert fmt(cv.Version(3, 1, 2)) == "~3.30102.0" + with pytest.raises(EsphomeError, match="not supported yet"): + fmt(cv.Version(4, 0, 0)) + + def test_tools_path_default_and_prefix(tmp_path: Path) -> None: with patch.dict(os.environ, {"ESPHOME_ARDUINO8266_PREFIX": str(tmp_path)}): assert framework.get_arduino8266_tools_path() == tmp_path.resolve() From 662978c402517a4260681586968affedea198404 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 23:55:38 -0500 Subject: [PATCH 3/3] Name dropped requests and non-platform dependency rejections, correct the dot_a_linkage attribution --- esphome/arduino/library.py | 43 +++++++++++++++------ tests/unit_tests/test_arduino_library.py | 49 ++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 12 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index 1c5840cd10..dee58b5fed 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -8,10 +8,11 @@ resolution/download pipeline in ``esphome.platformio.library``. Nothing here is core-specific: the caller names the PlatformIO platform, MCU, and cache key of the Arduino core it builds. -Known deviation: flat-layout (``library.properties``, no ``src/``) libraries -get the recursive default source filter rather than PlatformIO's root-only -Arduino-1.0 filter; no bundled library is affected, only user-supplied ones -carrying sources in unusual subdirectories. +Known deviations: flat-layout (``library.properties``, no ``src/``) +libraries get the recursive default source filter rather than PlatformIO's +root-only Arduino-1.0 filter (no bundled library is affected), and the +Arduino ``dot_a_linkage`` property is honored even though PlatformIO +ignores it. Mirrors PlatformIO's ``lib_ldf_mode=off`` behavior: each library builds into its own static archive and every library's include dir joins one global @@ -87,8 +88,9 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: # PlatformIO shell-lexes each build.flags entry flag_tokens = lex_build_flags(build.get("flags", []), f"library {name}") - # PIO precedence: build.libArchive, else the Arduino-format - # dot_a_linkage property, else archive (PlatformIO's default) + # build.libArchive is PIO behavior; dot_a_linkage is honored as a + # deliberate extra (Arduino IDE's property, which PIO ignores) so + # properties-only libraries can opt out of archiving too if "libArchive" in build: lib_archive = bool(build["libArchive"]) elif "dot_a_linkage" in data: @@ -245,11 +247,19 @@ def resolve_libraries( try: check_library_data(dep, pio_platform, "arduino") except InvalidLibrary as err: - # check_library_data's only raise is the platform filter, and - # rejecting another platform's dependency of a cross-platform - # manifest is routine (every ESPAsyncWebServer build hits it); - # a warning here would be noise, and the reason is in the log. - _LOGGER.debug("Skipping bundled dependency %s: %s", name, err) + # Rejecting another platform's dependency of a cross-platform + # manifest is routine (every ESPAsyncWebServer build hits + # it), so the platform filter stays at debug; any other + # cause means a dropped dependency and must be visible + if "platform" in str(err).lower(): + _LOGGER.debug("Skipping bundled dependency %s: %s", name, err) + else: + _LOGGER.warning( + "Skipping bundled dependency %s of %s: %s", + name, + component.name, + err, + ) continue bundled_names.add(name) bundled.append(_bundled_library(framework_path, name)) @@ -266,7 +276,7 @@ def resolve_libraries( _add_bundled_dependencies(component) if external: - convert_libraries( + resolved = convert_libraries( external, LibraryBackend( platform=pio_platform, @@ -275,5 +285,14 @@ def resolve_libraries( cache_key=cache_key, ), ) + if len(resolved) < len(external): + # A requested library the converter dropped would otherwise + # surface only as link errors far from the cause + _LOGGER.warning( + "%d of %d requested libraries were not resolved (resolved: %s)", + len(external) - len(resolved), + len(external), + ", ".join(sorted(c.name for c in resolved)) or "none", + ) return bundled + converted diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 81e17490aa..f0bff7b729 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -409,3 +409,52 @@ def test_resolve_libraries_dep_warnings( assert "malformed dependency entry" in caplog.text assert "Orphan" in caplog.text assert "owner but no version" in caplog.text + + +def test_resolve_libraries_warns_when_converter_drops_a_request( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A requested external library the converter drops is named, not lost.""" + framework = _make_framework(tmp_path) + _add_library("pngle", "1.0.0") + with patch.object(component, "convert_libraries", return_value=[]): + component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) + assert "1 of 1 requested libraries were not resolved" in caplog.text + + +def test_bundled_dependency_nonplatform_rejection_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """An InvalidLibrary whose cause is not the platform filter is visible.""" + from esphome.platformio.library import InvalidLibrary + + framework = _make_framework(tmp_path) + _add_library("ESP32Async/ESPAsyncWebServer", "3.9.6") + lib_dir = tmp_path / "converted" / "webserver" + (lib_dir / "src").mkdir(parents=True) + converted = _converted( + "esp32async__ESPAsyncWebServer", + lib_dir, + {"build": {}, "dependencies": [{"name": "Wire"}]}, + ) + with ( + _emitting_converter(converted), + patch.object( + component, + "check_library_data", + side_effect=InvalidLibrary("manifest is corrupt"), + ), + ): + component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) + assert "Skipping bundled dependency Wire" in caplog.text + assert "manifest is corrupt" in caplog.text