Probe the PATH ninja, chain import errors, harden registry payload checks, honest ccache docstring

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

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)

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})"
         )

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

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")