diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 46483f0577..87dd337396 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -1,7 +1,9 @@ """Shared ccache policy for build backends. -One place for the probe, the enable/override rules, and the ``CCACHE_*`` -defaults, so the backends cannot drift apart. +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. """ from __future__ import annotations diff --git a/esphome/build_helpers/ninja.py b/esphome/build_helpers/ninja.py index 6a7bac1f56..3b16a9d57d 100644 --- a/esphome/build_helpers/ninja.py +++ b/esphome/build_helpers/ninja.py @@ -64,12 +64,14 @@ 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 expands ``$VAR`` while CreateProcess passes - it literally -- the same divergence SCons-under-sh has, so this stays - PlatformIO parity. + 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. """ tok = tok.replace("$", "$$") # ninja would expand a bare $ to nothing - if force or _NEEDS_QUOTE.search(tok): + if force or not tok or _NEEDS_QUOTE.search(tok): + # An empty token must become "" or it vanishes from the argv return quote_arg(tok) return tok diff --git a/esphome/platformio/registry.py b/esphome/platformio/registry.py index b4ee997036..e6db487c83 100644 --- a/esphome/platformio/registry.py +++ b/esphome/platformio/registry.py @@ -71,13 +71,25 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None] f"The package registry returned invalid JSON for {package}: {err}" ) from err systype = get_systype() - for ver in data.get("versions", []): + versions = data.get("versions") + if not isinstance(versions, list): + # A schema change or an error/captive-portal payload must not be + # reported as "version not found" + raise EsphomeError( + f"Unexpected package registry response for {package}: {str(data)[:200]}" + ) + for ver in versions: if ver.get("name") != version: continue for file in ver.get("files", []): - # A bare string would make ``in`` a substring test - systems = file.get("system") or "*" - if isinstance(systems, str): + # 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. + systems = file.get("system") + if systems is None: + systems = ["*"] + elif isinstance(systems, str): systems = [systems] if "*" in systems or systype in systems: sha256 = (file.get("checksum") or {}).get("sha256") @@ -101,7 +113,7 @@ def install_package( dest: Path, mirrors: list[str], downloads_dir: Path, - expect: Collection[str] = (), + expect: Collection[str], ) -> None: """Download, verify, and extract one package if not already installed. diff --git a/tests/unit_tests/build_helpers/test_ccache.py b/tests/unit_tests/build_helpers/test_ccache.py new file mode 100644 index 0000000000..ff9426b3ff --- /dev/null +++ b/tests/unit_tests/build_helpers/test_ccache.py @@ -0,0 +1,79 @@ +"""Tests for the shared ccache policy in esphome.build_helpers.ccache.""" + +from __future__ import annotations + +import os +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from esphome.build_helpers import ccache + + +def test_resolve_opt_out() -> None: + with patch.dict(os.environ, {"ESPHOME_CCACHE_ENABLE": "0"}): + assert ccache.resolve_ccache_path() is None + + +def test_resolve_no_binary(caplog: pytest.LogCaptureFixture) -> None: + with ( + patch.dict(os.environ, {}, clear=True), + patch("shutil.which", return_value=None), + ): + assert ccache.resolve_ccache_path() is None + assert "no ccache binary" not in caplog.text + + +def test_resolve_probe_failure() -> None: + with ( + patch.dict(os.environ, {}, clear=True), + patch("shutil.which", return_value="/usr/bin/ccache"), + patch( + "esphome.build_helpers.ccache.subprocess.run", side_effect=OSError("boom") + ), + ): + assert ccache.resolve_ccache_path() is None + + +def test_resolve_explicit_skips_probe_and_warns_missing( + caplog: pytest.LogCaptureFixture, +) -> None: + with ( + patch.dict(os.environ, {"ESPHOME_CCACHE_ENABLE": "1"}, clear=True), + patch("shutil.which", return_value="/usr/bin/ccache"), + patch.object(ccache, "_ccache_runs", side_effect=AssertionError), + ): + assert ccache.resolve_ccache_path() == "/usr/bin/ccache" + with ( + patch.dict(os.environ, {"ESPHOME_CCACHE_ENABLE": "1"}, clear=True), + patch("shutil.which", return_value=None), + ): + assert ccache.resolve_ccache_path() is None + assert "no ccache binary is on PATH" in caplog.text + + +def test_probe_spawns_with_close_fds_false() -> None: + with patch("esphome.build_helpers.ccache.subprocess.run") as mock_run: + assert ccache._ccache_runs("/usr/bin/ccache") is True + assert mock_run.call_args.kwargs["close_fds"] is False + + +def test_defaults_env(tmp_path: Path) -> None: + with ( + patch("esphome.core.CORE", SimpleNamespace(build_path=tmp_path / "b")), + patch.dict(os.environ, {"CCACHE_NOHASHDIR": "false"}, clear=True), + ): + env = ccache.ccache_defaults_env(tmp_path / "cache") + assert env["CCACHE_DIR"] == str(tmp_path / "cache") + assert env["CCACHE_DEPEND"] == "1" + assert "CCACHE_NOHASHDIR" not in env # user value respected + + +def test_defaults_env_requires_build_path() -> None: + with ( + patch("esphome.core.CORE", SimpleNamespace(build_path=None)), + pytest.raises(ValueError, match="build_path"), + ): + ccache.ccache_defaults_env(Path("/x")) diff --git a/tests/unit_tests/build_helpers/test_ninja.py b/tests/unit_tests/build_helpers/test_ninja.py index 1dc49396c6..ea4998e107 100644 --- a/tests/unit_tests/build_helpers/test_ninja.py +++ b/tests/unit_tests/build_helpers/test_ninja.py @@ -76,3 +76,8 @@ def test_shell_token_quotes_shell_metacharacters() -> None: def test_quote_path_force_quotes() -> None: assert ninja_helper.quote_path(Path("a b")) == '"a b"' assert ninja_helper.quote_path("simple") == '"simple"' + + +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("") == '""' diff --git a/tests/unit_tests/test_platformio_registry.py b/tests/unit_tests/test_platformio_registry.py index 2883b6a651..6c4ad83c4c 100644 --- a/tests/unit_tests/test_platformio_registry.py +++ b/tests/unit_tests/test_platformio_registry.py @@ -204,7 +204,7 @@ def test_install_package_skips_when_marker_exists(tmp_path: Path) -> None: dest.mkdir() (dest / ".esphome_extracted").touch() with patch.object(registry, "download_from_mirrors") as mock_download: - registry.install_package("pkg", "1.0.0", dest, [], tmp_path / "dl") + registry.install_package("pkg", "1.0.0", dest, [], tmp_path / "dl", expect=()) mock_download.assert_not_called() @@ -218,7 +218,9 @@ def test_install_package_downloads_via_mirrors(tmp_path: Path) -> None: ): # Extraction is expected to create the directory mock_extract.side_effect = lambda *_a, **_kw: dest.mkdir() - registry.install_package("pkg", "1.0.0", dest, mirrors, tmp_path / "dl") + registry.install_package( + "pkg", "1.0.0", dest, mirrors, tmp_path / "dl", expect=() + ) assert mock_download.call_args[0][0] is mirrors assert mock_download.call_args[0][1] == { "VERSION": "1.0.0", @@ -240,7 +242,7 @@ def test_install_package_downloads_via_registry(tmp_path: Path) -> None: ), ): mock_extract.side_effect = lambda *_a, **_kw: dest.mkdir() - registry.install_package("pkg", "1.0.0", dest, [], tmp_path / "dl") + registry.install_package("pkg", "1.0.0", dest, [], tmp_path / "dl", expect=()) assert mock_download.call_args[0][0] == "http://x/pkg.tar.gz" assert mock_download.call_args[1] == {"sha256": "abc123", "size": 42} @@ -291,7 +293,9 @@ def test_install_package_marker_rechecked_under_lock(tmp_path: Path) -> None: patch.object(registry, "download_from_mirrors") as mock_download, patch.object(registry, "rmdir") as mock_rmdir, ): - registry.install_package("pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl") + registry.install_package( + "pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl", expect=() + ) mock_download.assert_not_called() mock_rmdir.assert_not_called() @@ -306,5 +310,39 @@ def test_install_package_uses_hard_lock(tmp_path: Path) -> None: patch.object(registry, "get_systype", return_value="linux_x86_64"), ): mock_extract.side_effect = lambda *_a, **_kw: dest.mkdir(exist_ok=True) - registry.install_package("pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl") + registry.install_package( + "pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl", expect=() + ) assert mock_lock.call_args.kwargs["fallback_to_soft"] is False + + +def test_registry_download_empty_system_list_does_not_match() -> None: + """An explicitly empty system list must not act as a wildcard.""" + with ( + _registry_response([{"system": [], "download_url": "http://x/any"}]), + patch.object(registry, "get_systype", return_value="linux_x86_64"), + pytest.raises(EsphomeError, match="No pkg 1.0.0 build"), + ): + registry.registry_download("pkg", "1.0.0") + + +def test_registry_download_unexpected_payload_is_named() -> None: + """An error envelope without a versions list is not 'version not found'.""" + + def fake_download(mirrors: list[str], substitutions: dict, target) -> str: + target.write(json.dumps({"message": "rate limited"}).encode()) + return "http://x" + + with ( + patch.object(registry, "download_from_mirrors", side_effect=fake_download), + pytest.raises(EsphomeError, match="Unexpected package registry response"), + ): + registry.registry_download("pkg", "1.0.0") + + +def test_registry_download_missing_system_key_matches_any() -> None: + """A file with no system key at all serves every host.""" + with _registry_response( + [{"download_url": "http://x/any", "checksum": {"sha256": "abc"}, "size": 1}] + ): + assert registry.registry_download("pkg", "1.0.0") == ("http://x/any", "abc", 1) diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index 77d9ed436e..555f25e06d 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -432,7 +432,7 @@ def test_ccache_env_enabled_by_default(setup_core: Path) -> None: with ( patch.dict(os.environ, {}, clear=True), patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.framework_helpers.subprocess.run"), + patch("esphome.build_helpers.ccache.subprocess.run"), ): env = toolchain._ccache_env() @@ -495,7 +495,7 @@ def test_ccache_env_disabled_when_probe_fails( with ( patch.dict(os.environ, {}, clear=True), patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.framework_helpers.subprocess.run", side_effect=probe_error), + patch("esphome.build_helpers.ccache.subprocess.run", side_effect=probe_error), ): env = toolchain._ccache_env() @@ -509,7 +509,7 @@ def test_ccache_env_forced_on_skips_probe(setup_core: Path) -> None: with ( patch.dict(os.environ, {"ESPHOME_CCACHE_ENABLE": "1"}, clear=True), patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.framework_helpers.subprocess.run") as mock_probe, + patch("esphome.build_helpers.ccache.subprocess.run") as mock_probe, ): env = toolchain._ccache_env() @@ -537,9 +537,9 @@ def test_ccache_env_strips_win_long_path_prefix(setup_core: Path) -> None: patch.dict(os.environ, {}, clear=True), # shutil.which is patched, so the win32 code path of the real # implementation (which crashes on a POSIX host) is never reached. - patch("esphome.platformio.toolchain.sys.platform", "win32"), + patch("esphome.framework_helpers.sys.platform", "win32"), patch("shutil.which", return_value=prefixed), - patch("esphome.framework_helpers.subprocess.run") as mock_probe, + patch("esphome.build_helpers.ccache.subprocess.run") as mock_probe, ): env = toolchain._ccache_env() @@ -588,7 +588,7 @@ def test_ccache_env_respects_user_values_and_refreshes_basedir( with ( patch.dict(os.environ, user_env, clear=True), patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.framework_helpers.subprocess.run"), + patch("esphome.build_helpers.ccache.subprocess.run"), ): env = toolchain._ccache_env() @@ -607,7 +607,7 @@ def test_run_platformio_cli_passes_ccache_env_to_subprocess_only( with ( patch.dict(os.environ, {}, clear=False), patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.framework_helpers.subprocess.run"), + patch("esphome.build_helpers.ccache.subprocess.run"), ): os.environ.pop("ESPHOME_CCACHE_ENABLE", None) mock_run_external_process.return_value = 0 @@ -629,7 +629,7 @@ def test_ccache_env_requires_build_path(setup_core: Path) -> None: with ( patch.dict(os.environ, {}, clear=True), patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.framework_helpers.subprocess.run"), + patch("esphome.build_helpers.ccache.subprocess.run"), pytest.raises(ValueError, match="CORE.build_path must be set"), ): toolchain._ccache_env() @@ -643,7 +643,7 @@ def test_run_platformio_cli_merges_caller_env( with ( patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.framework_helpers.subprocess.run"), + patch("esphome.build_helpers.ccache.subprocess.run"), ): mock_run_external_process.return_value = 0 toolchain.run_platformio_cli( @@ -871,7 +871,7 @@ def test_strip_win_long_path_prefix( platform: str, input_path: str, expected: str ) -> None: r"""``\\?\`` and ``\\?\UNC\`` prefixes are stripped only on win32.""" - with patch("esphome.platformio.toolchain.sys.platform", platform): + with patch("esphome.framework_helpers.sys.platform", platform): assert toolchain.strip_win_long_path_prefix(input_path) == expected @@ -898,7 +898,7 @@ def test_run_platformio_cli_strips_win_long_path_prefix( # so the stdlib sees it too) would send shutil.which down the Windows # code path, which crashes on a POSIX host. patch.dict(os.environ, {"ESPHOME_CCACHE_ENABLE": "0"}, clear=False), - patch("esphome.platformio.toolchain.sys.platform", "win32"), + patch("esphome.framework_helpers.sys.platform", "win32"), patch("esphome.platformio.toolchain.sys.executable", prefixed_exe), ): # Pop any pre-existing PYTHONEXEPATH so the assertion below reflects @@ -930,7 +930,7 @@ def test_run_platformio_cli_does_not_set_pythonexepath_without_strip( with ( patch.dict(os.environ, {}, clear=False), - patch("esphome.platformio.toolchain.sys.platform", "linux"), + patch("esphome.framework_helpers.sys.platform", "linux"), patch("esphome.platformio.toolchain.sys.executable", plain_exe), ): os.environ.pop("PYTHONEXEPATH", None)