diff --git a/esphome/__main__.py b/esphome/__main__.py index 88c314ad8d..6393e4a70e 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -2739,8 +2739,10 @@ def run_esphome(argv): ) # An explicit CLI toolchain must run the per-platform validators; the # cache was validated under whatever the last compile used. Only the - # read is gated: the refresh below may still save the freshly - # validated config, and its sidecar records the resolved toolchain. + # read is gated: the refresh below still saves the freshly validated + # config. The sidecar is only written when none exists; a + # compile-written one keeps the compile's toolchain (the firmware on + # disk was built by it), which upload/logs then restore. cache_read_eligible = cache_write_eligible and args.toolchain is None if cache_read_eligible: from esphome.compiled_config import load_compiled_config diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 70005f5d40..89e4bc2adf 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -54,16 +54,28 @@ def resolve_ccache_path() -> str | None: 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. The + 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). """ import shutil - from esphome.helpers import get_bool_env - - explicit = "ESPHOME_CCACHE_ENABLE" in os.environ - if explicit and not get_bool_env("ESPHOME_CCACHE_ENABLE"): + # Strict parse: bool(str) truthiness would flip "no"/"off" to enabled + # and silently skip the probe + raw = os.environ.get("ESPHOME_CCACHE_ENABLE") + explicit: bool | None = None + if raw is not None: + lowered = raw.strip().lower() + if lowered in ("1", "true", "yes", "on"): + explicit = True + elif lowered in ("0", "false", "no", "off"): + explicit = False + else: + _LOGGER.warning( + "Ignoring unrecognized ESPHOME_CCACHE_ENABLE=%r; use 1 or 0", raw + ) + if explicit is False: return None ccache = shutil.which("ccache") if ccache is None: diff --git a/esphome/platformio/registry.py b/esphome/platformio/registry.py index 89cf76de10..58fd62a46d 100644 --- a/esphome/platformio/registry.py +++ b/esphome/platformio/registry.py @@ -79,6 +79,10 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None] f"Unexpected package registry response for {package}: {str(data)[:200]}" ) for ver in versions: + if not isinstance(ver, dict): + raise EsphomeError( + f"Unexpected package registry response for {package}: {str(data)[:200]}" + ) if ver.get("name") != version: continue files = ver.get("files") @@ -87,6 +91,11 @@ def registry_download(package: str, version: str) -> tuple[str, str, int | None] f"Unexpected package registry response for {package}: {str(ver)[:200]}" ) for file in files: + if not isinstance(file, dict): + raise EsphomeError( + 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 @@ -133,6 +142,10 @@ def install_package( substitution) is trusted as configured. ``downloads_dir`` holds the archive between runs so an interrupted download resumes. """ + if not expect: + # Layout validation before marker.touch() is the only guard against + # caching a truncated mirror archive as a good install + raise ValueError("install_package requires a non-empty expect") marker = dest / ".esphome_extracted" if marker.is_file(): return diff --git a/tests/unit_tests/build_helpers/test_ccache.py b/tests/unit_tests/build_helpers/test_ccache.py index ff9426b3ff..a505be0aa3 100644 --- a/tests/unit_tests/build_helpers/test_ccache.py +++ b/tests/unit_tests/build_helpers/test_ccache.py @@ -77,3 +77,25 @@ def test_defaults_env_requires_build_path() -> None: pytest.raises(ValueError, match="build_path"), ): ccache.ccache_defaults_env(Path("/x")) + + +@pytest.mark.parametrize("value", ["no", "off", "false", "0"]) +def test_resolve_opt_out_synonyms(value: str) -> None: + """Every recognized falsy spelling disables ccache.""" + with patch.dict(os.environ, {"ESPHOME_CCACHE_ENABLE": value}): + assert ccache.resolve_ccache_path() is None + + +def test_resolve_unrecognized_value_warns_and_probes( + caplog: pytest.LogCaptureFixture, +) -> None: + """An unparsable ESPHOME_CCACHE_ENABLE is treated as unset: it must not + silently enable ccache or skip the runnability probe.""" + with ( + patch.dict(os.environ, {"ESPHOME_CCACHE_ENABLE": "enabled"}), + patch("shutil.which", return_value="/usr/bin/ccache"), + patch.object(ccache, "_ccache_runs", return_value=False) as mock_probe, + ): + assert ccache.resolve_ccache_path() is None + mock_probe.assert_called_once() + assert "unrecognized ESPHOME_CCACHE_ENABLE" in caplog.text diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py index 4c9e67a1c6..43b1461917 100644 --- a/tests/unit_tests/test_espidf_framework.py +++ b/tests/unit_tests/test_espidf_framework.py @@ -1400,7 +1400,7 @@ def _ccache_patches(tmp_path: Path, which: str | None, build_path: Path | None): "esphome.espidf.framework.get_idf_tools_path", return_value=tmp_path / "tools", ), - # ccache_defaults_env (framework_helpers) reads CORE at call time + # ccache_defaults_env (build_helpers.ccache) reads CORE at call time patch( "esphome.core.CORE", SimpleNamespace(build_path=build_path), diff --git a/tests/unit_tests/test_framework_helpers.py b/tests/unit_tests/test_framework_helpers.py index 2022c15bfe..56724205c2 100644 --- a/tests/unit_tests/test_framework_helpers.py +++ b/tests/unit_tests/test_framework_helpers.py @@ -2091,3 +2091,37 @@ class TestGetProjectCxxCompileFlags: def test_empty_flags(self) -> None: with patch("esphome.core.CORE", _make_core_cxx(set())): assert get_project_cxx_compile_flags() == [] + + +@pytest.mark.parametrize( + ("platform", "input_path", "expected"), + [ + # win32: drive-letter extended-length prefix is stripped + ( + "win32", + "\\\\?\\C:\\Users\\jesse\\AppData\\Local\\ESPHome Builder\\python\\python.exe", + "C:\\Users\\jesse\\AppData\\Local\\ESPHome Builder\\python\\python.exe", + ), + # win32: UNC extended-length prefix is translated to a regular UNC path + ( + "win32", + "\\\\?\\UNC\\server\\share\\python.exe", + "\\\\server\\share\\python.exe", + ), + # win32: paths without the prefix are returned unchanged + ( + "win32", + "C:\\Users\\jesse\\AppData\\Local\\ESPHome Builder\\python\\python.exe", + "C:\\Users\\jesse\\AppData\\Local\\ESPHome Builder\\python\\python.exe", + ), + # non-win32: prefix is left alone (no-op) + ("linux", "\\\\?\\C:\\python.exe", "\\\\?\\C:\\python.exe"), + ("darwin", "/usr/bin/python3", "/usr/bin/python3"), + ], +) +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.framework_helpers.sys.platform", platform): + assert framework_helpers.strip_win_long_path_prefix(input_path) == expected diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 1bf971fbb1..b991c9900e 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -7253,8 +7253,8 @@ def test_cli_toolchain_still_refreshes_the_validated_config_cache( tmp_path: Path, ) -> None: """An explicit --toolchain gates only the cache read; the freshly - validated config is still saved, and its sidecar records the resolved - toolchain for a later plain run.""" + validated config is still saved so a later plain run keeps the fast + path (an existing compile-written sidecar keeps its toolchain).""" from esphome.__main__ import run_esphome conf = tmp_path / "device.yaml" diff --git a/tests/unit_tests/test_platformio_registry.py b/tests/unit_tests/test_platformio_registry.py index a8f895ad43..caf5825c40 100644 --- a/tests/unit_tests/test_platformio_registry.py +++ b/tests/unit_tests/test_platformio_registry.py @@ -204,7 +204,9 @@ 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", expect=()) + registry.install_package( + "pkg", "1.0.0", dest, [], tmp_path / "dl", expect=("payload",) + ) mock_download.assert_not_called() @@ -217,9 +219,11 @@ def test_install_package_downloads_via_mirrors(tmp_path: Path) -> None: patch.object(registry, "get_systype", return_value="linux_x86_64"), ): # Extraction is expected to create the directory - mock_extract.side_effect = lambda *_a, **_kw: dest.mkdir() + mock_extract.side_effect = lambda *_a, **_kw: (dest / "payload").mkdir( + parents=True + ) registry.install_package( - "pkg", "1.0.0", dest, mirrors, tmp_path / "dl", expect=() + "pkg", "1.0.0", dest, mirrors, tmp_path / "dl", expect=("payload",) ) assert mock_download.call_args[0][0] is mirrors assert mock_download.call_args[0][1] == { @@ -241,8 +245,12 @@ def test_install_package_downloads_via_registry(tmp_path: Path) -> None: return_value=("http://x/pkg.tar.gz", "abc123", 42), ), ): - mock_extract.side_effect = lambda *_a, **_kw: dest.mkdir() - registry.install_package("pkg", "1.0.0", dest, [], tmp_path / "dl", expect=()) + mock_extract.side_effect = lambda *_a, **_kw: (dest / "payload").mkdir( + parents=True + ) + registry.install_package( + "pkg", "1.0.0", dest, [], tmp_path / "dl", expect=("payload",) + ) assert mock_download.call_args[0][0] == "http://x/pkg.tar.gz" assert mock_download.call_args[1] == {"sha256": "abc123", "size": 42} @@ -270,7 +278,9 @@ def test_install_package_unexpected_layout_raises(tmp_path: Path) -> None: patch.object(registry, "get_systype", return_value="linux_x86_64"), pytest.raises(EsphomeError, match="without the expected bin"), ): - mock_extract.side_effect = lambda *_a, **_kw: dest.mkdir() + mock_extract.side_effect = lambda *_a, **_kw: (dest / "payload").mkdir( + parents=True + ) registry.install_package( "pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl", expect=("bin",) ) @@ -294,7 +304,7 @@ def test_install_package_marker_rechecked_under_lock(tmp_path: Path) -> None: patch.object(registry, "rmdir") as mock_rmdir, ): registry.install_package( - "pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl", expect=() + "pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl", expect=("payload",) ) mock_download.assert_not_called() mock_rmdir.assert_not_called() @@ -309,9 +319,11 @@ def test_install_package_uses_hard_lock(tmp_path: Path) -> None: patch.object(registry, "archive_extract_all") as mock_extract, patch.object(registry, "get_systype", return_value="linux_x86_64"), ): - mock_extract.side_effect = lambda *_a, **_kw: dest.mkdir(exist_ok=True) + mock_extract.side_effect = lambda *_a, **_kw: (dest / "payload").mkdir( + parents=True, exist_ok=True + ) registry.install_package( - "pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl", expect=() + "pkg", "1.0.0", dest, ["http://m"], tmp_path / "dl", expect=("payload",) ) assert mock_lock.call_args.kwargs["fallback_to_soft"] is False @@ -364,3 +376,43 @@ def test_registry_download_missing_download_url_is_named() -> None: pytest.raises(EsphomeError, match="no download URL"), ): registry.registry_download("pkg", "1.0.0") + + +def test_install_package_empty_expect_rejected(tmp_path: Path) -> None: + """Layout validation is the only guard before marker.touch(), so an + empty expect is a caller bug, not a lenient install.""" + with pytest.raises(ValueError, match="non-empty expect"): + registry.install_package( + "pkg", "1.0.0", tmp_path / "pkg", [], tmp_path / "dl", expect=() + ) + + +def test_registry_download_non_dict_version_entry_is_named() -> None: + """A versions list of bare strings is an unexpected payload, not an + AttributeError traceback.""" + + def fake_download(mirrors: list[str], substitutions: dict, target) -> str: + target.write(json.dumps({"versions": ["1.0.0", "2.0.0"]}).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_non_dict_file_entry_is_named() -> None: + def fake_download(mirrors: list[str], substitutions: dict, target) -> str: + target.write( + json.dumps( + {"versions": [{"name": "1.0.0", "files": ["a.tar.gz"]}]} + ).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") diff --git a/tests/unit_tests/test_platformio_toolchain.py b/tests/unit_tests/test_platformio_toolchain.py index 555f25e06d..65c61619df 100644 --- a/tests/unit_tests/test_platformio_toolchain.py +++ b/tests/unit_tests/test_platformio_toolchain.py @@ -841,40 +841,6 @@ def test_ccache_wrapper_through_cmd_exe( assert marker.read_text() == "compiled" -@pytest.mark.parametrize( - ("platform", "input_path", "expected"), - [ - # win32: drive-letter extended-length prefix is stripped - ( - "win32", - "\\\\?\\C:\\Users\\jesse\\AppData\\Local\\ESPHome Builder\\python\\python.exe", - "C:\\Users\\jesse\\AppData\\Local\\ESPHome Builder\\python\\python.exe", - ), - # win32: UNC extended-length prefix is translated to a regular UNC path - ( - "win32", - "\\\\?\\UNC\\server\\share\\python.exe", - "\\\\server\\share\\python.exe", - ), - # win32: paths without the prefix are returned unchanged - ( - "win32", - "C:\\Users\\jesse\\AppData\\Local\\ESPHome Builder\\python\\python.exe", - "C:\\Users\\jesse\\AppData\\Local\\ESPHome Builder\\python\\python.exe", - ), - # non-win32: prefix is left alone (no-op) - ("linux", "\\\\?\\C:\\python.exe", "\\\\?\\C:\\python.exe"), - ("darwin", "/usr/bin/python3", "/usr/bin/python3"), - ], -) -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.framework_helpers.sys.platform", platform): - assert toolchain.strip_win_long_path_prefix(input_path) == expected - - def test_run_platformio_cli_strips_win_long_path_prefix( setup_core: Path, mock_run_external_process: Mock ) -> None: