diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 81bafe874e..8804e5028d 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -1,13 +1,5 @@ -"""Shared ccache policy for build backends. - -``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 ``IDF_CCACHE_ENABLE`` as a -higher-precedence override and falls back to the shared resolver (probe -included) when it is unset; PlatformIO feeds its SCons wrapper script -through env channels instead of ``CCACHE_*`` defaults. -""" +"""Shared ccache policy for build backends: env-knob parsing, binary +resolution, and default ``CCACHE_*`` values.""" from __future__ import annotations @@ -50,12 +42,8 @@ def parse_enable_env(name: str) -> bool | None: def resolve_ccache_path() -> str | None: """The ccache binary to wrap compiles with, or None when disabled. - Shared policy for every backend: on by default when a runnable ccache is - on PATH, ``ESPHOME_CCACHE_ENABLE=0`` opts out, and an explicit ``=1`` - warns when no binary is found and skips the runnability probe; - any other value warns and is treated as unset. The - Windows extended-length prefix is stripped before probing so the probe - validates the exact string the build will execute (#18399). + An explicit ``ESPHOME_CCACHE_ENABLE=1`` skips the runnability probe; the + Windows extended-length prefix is stripped before probing (#18399). """ import shutil @@ -85,9 +73,8 @@ def ccache_defaults_env(cache_dir: Path) -> dict[str, str]: """ from esphome.core import CORE - # build_path is set during preload for every config-loading command; unset - # means the caller built the environment too early. Fail loudly rather - # than silently drop CCACHE_BASEDIR (losing cross-device cache hits). + # An unset build_path means the env was built before preload; fail loudly + # rather than silently drop CCACHE_BASEDIR. if CORE.build_path is None: raise ValueError( "CORE.build_path must be set before constructing the build environment" diff --git a/esphome/build_helpers/ninja.py b/esphome/build_helpers/ninja.py index 9850dfd828..8c25bc9513 100644 --- a/esphome/build_helpers/ninja.py +++ b/esphome/build_helpers/ninja.py @@ -25,11 +25,7 @@ def _ninja_runs(binary: str) -> bool: def find_ninja() -> Path: """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. - """ + PyPI wheel.""" if binary := shutil.which("ninja"): binary = strip_win_long_path_prefix(binary) if _ninja_runs(binary): @@ -58,13 +54,9 @@ def escape(value: Path | str) -> str: def quote_arg(tok: str) -> str: - """Wrap a token in double quotes with the Windows argv rule. - - 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. CreateProcess-only; POSIX sh collapses - backslash runs inside double quotes, so shell_token single-quotes - there instead. ``$`` must already be doubled for ninja. + """Quote with the CreateProcess argv rule (as ``subprocess.list2cmdline``): + backslash runs double only before a quote. Windows-only; ``$`` 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) @@ -78,16 +70,11 @@ _NEEDS_QUOTE = re.compile(r"[^\w@%+=:,./-]") def shell_token(tok: str, force: bool = False) -> str: - """Quote a lexed token only when needed; ``force`` always quotes. + """Re-quote a lexed token for the platform shell; ``force`` always quotes. - 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. 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. + Single quotes on POSIX (/bin/sh), the argv rule on Windows + (CreateProcess). ``$`` is doubled first because ninja expands it before + the command reaches the shell. """ tok = tok.replace("$", "$$") # ninja would expand a bare $ to nothing if not (force or not tok or _NEEDS_QUOTE.search(tok)): diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 3977c6560c..c6d9ef13f1 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -1155,11 +1155,8 @@ def _ccache_env() -> dict[str, str]: Only values the user has not already set in the environment are returned, so 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. + # IDF_CCACHE_ENABLE (the backend-native knob) wins over the shared + # ESPHOME_CCACHE_ENABLE. idf_knob = parse_enable_env("IDF_CCACHE_ENABLE") if idf_knob is False: return {} @@ -1167,8 +1164,6 @@ def _ccache_env() -> dict[str, str]: # ESP-IDF silently skips ccache without the binary; don't enable it. return {} - # ccache is enabled past here; the shared helper carries the CCACHE_* - # policy (and the fail-loud build_path guard). env = ccache_defaults_env(get_idf_tools_path() / "ccache") if idf_knob is None: # An unparsable IDF_CCACHE_ENABLE must not leak to idf.py as truthy diff --git a/esphome/framework_helpers.py b/esphome/framework_helpers.py index 3da8f403ee..d98d9e4982 100644 --- a/esphome/framework_helpers.py +++ b/esphome/framework_helpers.py @@ -199,10 +199,8 @@ def run_command( 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. + ``shutil.which`` proves existence, not runnability (Windows .bat/.cmd + shims, stale package-manager shims). """ try: subprocess.run( diff --git a/esphome/platformio/registry.py b/esphome/platformio/registry.py index 09a66e0cf9..99ccbe2334 100644 --- a/esphome/platformio/registry.py +++ b/esphome/platformio/registry.py @@ -1,9 +1,5 @@ -"""Install packages from the PlatformIO registry without PlatformIO. - -Native toolchains install the exact registry packages the PlatformIO backend -uses, so the bits are identical, but resolve and verify them with esphome's -own download machinery instead of importing the platformio package. -""" +"""Install packages from the PlatformIO registry without importing the +platformio package (identical bits, esphome's own download machinery).""" from __future__ import annotations @@ -33,11 +29,9 @@ _REGISTRY_URL = ( def get_systype() -> str: """The registry system tag for the current host. - A transliteration of ``platformio.util.get_systype()``, honoring the same - ``PLATFORMIO_SYSTEM_TYPE`` override, so this module never imports the - platformio package. One deviation: windows-arm64 maps straight to - ``windows_amd64``: the registry ships no arm64 toolchains and those hosts - run x86 binaries via emulation, which upstream leaves to the override. + Transliterates ``platformio.util.get_systype()`` (same + ``PLATFORMIO_SYSTEM_TYPE`` override). Deviation: windows-arm64 maps to + ``windows_amd64`` (no arm64 toolchains; x86 emulation). """ if systype := os.environ.get("PLATFORMIO_SYSTEM_TYPE"): return systype @@ -100,10 +94,8 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None] f"Unexpected package registry response for {package}: " f"{str(ver)[:200]}" ) - # 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 - # substring test. + # Only a missing key means "any system"; an empty list must not + # match, and a bare string would make ``in`` a substring test. systems = file.get("system") if systems is None: systems = ["*"] @@ -138,12 +130,8 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None] 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. - """ + """Raise when an install tree is missing an expected directory (runs on + fresh extracts and on marker hits).""" for rel in expect: if not (dest / rel).is_dir(): raise EsphomeError( @@ -177,21 +165,16 @@ def install_package( return from filelock import FileLock - # The cache is machine-global; serialize concurrent cold builds so one - # process cannot wipe the directory another is extracting into (same - # filelock pattern as platformio/toolchain.py and git.py). + # Serialize concurrent cold builds (same filelock pattern as git.py). dest.parent.mkdir(parents=True, exist_ok=True) - # fallback_to_soft would silently degrade to an existence lock on a - # flock-less filesystem; a hard-killed run would then hang every later - # build forever (same hazard git.py documents). + # A soft-lock fallback would turn a hard-killed run into a permanent + # hang (see git.py). with FileLock(f"{dest}.lock", fallback_to_soft=False): if marker.is_file(): # Another process finished the install while we waited return rmdir(dest, msg=f"Clean up incomplete {name} install") - # A persistent download location (not a temp dir) so an interrupted - # download resumes across esphome runs via download_with_resume's - # .part file, mirroring the espidf dist/ convention. + # Persistent location so an interrupted download resumes across runs. downloads_dir.mkdir(parents=True, exist_ok=True) archive = downloads_dir / f"{name}-{version}" _LOGGER.info("Downloading %s %s ...", name, version) diff --git a/tests/unit_tests/build_helpers/test_ninja.py b/tests/unit_tests/build_helpers/test_ninja.py index b1a43bd4af..6f0bbda0b9 100644 --- a/tests/unit_tests/build_helpers/test_ninja.py +++ b/tests/unit_tests/build_helpers/test_ninja.py @@ -4,6 +4,7 @@ from __future__ import annotations import os from pathlib import Path +import subprocess import sys from unittest.mock import MagicMock, patch @@ -84,7 +85,6 @@ def test_shell_token_quotes_shell_metacharacters() -> None: def test_shell_token_posix_roundtrips_through_sh() -> None: """Backslash runs, $, backticks, and quotes must reach the compiler exactly as lexed once ninja un-doubles $$ and /bin/sh strips quotes.""" - import subprocess if sys.platform == "win32": pytest.skip("POSIX sh quoting") diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index 32e2d7a3fb..d66f1fc7db 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -1395,8 +1395,6 @@ def test_get_framework_env_without_python_env_uses_os_path(tmp_path: Path) -> No def _ccache_patches(tmp_path: Path, which: str | None, build_path: Path | None): return ( - # The gate defers to the shared resolver (which carries the PATH - # lookup, ESPHOME_CCACHE_ENABLE parse, and runnability probe) patch("esphome.espidf.framework.resolve_ccache_path", return_value=which), patch( "esphome.espidf.framework.get_idf_tools_path",