From 048025e9ddc75d43626c27f5591da6418ef3feb4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 21 Aug 2026 20:44:49 -0500 Subject: [PATCH 1/3] Form the local-library URL with as_uri so the test passes on Windows --- tests/unit_tests/test_arduino_library.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 195bd781c0..4a593413e3 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -774,7 +774,9 @@ def test_dict_shorthand_dependency_skips_registry_through_real_converter( (local_lib / "library.json").write_text( '{"name": "LocalLib", "version": "1.0.0", "dependencies": {"Wire": "*"}}' ) - _add_library(f"file://{local_lib}", None) + # as_uri() forms a valid file:// URL on every platform (file:///C:/... + # on Windows; a bare f-string would embed backslashes) + _add_library(local_lib.as_uri(), None) # The real converter writes its component cache under the config dir CORE.config_path = tmp_path / "test.yaml" CORE.config_path.write_text("") From 4101ad4fb1e5aa6f3d921b8ced19a39695c05e1a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 21 Aug 2026 20:46:10 -0500 Subject: [PATCH 2/3] One owner for the toolchain tool layout, thread a pre-resolved ccache toolchain_tool carries the bin/xtensa-lx106-elf- pattern and the Windows suffix that four call sites previously spelled out (only one of which handled the suffix). ccache_env and get_build_env accept the already-resolved ccache path so run_compile can resolve once instead of paying the PATH scan and runnability probe three times per build. --- esphome/arduino8266/framework.py | 30 +++++++++++++++---- .../unit_tests/test_arduino8266_framework.py | 18 +++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index a5a7eaf069..cf3a958fab 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -20,7 +20,7 @@ from __future__ import annotations import os from pathlib import Path -from typing import NamedTuple +from typing import Any, NamedTuple from esphome.build_helpers.ccache import ccache_defaults_env, resolve_ccache_path from esphome.build_helpers.ninja import find_ninja @@ -141,7 +141,25 @@ def check_and_install(framework_version: Version) -> InstalledPaths: ) -def get_build_env(toolchain_path: Path) -> dict[str, str]: +# Sentinel: "resolve for me" (None is a real value meaning disabled). +# run_compile resolves once and threads the result so one build never pays +# the PATH scan and runnability probe three times. +_CCACHE_UNRESOLVED: Any = object() + + +def toolchain_tool(toolchain_path: Path, name: str) -> Path: + """Path to one toolchain tool (gcc, g++, ar, size, addr2line, ...). + + The single owner of the ``bin/xtensa-lx106-elf-`` layout and the + Windows suffix, so a toolchain package bump touches one spot. + """ + suffix = ".exe" if os.name == "nt" else "" + return toolchain_path / "bin" / f"xtensa-lx106-elf-{name}{suffix}" + + +def get_build_env( + toolchain_path: Path, ccache: str | None = _CCACHE_UNRESOLVED +) -> dict[str, str]: env = os.environ.copy() # Drop empty entries: a trailing separator from an absent PATH would # make the shell search the current directory for tools @@ -150,7 +168,7 @@ def get_build_env(toolchain_path: Path) -> dict[str, str]: *filter(None, env.get("PATH", "").split(os.pathsep)), ] env["PATH"] = os.pathsep.join(parts) - env.update(ccache_env()) + env.update(ccache_env(ccache)) return env @@ -164,7 +182,7 @@ def ccache_path() -> str | None: return resolve_ccache_path() -def ccache_env() -> dict[str, str]: +def ccache_env(ccache: str | None = _CCACHE_UNRESOLVED) -> dict[str, str]: """Return ccache settings for the build subprocess (not os.environ). Mirrors ``espidf.framework._ccache_env``: cache under the machine-global @@ -172,6 +190,8 @@ def ccache_env() -> dict[str, str]: scoped to the build dir so devices share framework cache entries. Values the user already set in the environment are respected. """ - if ccache_path() is None: + if ccache is _CCACHE_UNRESOLVED: + ccache = ccache_path() + if ccache is None: return {} return ccache_defaults_env(get_arduino8266_tools_path() / "ccache") diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index 21bf756e6b..cd79c57f8f 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -152,3 +152,21 @@ def test_get_build_env_without_path_has_no_empty_entry(tmp_path: Path) -> None: ): env = framework.get_build_env(tmp_path) assert env["PATH"].split(os.pathsep) == [str(tmp_path / "bin"), "/usr/bin", "/bin"] + + +def test_ccache_env_accepts_a_preresolved_path() -> None: + """A caller that already resolved ccache threads it through; the probe + must not run again (None means resolved-and-disabled).""" + with patch.object(framework, "ccache_path") as mock_resolve: + assert framework.ccache_env(None) == {} + env = framework.ccache_env("/usr/bin/ccache") + mock_resolve.assert_not_called() + assert env["CCACHE_DIR"].endswith("ccache") + + +def test_toolchain_tool_layout(tmp_path: Path) -> None: + """One owner for the bin/xtensa-lx106-elf- layout.""" + tool = framework.toolchain_tool(tmp_path, "addr2line") + assert tool.parent == tmp_path / "bin" + assert tool.name.startswith("xtensa-lx106-elf-addr2line") + assert (tool.suffix == ".exe") is (os.name == "nt") From e5ad855337161c0f94640b8f2056d78d98b95b3d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 21 Aug 2026 20:47:17 -0500 Subject: [PATCH 3/3] Use the shared tool accessor for gcc, drop dead dataclass defaults, add a resolve test helper toolchain_tool owns the bin path and Windows suffix now; _BuildConfig's knob and MMU fields are required since the only constructor always passes both; the tests' set-flags-then-resolve idiom collapses into one _resolve helper (12 sites). --- esphome/build_gen/arduino8266.py | 9 +-- .../unit_tests/build_gen/test_arduino8266.py | 62 +++++++------------ 2 files changed, 28 insertions(+), 43 deletions(-) diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index abbaa3f8bf..8c71556d6f 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -15,13 +15,14 @@ from the build flags with the same precedence as the PlatformIO builder. from __future__ import annotations import contextlib -from dataclasses import dataclass, field +from dataclasses import dataclass import logging import os from pathlib import Path import subprocess from typing import TYPE_CHECKING +from esphome.arduino8266.framework import toolchain_tool from esphome.build_helpers.ninja import shell_token as _shell_token from esphome.components.esp8266 import build_surgery from esphome.core import CORE, EsphomeError @@ -168,8 +169,8 @@ class _BuildConfig: exceptions: bool vtables: str fp_in_irom: bool - knob_defines: list[str] = field(default_factory=list) - mmu_defines: list[str] = field(default_factory=list) + knob_defines: list[str] + mmu_defines: list[str] def _lexed_build_flags() -> list[str]: @@ -440,7 +441,7 @@ def generate_ld_scripts( rate-table DRAM relocation, and enlarged memory segments in testing mode. """ framework = paths.framework - gcc = paths.toolchain / "bin" / "xtensa-lx106-elf-gcc" + gcc = toolchain_tool(paths.toolchain, "gcc") ld_dir = CORE.relative_pioenvs_path(CORE.name, "ld") mkdir_p(ld_dir) diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index f52a9fa617..cebc7a6f2e 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -59,12 +59,15 @@ def _set_flags(*flags: str) -> None: CORE.build_flags = set(flags) +def _resolve(*flags: str): + """Set the build flags and resolve the knob config in one step.""" + _set_flags(*flags) + return _resolve_build_config(_flag_defines(set(), arduino8266._lexed_build_flags())) + + def test_build_config_defaults() -> None: - _set_flags() - config = _resolve_build_config( - _flag_defines(set(), arduino8266._lexed_build_flags()) - ) + config = _resolve() assert config.nonosdk == "NONOSDK22x_190703" assert config.lwip_lib == "lwip2-536-feat" assert not config.exceptions @@ -82,10 +85,7 @@ def test_build_config_esphome_lwip_knob() -> None: """The lwIP variant ESPHome selects maps to the same defines and library as the PlatformIO builder.""" - _set_flags("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH") - config = _resolve_build_config( - _flag_defines(set(), arduino8266._lexed_build_flags()) - ) + config = _resolve("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH") assert config.lwip_lib == "lwip2-1460" assert "TCP_MSS=1460" in config.knob_defines assert "LWIP_FEATURES=0" in config.knob_defines @@ -111,9 +111,8 @@ def test_build_config_knobs() -> None: def test_build_config_mmu_custom_requires_sizes() -> None: - _set_flags("-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM") with pytest.raises(EsphomeError, match="MMU_IRAM_SIZE"): - _resolve_build_config(_flag_defines(set(), arduino8266._lexed_build_flags())) + _resolve("-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM") _set_flags( "-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM", @@ -205,10 +204,7 @@ def test_build_config_lwip_variants( ) -> None: """Every lwIP knob maps to the same defines and library as the PIO builder.""" - _set_flags(f"-D{knob}") - config = _resolve_build_config( - _flag_defines(set(), arduino8266._lexed_build_flags()) - ) + config = _resolve(f"-D{knob}") assert config.lwip_lib == lib assert f"TCP_MSS={mss}" in config.knob_defines assert f"LWIP_FEATURES={features}" in config.knob_defines @@ -254,10 +250,7 @@ def test_build_config_mmu_variants(knob: str, expected: list[str]) -> None: def test_build_config_waveform_locked_phase() -> None: - _set_flags("-DPIO_FRAMEWORK_ARDUINO_WAVEFORM_LOCKED_PHASE", "-DFP_IN_IROM") - config = _resolve_build_config( - _flag_defines(set(), arduino8266._lexed_build_flags()) - ) + config = _resolve("-DPIO_FRAMEWORK_ARDUINO_WAVEFORM_LOCKED_PHASE", "-DFP_IN_IROM") assert "WAVEFORM_LOCKED_PHASE=1" in config.knob_defines assert config.fp_in_irom @@ -478,9 +471,8 @@ def test_build_config_custom_mmu_without_knob_raises() -> None: layout the linker script does not implement; refuse instead of warning (PlatformIO warns, but its defaults win the compile line; ours would not).""" - _set_flags("-DMMU_IRAM_SIZE=0xC000") with pytest.raises(EsphomeError, match="PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM"): - _resolve_build_config(_flag_defines(set(), arduino8266._lexed_build_flags())) + _resolve("-DMMU_IRAM_SIZE=0xC000") def test_flag_defines_lexes_quoted_single_tokens() -> None: @@ -602,15 +594,13 @@ def test_flag_defines_respects_unflags() -> None: def test_vtables_unknown_raises() -> None: """A typo'd knob would win the sorted pick and die in the SDK header's #error; fail by name at generation instead.""" - _set_flags("-DVTABLES_IN_BANANA") with pytest.raises(EsphomeError, match="Unknown VTABLES_IN_.*BANANA"): - _resolve_build_config(_flag_defines(set(), arduino8266._lexed_build_flags())) + _resolve("-DVTABLES_IN_BANANA") def test_vtables_conflicting_raises() -> None: - _set_flags("-DVTABLES_IN_DRAM", "-DVTABLES_IN_IRAM") with pytest.raises(EsphomeError, match="Conflicting VTABLES_IN_"): - _resolve_build_config(_flag_defines(set(), arduino8266._lexed_build_flags())) + _resolve("-DVTABLES_IN_DRAM", "-DVTABLES_IN_IRAM") def test_project_flags_empty_lib_flags_warn( @@ -653,26 +643,21 @@ def test_generate_ld_scripts_surfaces_preprocessor_warnings( def test_build_config_mmu_knob_with_raw_mmu_flag_raises() -> None: """A variant knob plus a raw MMU_* define would split the compile line from the linker script; refuse like the no-knob case.""" - _set_flags("-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48", "-DMMU_IRAM_SIZE=0x4000") with pytest.raises(EsphomeError, match="MMU_IRAM_SIZE conflict with .*CACHE16"): - _resolve_build_config(_flag_defines(set(), arduino8266._lexed_build_flags())) + _resolve("-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48", "-DMMU_IRAM_SIZE=0x4000") def test_build_config_raw_lwip_define_raises() -> None: """TCP_MSS/LWIP_* belong to the lwIP knobs: a raw value would win the compile line while the prebuilt library stays the knob's.""" - _set_flags("-DTCP_MSS=1024") with pytest.raises(EsphomeError, match="TCP_MSS are set by the .*LWIP2"): - _resolve_build_config(_flag_defines(set(), arduino8266._lexed_build_flags())) + _resolve("-DTCP_MSS=1024") def test_build_config_mmu_defines_do_not_alias_the_table() -> None: """The resolved list must be a copy; mutating it must not corrupt the module table for later builds in the same process.""" - _set_flags("-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48") - config = _resolve_build_config( - _flag_defines(set(), arduino8266._lexed_build_flags()) - ) + config = _resolve("-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48") config.mmu_defines.append("MMU_BOGUS") again = _resolve_build_config( _flag_defines(set(), arduino8266._lexed_build_flags()) @@ -760,14 +745,13 @@ def test_generate_ld_scripts_surgery_failure_is_named(tmp_path: Path) -> None: def test_build_config_mmu_conflict_names_the_variant_knob_with_custom() -> None: """With MMU_CUSTOM also set, the actionable fix is dropping the variant knob, not setting the knob the user already set.""" - _set_flags( - "-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48", - "-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM", - "-DMMU_IRAM_SIZE=0xC000", - "-DMMU_ICACHE_SIZE=0x4000", - ) with pytest.raises(EsphomeError, match="drop PIO_FRAMEWORK_ARDUINO_MMU_CACHE16"): - _resolve_build_config(_flag_defines(set(), arduino8266._lexed_build_flags())) + _resolve( + "-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48", + "-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM", + "-DMMU_IRAM_SIZE=0xC000", + "-DMMU_ICACHE_SIZE=0x4000", + ) def test_generate_ld_scripts_testing_surgery_failure_is_named(