From 3063b9067ad1470ffa6c23e5118e894c19cb4237 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 20:34:24 -0500 Subject: [PATCH 1/5] State what the fingerprint test actually pins --- tests/unit_tests/components/esp8266/test_build_surgery.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/components/esp8266/test_build_surgery.py b/tests/unit_tests/components/esp8266/test_build_surgery.py index 944518b1fa..7e6e7ea680 100644 --- a/tests/unit_tests/components/esp8266/test_build_surgery.py +++ b/tests/unit_tests/components/esp8266/test_build_surgery.py @@ -108,8 +108,8 @@ def test_board_build_covers_every_board() -> None: def test_surgery_fingerprint_covers_module_source() -> None: - """The fingerprint hashes the module source, so any surgery edit - invalidates linker-script caches stamped with it.""" + """Pins the mechanism: the fingerprint is the sha256 of the module + source (behavioral coverage follows from that, not from this test).""" import hashlib import inspect From b42babf749f094c14025a088c486cedf6a1bb9d0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 20:35:49 -0500 Subject: [PATCH 2/5] Route platformio build_unflags natively, correct the gate and fallback comments --- esphome/__main__.py | 10 +++++----- esphome/core/config.py | 14 ++++++++++---- tests/unit_tests/core/test_config.py | 2 ++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/esphome/__main__.py b/esphome/__main__.py index f1b0fb7592..7b1afd464b 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -2753,11 +2753,11 @@ def run_esphome(argv): return 2 CORE.config = config - # Fallback for platforms whose validators didn't set the toolchain - # (only the esp32 component reads esp32.framework.toolchain). All - # other platforms only support PlatformIO today. Must run before the - # cache refresh below so its sidecar records the same toolchain a - # compile would. + # Every platform resolves the toolchain during validation now, but the + # compiled-config cache fast path skips validation entirely and a + # sidecar written before the toolchain field existed restores nothing; + # this fallback covers that path. Must run before the cache refresh + # below so its sidecar records the same toolchain a compile would. if CORE.toolchain is None: CORE.toolchain = Toolchain.PLATFORMIO diff --git a/esphome/core/config.py b/esphome/core/config.py index 12f55283ad..1c481894b2 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -560,10 +560,11 @@ async def _add_platformio_options(pio_options: dict[str, str | list[str]]) -> No if CORE.using_native_toolchain: # The native builds don't read platformio.ini; honor the options # with a native equivalent and warn about the rest, which would - # otherwise be silently ignored. __main__'s write_cpp_file and - # compile_program dispatch must agree with this gate: a toolchain - # treated as native here must not fall through to the PlatformIO - # project writer there. + # otherwise be silently ignored. Every dispatch site that tests a + # specific using_toolchain_* as a stand-in for "native" (project + # writing, compile, upload, firmware paths) must agree with this + # gate: a toolchain treated as native here must never fall through + # to a PlatformIO code path there. for key, val in pio_options.items(): vals = [val] if isinstance(val, str) else val if key == CONF_BUILD_FLAGS: @@ -576,6 +577,11 @@ async def _add_platformio_options(pio_options: dict[str, str | list[str]]) -> No ) for flag in vals: cg.add_build_flag(flag) + elif key == "build_unflags": + # Native equivalent: add_build_unflag (honored token-level by + # the arduino generator; the IDF generator warns there) + for flag in vals: + CORE.add_build_unflag(flag) elif key == "lib_deps": # Routed through the regular library mechanism so the # libraries reach the native backend's converter (IDF diff --git a/tests/unit_tests/core/test_config.py b/tests/unit_tests/core/test_config.py index 80e7f3b5c2..f322c1d4c9 100644 --- a/tests/unit_tests/core/test_config.py +++ b/tests/unit_tests/core/test_config.py @@ -1285,6 +1285,7 @@ async def test_add_platformio_options_native_idf( await config._add_platformio_options( { "build_flags": "-DSINGLE_FLAG", # string and list forms both valid + "build_unflags": ["-Os"], "lib_deps": ["bblanchon/ArduinoJson@7.4.2"], "lib_ignore": "libsodium", "upload_speed": "115200", @@ -1294,6 +1295,7 @@ async def test_add_platformio_options_native_idf( assert "-DSINGLE_FLAG" in CORE.build_flags assert "ArduinoJson" in CORE.platformio_libraries + assert "-Os" in CORE.build_unflags # lib_ignore is stored (listified) for generate_idf_components to read; # nothing else lands in platformio_options on the native toolchain. assert CORE.platformio_options == {"lib_ignore": ["libsodium"]} From ec8437fea278186f99659182058962473783a308 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 20:38:09 -0500 Subject: [PATCH 3/5] Quote empty tokens, honest docstrings, strict registry payloads, required layout checks, honest patch targets --- esphome/build_helpers/ccache.py | 6 +- esphome/build_helpers/ninja.py | 10 ++- esphome/platformio/registry.py | 22 ++++-- tests/unit_tests/build_helpers/test_ccache.py | 79 +++++++++++++++++++ tests/unit_tests/build_helpers/test_ninja.py | 5 ++ tests/unit_tests/test_platformio_registry.py | 48 +++++++++-- tests/unit_tests/test_platformio_toolchain.py | 24 +++--- 7 files changed, 166 insertions(+), 28 deletions(-) create mode 100644 tests/unit_tests/build_helpers/test_ccache.py 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) From 4969d9bf4a76d24edc69b31845ef0207cb40656c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 20:39:20 -0500 Subject: [PATCH 4/5] Pin the lx106 layout, reject non-3.x cores, pin installer args, delegate ccache policy tests --- esphome/arduino8266/framework.py | 12 ++- .../unit_tests/test_arduino8266_framework.py | 92 +++++++------------ 2 files changed, 42 insertions(+), 62 deletions(-) diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index da10502bc2..219d0f2fb9 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -63,8 +63,15 @@ def framework_package_version(ver: Version) -> str: Same encoding as the PlatformIO package registry uses for core 3.x releases (3.1.2 -> 3.30102.0). The native toolchain only supports core - >= MIN_FRAMEWORK_VERSION, so the 1.x/2.x encodings never apply here. + 3.x: 1.x/2.x fall below MIN_FRAMEWORK_VERSION, and a future major bump + needs its own encoding and toolchain pin rather than a registry lookup + for a package that cannot exist. """ + if ver.major != 3: + raise EsphomeError( + f"The native toolchain does not support Arduino core {ver} " + "(only 3.x); use 'toolchain: platformio'" + ) return f"3.{ver.major}{ver.minor:02d}{ver.patch:02d}.0" @@ -113,7 +120,8 @@ def check_and_install(framework_version: Version) -> InstalledPaths: toolchain_path, ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS, downloads_dir, - expect=("bin",), + # xtensa-lx106-elf pins the target: every gcc package has a bin/ + expect=("bin", "xtensa-lx106-elf"), ) return InstalledPaths( framework=framework_path, toolchain=toolchain_path, ninja=ninja_path diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index e22b0b185c..0d84d024ea 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -4,7 +4,6 @@ from __future__ import annotations import os from pathlib import Path -import subprocess from unittest.mock import patch import pytest @@ -23,6 +22,9 @@ def _clear_caches(tmp_path: Path) -> None: def test_framework_package_version() -> None: assert framework.framework_package_version(cv.Version(3, 1, 2)) == "3.30102.0" assert framework.framework_package_version(cv.Version(3, 2, 0)) == "3.30200.0" + # A future major bump needs its own encoding, not a doomed registry lookup + with pytest.raises(EsphomeError, match="only 3.x"): + framework.framework_package_version(cv.Version(4, 0, 0)) def test_tools_path_default_and_prefix(tmp_path: Path) -> None: @@ -46,11 +48,25 @@ def test_check_and_install_returns_paths(tmp_path: Path) -> None: assert paths.toolchain == tmp_path / "toolchains" / framework.TOOLCHAIN_VERSION assert paths.ninja == tmp_path / "ninja" assert mock_install.call_count == 2 - # The layout checks cover the directories write_project needs, including - # the bundled libraries/ tree - fw_expect = mock_install.call_args_list[0].kwargs["expect"] - assert fw_expect == ("cores/esp8266", "tools/sdk", "libraries") - assert mock_install.call_args_list[1].kwargs["expect"] == ("bin",) + # Full argument pinning: a copy-paste swap between the two near-identical + # calls (mirrors, destination) must not stay green + fw_call, tc_call = mock_install.call_args_list + assert fw_call.args == ( + framework.FRAMEWORK_PACKAGE, + "3.30102.0", + tmp_path / "frameworks" / "3.30102.0", + framework.ESPHOME_ARDUINO8266_FRAMEWORK_MIRRORS, + tmp_path / "downloads", + ) + assert fw_call.kwargs["expect"] == ("cores/esp8266", "tools/sdk", "libraries") + assert tc_call.args == ( + framework.TOOLCHAIN_PACKAGE, + framework.TOOLCHAIN_VERSION, + tmp_path / "toolchains" / framework.TOOLCHAIN_VERSION, + framework.ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS, + tmp_path / "downloads", + ) + assert tc_call.kwargs["expect"] == ("bin", "xtensa-lx106-elf") def test_get_build_env_prepends_toolchain_bin(tmp_path: Path) -> None: @@ -60,52 +76,18 @@ def test_get_build_env_prepends_toolchain_bin(tmp_path: Path) -> None: assert env["CCACHE_DIR"] == "x" -def test_ccache_path_disabled_by_env() -> None: - with patch.dict(os.environ, {"ESPHOME_CCACHE_ENABLE": "0"}): - assert framework.ccache_path() is None - - -def test_ccache_path_no_binary(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.delenv("ESPHOME_CCACHE_ENABLE", raising=False) - with patch("shutil.which", return_value=None): - assert framework.ccache_path() is None - - -def test_ccache_path_probe_failure(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.delenv("ESPHOME_CCACHE_ENABLE", raising=False) - with ( - patch("shutil.which", return_value="/usr/bin/ccache"), - patch("subprocess.run", side_effect=subprocess.SubprocessError), - ): - assert framework.ccache_path() is None - - -def test_ccache_path_ok(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.delenv("ESPHOME_CCACHE_ENABLE", raising=False) - with ( - patch("shutil.which", return_value="/usr/bin/ccache"), - patch("subprocess.run"), - ): - assert framework.ccache_path() == "/usr/bin/ccache" - - -def test_ccache_path_explicit_missing_binary_warns( - monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +def test_ccache_path_delegates_and_caches( + monkeypatch: pytest.MonkeyPatch, ) -> None: - monkeypatch.setenv("ESPHOME_CCACHE_ENABLE", "1") - with patch("shutil.which", return_value=None): - assert framework.ccache_path() is None - assert "no ccache binary is on PATH" in caplog.text - - -def test_ccache_path_explicit_skips_probe(monkeypatch: pytest.MonkeyPatch) -> None: - """An explicit opt-in trusts the binary without the runnability probe.""" - monkeypatch.setenv("ESPHOME_CCACHE_ENABLE", "1") - with ( - patch("shutil.which", return_value="/usr/bin/ccache"), - patch("esphome.build_helpers.ccache._ccache_runs", side_effect=AssertionError), - ): + """The wrapper delegates to the shared policy (covered in + build_helpers/test_ccache.py) and caches the result.""" + monkeypatch.delenv("ESPHOME_CCACHE_ENABLE", raising=False) + with patch.object( + framework, "resolve_ccache_path", return_value="/usr/bin/ccache" + ) as mock_resolve: assert framework.ccache_path() == "/usr/bin/ccache" + assert framework.ccache_path() == "/usr/bin/ccache" + mock_resolve.assert_called_once() def test_ccache_env(tmp_path: Path) -> None: @@ -123,16 +105,6 @@ def test_ccache_env(tmp_path: Path) -> None: assert env["CCACHE_DIR"].endswith("ccache") -def test_ccache_env_requires_build_path() -> None: - """Building the env before preload set build_path fails loudly.""" - CORE.build_path = None - with ( - patch.object(framework, "ccache_path", return_value="/cc/ccache"), - pytest.raises(ValueError, match="build_path"), - ): - framework.ccache_env() - - def test_check_and_install_rejects_old_core(tmp_path: Path) -> None: """Calling the installer below the floor fails before any download.""" with pytest.raises(EsphomeError, match=">= 3.1.1"): From fdbd4157ff9e17ee9a9eb921eb88b755e2a74df3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 20:41:17 -0500 Subject: [PATCH 5/5] Carry lib_archive, warn on unresolvable deps, unquote rsp lines, fail empty archives, note deviations --- esphome/arduino/library.py | 49 ++++++++++++++--- esphome/build_gen/build_tool.py | 23 +++++--- tests/unit_tests/build_gen/test_build_tool.py | 37 +++++++++++++ tests/unit_tests/test_arduino_library.py | 53 +++++++++++++++++++ 4 files changed, 148 insertions(+), 14 deletions(-) diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index 443438a725..1c5840cd10 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -8,6 +8,11 @@ resolution/download pipeline in ``esphome.platformio.library``. Nothing here is core-specific: the caller names the PlatformIO platform, MCU, and cache key of the Arduino core it builds. +Known deviation: flat-layout (``library.properties``, no ``src/``) libraries +get the recursive default source filter rather than PlatformIO's root-only +Arduino-1.0 filter; no bundled library is affected, only user-supplied ones +carrying sources in unusual subdirectories. + Mirrors PlatformIO's ``lib_ldf_mode=off`` behavior: each library builds into its own static archive and every library's include dir joins one global include path. @@ -52,6 +57,9 @@ class ArduinoLibrary: include_dirs: list[Path] = field(default_factory=list) # Extra compile flags private to this library's own sources flags: list[str] = field(default_factory=list) + # PlatformIO's build.libArchive / Arduino's dot_a_linkage: when False the + # objects go to the linker directly (symbols nothing references survive) + lib_archive: bool = True # Link inputs the library contributes (-L dirs / -l libs, e.g. from # precompiled vendor blobs) and -Wl, options for the firmware link link_dirs: list[Path] = field(default_factory=list) @@ -79,7 +87,15 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: # PlatformIO shell-lexes each build.flags entry flag_tokens = lex_build_flags(build.get("flags", []), f"library {name}") - lib = ArduinoLibrary(name=name) + # PIO precedence: build.libArchive, else the Arduino-format + # dot_a_linkage property, else archive (PlatformIO's default) + if "libArchive" in build: + lib_archive = bool(build["libArchive"]) + elif "dot_a_linkage" in data: + lib_archive = str(data["dot_a_linkage"]).lower() == "true" + else: + lib_archive = True + lib = ArduinoLibrary(name=name, lib_archive=lib_archive) include_flags: list[str] = [] for tok in flag_tokens: if tok.startswith("-I"): @@ -190,13 +206,30 @@ def resolve_libraries( # it cannot be resolved from the registry. for dep in normalize_dependencies(component.data.get("dependencies")): name = dep.get("name") - if ( - not name - or dep.get("owner") - or "version" in dep - or name in bundled_names - or is_lib_ignored(name, lib_ignore) - ): + if not name: + _LOGGER.warning( + "Ignoring malformed dependency entry %r of library %s", + dep, + component.name, + ) + continue + if name in bundled_names or is_lib_ignored(name, lib_ignore): + continue + if "version" in dep: + # The converter resolves versioned deps from the registry. + # Note: the dict shorthand {"Wire": "*"} normalizes to + # version="*" and takes this path even for a bundled name; + # use the list form for bundled dependencies. + continue + if dep.get("owner"): + # Owner but no version: the converter skips it too, so this + # is the only place the drop can be made visible + _LOGGER.warning( + "Dependency %s of library %s has an owner but no version " + "to resolve; skipping", + name, + component.name, + ) continue if not (framework_path / "libraries" / name).is_dir(): # The shared converter skips version-less deps too, so this diff --git a/esphome/build_gen/build_tool.py b/esphome/build_gen/build_tool.py index 692d006661..4cdbb88c28 100644 --- a/esphome/build_gen/build_tool.py +++ b/esphome/build_gen/build_tool.py @@ -24,13 +24,24 @@ def main() -> int: # Expand the response file here instead of passing @rspfile: GNU ar # treats backslashes in response files as escapes, corrupting Windows # paths ("sub\a.o" -> "suba.o"). - # One path per line (rspfile_content = $in_newline, written without - # escaping), so a path containing a space survives. Expanding into - # argv trades away the OS command-line length limit rspfiles dodge; - # the relative object paths used here stay far below it. - objects = Path(rspfile).read_text(encoding="utf-8").splitlines() + # One path per line (rspfile_content = $in_newline). ninja shell-quotes + # a path containing specials, so undo a simple surrounding quote per + # line. Expanding into argv trades away the OS command-line length + # limit rspfiles dodge; the relative object paths here stay far + # below it. + objects = [ + line[1:-1] + if len(line) >= 2 and line[0] == line[-1] and line[0] in "'\"" + else line + for line in Path(rspfile).read_text(encoding="utf-8").splitlines() + if line + ] + if not objects: + # An empty archive would "succeed" here and fail far away at link + print(f"ar: no objects listed in {rspfile} for {archive}", file=sys.stderr) + return 1 return subprocess.run( - [ar, "rc", archive, *filter(None, objects)], check=False, close_fds=False + [ar, "rc", archive, *objects], check=False, close_fds=False ).returncode if mode == "copy": src, dst = sys.argv[2:4] diff --git a/tests/unit_tests/build_gen/test_build_tool.py b/tests/unit_tests/build_gen/test_build_tool.py index 0c8eb00f9d..875ffcb773 100644 --- a/tests/unit_tests/build_gen/test_build_tool.py +++ b/tests/unit_tests/build_gen/test_build_tool.py @@ -87,3 +87,40 @@ def test_ar_expands_rspfile_without_escaping(tmp_path) -> None: "obj/a.o", "sub\\b.o", ] + + +def test_ar_unquotes_ninja_escaped_paths(tmp_path: Path) -> None: + """The shim strips a simple surrounding quote, since ninja shell- + quotes special rsp paths, so ar sees the real filename.""" + rsp = tmp_path / "t.rsp" + rsp.write_text("'obj/a b.o'\nobj/c.o\n") + with ( + patch.object( + build_tool.sys, "argv", ["bt", "ar", "/usr/bin/ar", "lib.a", str(rsp)] + ), + patch.object(build_tool.subprocess, "run") as mock_run, + ): + mock_run.return_value.returncode = 0 + rc = build_tool.main() + assert rc == 0 + assert mock_run.call_args.args[0] == [ + "/usr/bin/ar", + "rc", + "lib.a", + "obj/a b.o", + "obj/c.o", + ] + + +def test_ar_empty_object_list_fails( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A lost object list is an error here, not undefined symbols at link.""" + rsp = tmp_path / "t.rsp" + rsp.write_text("\n\n") + with patch.object( + build_tool.sys, "argv", ["bt", "ar", "/usr/bin/ar", "lib.a", str(rsp)] + ): + rc = build_tool.main() + assert rc == 1 + assert "no objects listed" in capsys.readouterr().err diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index a4c19f8c31..81e17490aa 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -356,3 +356,56 @@ def test_bundled_library_prefers_library_json(tmp_path: Path) -> None: ) lib = component._bundled_library(framework, "GDBStub") assert [s.name for s in lib.sources] == ["gdb.cpp"] + + +def test_library_info_lib_archive_flag(tmp_path: Path) -> None: + """Both libArchive (library.json) and dot_a_linkage (properties) reach + the generator's contract; default is archive.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + assert component._library_info("x", read_path, {}).lib_archive is True + assert ( + component._library_info( + "x", read_path, {"build": {"libArchive": False}} + ).lib_archive + is False + ) + assert ( + component._library_info("x", read_path, {"dot_a_linkage": "false"}).lib_archive + is False + ) + assert ( + component._library_info("x", read_path, {"dot_a_linkage": "true"}).lib_archive + is True + ) + + +def test_resolve_libraries_dep_warnings( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """Nameless and owner-without-version dependencies are dropped loudly.""" + framework = _make_framework(tmp_path) + _add_library("ESP32Async/ESPAsyncWebServer", "3.9.6") + lib_dir = tmp_path / "converted" / "webserver" + (lib_dir / "src").mkdir(parents=True) + converted = _converted( + "esp32async__ESPAsyncWebServer", + lib_dir, + { + "build": {}, + "dependencies": [ + {"owner": "someone"}, + {"name": "Orphan", "owner": "someone"}, + ], + }, + ) + with _emitting_converter(converted): + component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) + assert "malformed dependency entry" in caplog.text + assert "Orphan" in caplog.text + assert "owner but no version" in caplog.text