Merge branch 'esp8266-native-build-infra' into esp8266-native-framework-installer

esphome/build_helpers/ccache.py

@@ -13,40 +13,18 @@ from __future__ import annotations
 import logging
 import os
 from pathlib import Path
-import subprocess
 
-from esphome.framework_helpers import strip_win_long_path_prefix
+from esphome.framework_helpers import strip_win_long_path_prefix, tool_version_runs
 
 _LOGGER = logging.getLogger(__name__)
 
 
 def _ccache_runs(ccache: str) -> bool:
-    """Return True when the ``ccache`` found on PATH actually runs.
-
-    ``shutil.which`` proves existence, not runnability: on Windows it also
-    matches ``.bat``/``.cmd`` wrappers and stale package-manager shims whose
-    target is gone. Wrapping compiles around such a find fails every compile
-    step with an opaque OS error, so probe once and fall back to compiling
-    without ccache when the probe fails.
-    """
-    try:
-        subprocess.run(
-            [ccache, "--version"],
-            check=True,
-            stdout=subprocess.DEVNULL,
-            stderr=subprocess.DEVNULL,
-            timeout=15,
-            # Repo-wide convention (posix_spawn fast path); see the
-            # close_fds=False call sites across esphome/ and script/helpers.py
-            close_fds=False,
-        )
-    except (OSError, subprocess.SubprocessError):
-        _LOGGER.warning(
-            "Ignoring ccache at %s because it failed to run; compiling without ccache",
-            ccache,
-        )
-        return False
-    return True
+    """Return True when the ``ccache`` found on PATH actually runs."""
+    return tool_version_runs(
+        ccache,
+        "Ignoring ccache at %s because it failed to run; compiling without ccache",
+    )
 
 
 def resolve_ccache_path() -> str | None:

esphome/build_helpers/ninja.py

@@ -7,38 +7,20 @@ 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
+from esphome.framework_helpers import strip_win_long_path_prefix, tool_version_runs
 
 _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
+    """Whether the ninja found on PATH actually runs (see tool_version_runs)."""
+    return tool_version_runs(
+        binary,
+        "Ignoring ninja at %s because it failed to run; "
+        "falling back to the bundled wheel",
+    )
 
 
 def find_ninja() -> Path:
@@ -80,8 +62,9 @@ def quote_arg(tok: str) -> str:
 
     Same escaping rule as ``subprocess.list2cmdline``: a backslash run
     doubles only immediately before a quote (or the closing quote), and the
-    quote itself is escaped. POSIX sh parses the result identically for
-    backslashes and quotes. ``$`` must already be doubled for ninja.
+    quote itself is escaped. CreateProcess-only; POSIX sh collapses
+    backslash runs inside double quotes, so shell_token single-quotes
+    there instead. ``$`` must already be doubled for ninja.
     """
     quoted = re.sub(r'(\\*)"', lambda m: m.group(1) * 2 + '\\"', tok)
     quoted = re.sub(r"(\\+)\Z", lambda m: m.group(1) * 2, quoted)
@@ -99,17 +82,22 @@ def shell_token(tok: str, force: bool = False) -> str:
 
     Lexing strips the quoting a user wrote (``-DX="a b"`` becomes the single
     token ``-DX=a b``); re-quote on the way out so the compiler receives the
-    same argv element SCons would pass under PlatformIO. After ninja
-    un-doubles ``$$``, sh still applies every expansion double quotes allow
-    (``$VAR``, ``$(...)``, backticks) while CreateProcess passes them
-    literally; SCons on POSIX spawns without a shell, so this is a known,
-    deliberate divergence for tokens carrying those characters.
+    same argv element SCons would pass under PlatformIO. Ninja hands POSIX
+    commands to ``/bin/sh -c`` and Windows commands to CreateProcess, so the
+    quoting style is chosen per platform: single quotes on POSIX (sh expands
+    nothing inside them, matching SCons's no-shell spawn) and the argv rule
+    on Windows. ``$`` is doubled first in either case because ninja expands
+    ``$`` before the command reaches the shell.
     """
     tok = tok.replace("$", "$$")  # ninja would expand a bare $ to nothing
-    if force or not tok or _NEEDS_QUOTE.search(tok):
-        # An empty token must become "" or it vanishes from the argv
+    if not (force or not tok or _NEEDS_QUOTE.search(tok)):
+        return tok
+    # An empty token must become '' / "" or it vanishes from the argv
+    if os.name == "nt":
         return quote_arg(tok)
-    return tok
+    # shlex.quote's rule; inlined because the $-doubled token must not be
+    # re-examined for safe characters
+    return "'" + tok.replace("'", "'\"'\"'") + "'"
 
 
 def quote_path(value: Path | str) -> str:

esphome/espidf/framework.py

@@ -11,7 +11,7 @@ import re
 import shutil
 from typing import Any, NoReturn
 
-from esphome.build_helpers.ccache import ccache_defaults_env
+from esphome.build_helpers.ccache import ccache_defaults_env, resolve_ccache_path
 from esphome.build_helpers.tools_cache import tools_cache_path
 from esphome.core import Version
 from esphome.framework_helpers import (
@@ -1150,10 +1150,14 @@ def _ccache_env() -> dict[str, str]:
     a custom ``CCACHE_DIR`` / ``CCACHE_MAXSIZE`` / etc. is respected.
     """
     # Honor an explicit choice already in the environment (opt-out or opt-in).
+    # IDF_CCACHE_ENABLE (this backend's native knob) wins over the shared
+    # ESPHOME_CCACHE_ENABLE, which resolve_ccache_path parses; without it a
+    # user disabling ccache to debug a miscompile would silently keep it
+    # enabled here.
     if "IDF_CCACHE_ENABLE" in os.environ:
         if not get_bool_env("IDF_CCACHE_ENABLE"):
             return {}
-    elif shutil.which("ccache") is None:
+    elif resolve_ccache_path() is None:
         # ESP-IDF silently skips ccache without the binary; don't enable it.
         return {}
 

esphome/framework_helpers.py

@@ -196,6 +196,30 @@ def run_command(
         return False, None, None
 
 
+def tool_version_runs(binary: str, warning: str) -> bool:
+    """Probe ``binary --version``; on failure warn with ``warning`` % binary.
+
+    ``shutil.which`` proves existence, not runnability: on Windows it also
+    matches ``.bat``/``.cmd`` wrappers and stale package-manager shims whose
+    target is gone. Callers probe once and fall back instead of failing
+    every build step with an opaque OS error.
+    """
+    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(warning, binary)
+        return False
+    return True
+
+
 def run_command_ok(*args, **kwargs) -> bool:
     """
     Execute a command and return only the success status.

esphome/platformio/registry.py

@@ -70,6 +70,10 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None]
         raise EsphomeError(
             f"The package registry returned invalid JSON for {package}: {err}"
         ) from err
+    if not isinstance(data, dict):
+        raise EsphomeError(
+            f"Unexpected package registry response for {package}: {str(data)[:200]}"
+        )
     systype = get_systype()
     versions = data.get("versions")
     if not isinstance(versions, list):
@@ -127,6 +131,21 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None]
     raise EsphomeError(f"{package} {version} not found in the package registry")
 
 
+def _check_layout(name: str, dest: Path, expect: Collection[str]) -> None:
+    """Raise when an install tree is missing an expected directory.
+
+    Runs on fresh extracts and on marker hits: a marked tree that later
+    lost files (manual deletion, antivirus quarantine) must fail by name
+    instead of surfacing as an opaque toolchain error.
+    """
+    for rel in expect:
+        if not (dest / rel).is_dir():
+            raise EsphomeError(
+                f"{name} at {dest} is missing the expected {rel} "
+                "directory; run 'esphome clean-all' and retry"
+            )
+
+
 def install_package(
     name: str,
     version: str,
@@ -148,6 +167,7 @@ def install_package(
         raise ValueError("install_package requires a non-empty expect")
     marker = dest / ".esphome_extracted"
     if marker.is_file():
+        _check_layout(name, dest, expect)
         return
     from filelock import FileLock
 
@@ -185,11 +205,6 @@ def install_package(
         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"
-                )
+        _check_layout(name, dest, expect)
         marker.touch()
         archive.unlink(missing_ok=True)