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/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 9739058d41..7fe68d46bd 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -15,7 +15,7 @@ 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 @@ -23,6 +23,7 @@ import subprocess import sys from typing import TYPE_CHECKING +from esphome.arduino8266.framework import toolchain_tool from esphome.build_helpers.ninja import ( escape as _e, quote_path as _q, @@ -41,9 +42,11 @@ from esphome.components.esp8266.const import ( KEY_FLASH_SIZE, KEY_SCANF_FLOAT, ) -from esphome.const import KEY_CORE, KEY_FRAMEWORK_VERSION from esphome.core import CORE, EsphomeError -from esphome.framework_helpers import get_project_cxx_compile_flags +from esphome.framework_helpers import ( + get_project_cxx_compile_flags, + strip_win_long_path_prefix, +) from esphome.helpers import mkdir_p, write_file_if_changed from esphome.platformio.library import SOURCE_KIND_FOR_SUFFIX, lex_build_flags @@ -54,10 +57,8 @@ _LOGGER = logging.getLogger(__name__) # Compile rule per source suffix, derived from the shared suffix -> kind map # so every source extension a library manifest can select has a rule. -_RULE_FOR_KIND = {"c": "cc", "cxx": "cxx", "asm": "asm"} -_RULE_FOR_SUFFIX = { - suffix: _RULE_FOR_KIND[kind] for suffix, kind in SOURCE_KIND_FOR_SUFFIX.items() -} +# Compile rule names are exactly the shared suffix -> kind values (c, cxx, +# asm), so every source extension a library manifest can select has a rule. # Always excluded from the core build: ESPHome uses its own native OTA # backend, so the Arduino Updater (and its 228-byte global) never links. @@ -204,8 +205,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]: @@ -475,7 +476,7 @@ def _collect_sources(root: Path, exclude: set[str] = frozenset()) -> list[Path]: return sorted( p for p in root.rglob("*") - if p.suffix in _RULE_FOR_SUFFIX and p.name not in exclude + if p.suffix in SOURCE_KIND_FOR_SUFFIX and p.name not in exclude ) @@ -511,7 +512,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) @@ -644,7 +645,9 @@ def _ninja_compile_edges( rel = src.relative_to(root).as_posix() obj = f"obj/{group}/{rel}.o" escaped_obj = _e(obj) - lines.append(f"build {escaped_obj}: {_RULE_FOR_SUFFIX[src.suffix]} {_e(src)}") + lines.append( + f"build {escaped_obj}: {SOURCE_KIND_FOR_SUFFIX[src.suffix]} {_e(src)}" + ) if flags: lines.append(f" flags = {flags}") # Escaped once here: the returned paths only ever appear in build @@ -657,14 +660,15 @@ def _common_parent(paths: list[Path]) -> Path: return Path(os.path.commonpath([str(p.parent) for p in paths])) -def write_project(paths: InstalledPaths) -> bool: +def write_project(paths: InstalledPaths, ccache: str | None) -> bool: """Write the ninja build for the current configuration. - Returns True when ``build.ninja`` changed, so the caller can skip work - derived purely from it (the compile database) on unchanged builds. + ``ccache`` is the caller's already-resolved binary (None when disabled) + so one build never pays the runnability probe per consumer. Returns + True when ``build.ninja`` changed, so the caller can skip work derived + purely from it (the compile database) on unchanged builds. """ from esphome.arduino.library import resolve_libraries - from esphome.arduino8266.framework import ccache_path framework = paths.framework toolchain_bin = paths.toolchain / "bin" @@ -678,9 +682,10 @@ def write_project(paths: InstalledPaths) -> bool: config = _resolve_build_config(flag_defines) esp8266_data = CORE.data[KEY_ESP8266] board = esp8266_data[KEY_BOARD] - # Config validation accepts the board as a bare string, so this is the - # first place an unknown board can fail by name instead of a KeyError - if board not in BOARDS or board not in ESP8266_BOARD_BUILD: + # _validate_native_toolchain rejects unknown boards at config time and a + # test pins the two board tables equal; this backstop covers callers + # that bypassed validation + if board not in ESP8266_BOARD_BUILD: raise EsphomeError(f"Board '{board}' is not supported by the native toolchain") board_build = ESP8266_BOARD_BUILD[board] flash_ld_name = _flash_ld_name(board) @@ -800,7 +805,6 @@ def write_project(paths: InstalledPaths) -> bool: ) build_tool = Path(__file__).parent / "build_tool.py" - ccache = ccache_path() # $in/$out stay unquoted in the rule commands: ninja shell-escapes its # built-in path variables itself when expanding a command (POSIX and @@ -809,13 +813,16 @@ def write_project(paths: InstalledPaths) -> bool: lines = [ "# Auto-generated by ESPHome", "ninja_required_version = 1.5", - f"cc = {_q(toolchain_bin / 'xtensa-lx106-elf-gcc')}", - f"cxx = {_q(toolchain_bin / 'xtensa-lx106-elf-g++')}", - f"python = {_q(sys.executable)}", + f"cc = {_q(toolchain_tool(paths.toolchain, 'gcc'))}", + f"cxx = {_q(toolchain_tool(paths.toolchain, 'g++'))}", + # The NSIS launcher starts Python with a \\?\ extended-length path + # that cmd.exe cannot spawn; same strip every other emitted binary + # path gets + f"python = {_q(strip_win_long_path_prefix(sys.executable))}", f"buildtool = {_q(build_tool)}", f"ccache = {_q(ccache) if ccache else ''}", "", - "rule cc", + "rule c", " command = $ccache $cc -MMD -MF $out.d $cflags $flags -c $in -o $out", " depfile = $out.d", " deps = gcc", @@ -831,7 +838,7 @@ def write_project(paths: InstalledPaths) -> bool: " deps = gcc", " description = AS $out", "rule ar", - f" command = $python $buildtool ar {_q(toolchain_bin / 'xtensa-lx106-elf-ar')} $out $out.rsp", + f" command = $python $buildtool ar {_q(toolchain_tool(paths.toolchain, 'ar'))} $out $out.rsp", " rspfile = $out.rsp", " rspfile_content = $in_newline", " description = AR $out", @@ -937,18 +944,16 @@ def write_project(paths: InstalledPaths) -> bool: return write_file_if_changed(build_dir / "build.ninja", "\n".join(lines)) -def get_flash_ld_path(build_dir: Path) -> Path: - """The flash linker script the link actually uses (for size reporting).""" - from esphome.arduino8266.framework import ( - framework_package_version, - get_framework_path, - ) +def get_flash_ld_path(build_dir: Path, paths: InstalledPaths) -> Path: + """The flash linker script the link actually uses (for size reporting). + Reads the same install the ninja file linked against instead of + re-resolving the framework version. + """ name = _active_flash_ld_name(_flash_ld_name(CORE.data[KEY_ESP8266][KEY_BOARD])) if CORE.testing_mode: return build_dir / "ld" / name - version = framework_package_version(CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION]) - return get_framework_path(version) / "tools" / "sdk" / "ld" / name + return paths.framework / "tools" / "sdk" / "ld" / name def _flash_size_str(flash_size: int) -> str: diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index f3bce376d0..6f099214f4 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -69,19 +69,15 @@ def _shq(tok: str) -> str: return f'"{tok}"' if os.name == "nt" else f"'{tok}'" -def test_rule_map_covers_all_source_suffixes() -> None: - """Every suffix a library manifest can select must map to a ninja rule.""" - from esphome.platformio.library import SRC_FILE_EXTENSIONS - - assert set(arduino8266._RULE_FOR_SUFFIX) == set(SRC_FILE_EXTENSIONS) +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 @@ -99,10 +95,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 @@ -128,9 +121,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", @@ -222,9 +214,8 @@ def _write_ninja( "esphome.arduino.library.resolve_libraries", return_value=libraries or [], ), - patch("esphome.arduino8266.framework.ccache_path", return_value=ccache), ): - arduino8266.write_project(paths) + arduino8266.write_project(paths, ccache) return (CORE.relative_pioenvs_path(CORE.name) / "build.ninja").read_text() @@ -300,7 +291,7 @@ def test_write_project_link_line_and_exclusions(tmp_path: Path) -> None: assert "core_esp8266_main.cpp.o" in content # Assembly and C sources compile through their own rules assert "cont.S.o: asm" in content - assert "abi.c.o: cc" in content + assert "abi.c.o: c" in content # throw_stubs is force-included for ESPHome sources only, via one shared # srcflags variable rather than a copy of the flags line per edge src_lines = [line for line in content.splitlines() if "obj/src/" in line] @@ -345,10 +336,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 @@ -394,10 +382,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 @@ -554,25 +539,21 @@ def test_write_project_libraries_and_variant( def test_get_flash_ld_path(tmp_path: Path) -> None: + paths = InstalledPaths( + framework=tmp_path / "framework", + toolchain=tmp_path / "toolchain", + ninja=Path("ninja"), + ) CORE.testing_mode = True - assert get_flash_ld_path(tmp_path) == ( + assert get_flash_ld_path(tmp_path, paths) == ( tmp_path / "ld" / "testing_eagle.flash.4m.ld" ) CORE.testing_mode = False - with ( - patch( - "esphome.arduino8266.framework.get_framework_path", - return_value=tmp_path / "framework", - ), - patch( - "esphome.arduino8266.framework.framework_package_version", - return_value="3.30102.0", - ), - ): - assert get_flash_ld_path(tmp_path) == ( - tmp_path / "framework" / "tools" / "sdk" / "ld" / "eagle.flash.4m.ld" - ) + # Reads the same install the ninja file linked against; no re-resolve + assert get_flash_ld_path(tmp_path, paths) == ( + tmp_path / "framework" / "tools" / "sdk" / "ld" / "eagle.flash.4m.ld" + ) def test_flash_size_str() -> None: @@ -763,8 +744,8 @@ def test_write_project_returns_changed(tmp_path: Path) -> None: patch("esphome.arduino.library.resolve_libraries", return_value=[]), patch("esphome.arduino8266.framework.ccache_path", return_value=None), ): - assert arduino8266.write_project(paths) is True - assert arduino8266.write_project(paths) is False + assert arduino8266.write_project(paths, None) is True + assert arduino8266.write_project(paths, None) is False def test_write_project_missing_elf2bin_raises(tmp_path: Path) -> None: @@ -781,7 +762,7 @@ def test_write_project_missing_elf2bin_raises(tmp_path: Path) -> None: patch("esphome.arduino.library.resolve_libraries", return_value=[]), pytest.raises(EsphomeError, match="elf2bin"), ): - arduino8266.write_project(paths) + arduino8266.write_project(paths, None) def test_write_project_missing_src_dir_raises(tmp_path: Path) -> None: @@ -796,7 +777,7 @@ def test_write_project_missing_src_dir_raises(tmp_path: Path) -> None: ), pytest.raises(EsphomeError, match="source directory"), ): - arduino8266.write_project(paths) + arduino8266.write_project(paths, None) def test_build_config_custom_mmu_without_knob_raises() -> None: @@ -804,9 +785,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: @@ -972,15 +952,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( @@ -1023,26 +1001,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()) @@ -1155,14 +1128,13 @@ def test_write_project_lexes_build_flags_once( 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( 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") 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("")