From 0c48f749c21706763327c5b8bc91f6caeffa2b20 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 01:45:39 -0500 Subject: [PATCH] Fix testing-mode segment requirements per linker script; simplify downloads and native command dispatch --- esphome/__main__.py | 50 +++++-------- esphome/arduino8266/component.py | 10 +-- esphome/arduino8266/framework.py | 73 ++++++++++--------- esphome/arduino8266/toolchain.py | 1 - esphome/build_gen/arduino8266.py | 5 +- esphome/components/esp8266/build_surgery.py | 22 +++--- .../unit_tests/build_gen/test_arduino8266.py | 7 ++ .../components/esp8266/test_build_surgery.py | 14 ++-- .../unit_tests/test_arduino8266_component.py | 4 +- .../unit_tests/test_arduino8266_framework.py | 11 ++- .../unit_tests/test_arduino8266_toolchain.py | 4 +- 11 files changed, 97 insertions(+), 104 deletions(-) diff --git a/esphome/__main__.py b/esphome/__main__.py index 6eae4b7bc8..f104eace8a 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -813,7 +813,7 @@ def write_cpp_file() -> int: from esphome.build_gen import espidf espidf.write_project() - elif CORE.is_esp8266 and CORE.using_toolchain_arduino: + elif CORE.using_toolchain_arduino and CORE.is_esp8266: # The ninja project is generated at compile time by # esphome.arduino8266.toolchain (it needs the downloaded framework). pass @@ -968,7 +968,7 @@ def upload_using_esptool( flash_images = [ FlashImage(path=toolchain.get_factory_firmware_path(), offset="0x0") ] - elif CORE.is_esp8266 and CORE.using_toolchain_arduino: + elif CORE.using_toolchain_arduino and CORE.is_esp8266: # The native backend writes PlatformIO-compatible output paths, so the # shared property already points at the right file. flash_images = [FlashImage(path=CORE.firmware_bin, offset="0x0")] @@ -1920,27 +1920,16 @@ def command_update_all(args: ArgsProtocol) -> int | None: def command_idedata(args: ArgsProtocol, config: ConfigType) -> int: import json + native_toolchain = None if CORE.using_toolchain_esp_idf: - # Native ESP-IDF derives idedata from the build's compile_commands.json, - # so the configuration must already be compiled. - from esphome.espidf import toolchain as espidf_toolchain + from esphome.espidf import toolchain as native_toolchain + elif CORE.using_toolchain_arduino and CORE.is_esp8266: + from esphome.arduino8266 import toolchain as native_toolchain - idedata = espidf_toolchain.get_idedata() - if idedata is None: - _LOGGER.error( - "No idedata available; compile the configuration first", - ) - return 1 - - print(json.dumps(idedata, indent=2) + "\n") - return 0 - - if CORE.is_esp8266 and CORE.using_toolchain_arduino: - # Same contract as the ESP-IDF branch: idedata is derived from the - # build's compile_commands.json, so a compile must have run. - from esphome.arduino8266 import toolchain as arduino8266_toolchain - - idedata = arduino8266_toolchain.get_idedata() + if native_toolchain is not None: + # Native toolchains derive idedata from the build's + # compile_commands.json, so the configuration must already be compiled. + idedata = native_toolchain.get_idedata() if idedata is None: _LOGGER.error( "No idedata available; compile the configuration first", @@ -1990,20 +1979,17 @@ def command_analyze_memory(args: ArgsProtocol, config: ConfigType) -> int: # Get idedata for analysis idedata = None + native_toolchain = None if CORE.using_toolchain_esp_idf: - from esphome.espidf import toolchain + from esphome.espidf import toolchain as native_toolchain + elif CORE.using_toolchain_arduino and CORE.is_esp8266: + from esphome.arduino8266 import toolchain as native_toolchain - objdump_path = str(toolchain.get_objdump_path()) - readelf_path = str(toolchain.get_readelf_path()) + if native_toolchain is not None: + objdump_path = str(native_toolchain.get_objdump_path()) + readelf_path = str(native_toolchain.get_readelf_path()) - firmware_elf = toolchain.get_elf_path() - elif CORE.is_esp8266 and CORE.using_toolchain_arduino: - from esphome.arduino8266 import toolchain - - objdump_path = str(toolchain.get_objdump_path()) - readelf_path = str(toolchain.get_readelf_path()) - - firmware_elf = toolchain.get_elf_path() + firmware_elf = native_toolchain.get_elf_path() else: from esphome.platformio import toolchain diff --git a/esphome/arduino8266/component.py b/esphome/arduino8266/component.py index f5c5c2d3ca..59d8746769 100644 --- a/esphome/arduino8266/component.py +++ b/esphome/arduino8266/component.py @@ -39,12 +39,6 @@ _LOGGER = logging.getLogger(__name__) ESP8266_PLATFORM = "espressif8266" -# Bare names components register that intentionally have no bundled library -# ("Updater" is replaced by ESPHome's native OTA backend). Anything else -# missing from the framework tree is worth a warning: it is likely a typo or -# a library the build genuinely needs. -_KNOWN_ABSENT_BUNDLED = frozenset({"Updater"}) - @dataclass class ArduinoLibrary: @@ -139,9 +133,9 @@ def resolve_libraries(framework_path: Path) -> list[ArduinoLibrary]: external.append(library) elif (framework_path / "libraries" / library.name).is_dir(): bundled.append(_bundled_library(framework_path, library.name)) - elif library.name in _KNOWN_ABSENT_BUNDLED: - _LOGGER.debug("Skipping known-absent bundled library %s", library.name) else: + # Likely a typo or a library the build genuinely needs; a debug + # log here would surface only as a wall of include errors later. _LOGGER.warning( "Library %s is not bundled with the Arduino framework and has " "no version or repository to download it from; skipping", diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index 0a1447048b..b06acd9592 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -22,7 +22,6 @@ from pathlib import Path import platform import shutil import stat -import tempfile import platformdirs @@ -69,11 +68,12 @@ ESPHOME_ARDUINO8266_FRAMEWORK_MIRRORS = str_to_lst_of_str( ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS = str_to_lst_of_str( os.environ.get("ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS", "") ) +# Default ninja source; downloads from it verify against _NINJA_SHA256. +_NINJA_URL = ( + "https://github.com/ninja-build/ninja/releases/download/v{VERSION}/{ARCHIVE}" +) ESPHOME_ARDUINO8266_NINJA_MIRRORS = str_to_lst_of_str( - os.environ.get( - "ESPHOME_ARDUINO8266_NINJA_MIRRORS", - "https://github.com/ninja-build/ninja/releases/download/v{VERSION}/{ARCHIVE}", - ) + os.environ.get("ESPHOME_ARDUINO8266_NINJA_MIRRORS", "") ) @@ -110,6 +110,12 @@ def get_toolchain_path() -> Path: return get_arduino8266_tools_path() / "toolchains" / TOOLCHAIN_VERSION +def _downloads_path() -> Path: + path = get_arduino8266_tools_path() / "downloads" + path.mkdir(parents=True, exist_ok=True) + return path + + def _pio_system() -> str: """The PlatformIO registry system tag for the current host. @@ -174,20 +180,22 @@ def _install_package( if marker.is_file(): return rmdir(dest, msg=f"Clean up incomplete {name} install") - with tempfile.TemporaryDirectory() as tmp_dir: - archive = Path(tmp_dir) / "package" - _LOGGER.info("Downloading %s %s ...", name, version) - if mirrors: - with archive.open("wb") as file: - download_from_mirrors( - mirrors, {"VERSION": version, "SYSTEM": _pio_system()}, file - ) - else: - url, sha256, size = _registry_download(name, version) - download_with_resume(url, archive, sha256=sha256, size=size) - _LOGGER.info("Extracting %s ...", name) - archive_extract_all(archive, dest, progress_header="Extracting") + # A persistent download location (not a temp dir) so an interrupted + # download resumes across esphome runs via download_with_resume's .part + # file, mirroring the espidf dist/ convention. + archive = _downloads_path() / f"{name}-{version}" + _LOGGER.info("Downloading %s %s ...", name, version) + if mirrors: + download_from_mirrors( + mirrors, {"VERSION": version, "SYSTEM": _pio_system()}, archive + ) + else: + url, sha256, size = _registry_download(name, version) + download_with_resume(url, archive, sha256=sha256, size=size) + _LOGGER.info("Extracting %s ...", name) + archive_extract_all(archive, dest, progress_header="Extracting") marker.touch() + archive.unlink(missing_ok=True) def _ninja_archive_name() -> str: @@ -212,22 +220,19 @@ def _check_ninja_install() -> Path: return binary rmdir(ninja_dir, msg="Clean up incomplete ninja install") archive_name = _ninja_archive_name() - with tempfile.TemporaryDirectory() as tmp_dir: - archive = Path(tmp_dir) / archive_name - _LOGGER.info("Downloading ninja %s ...", NINJA_VERSION) - if "ESPHOME_ARDUINO8266_NINJA_MIRRORS" in os.environ: - with archive.open("wb") as file: - download_from_mirrors( - ESPHOME_ARDUINO8266_NINJA_MIRRORS, - {"VERSION": NINJA_VERSION, "ARCHIVE": archive_name}, - file, - ) - else: - url = ESPHOME_ARDUINO8266_NINJA_MIRRORS[0].format( - VERSION=NINJA_VERSION, ARCHIVE=archive_name - ) - download_with_resume(url, archive, sha256=_NINJA_SHA256[archive_name]) - archive_extract_all(archive, ninja_dir) + archive = _downloads_path() / archive_name + _LOGGER.info("Downloading ninja %s ...", NINJA_VERSION) + if ESPHOME_ARDUINO8266_NINJA_MIRRORS: + download_from_mirrors( + ESPHOME_ARDUINO8266_NINJA_MIRRORS, + {"VERSION": NINJA_VERSION, "ARCHIVE": archive_name}, + archive, + ) + else: + url = _NINJA_URL.format(VERSION=NINJA_VERSION, ARCHIVE=archive_name) + download_with_resume(url, archive, sha256=_NINJA_SHA256[archive_name]) + archive_extract_all(archive, ninja_dir) + archive.unlink(missing_ok=True) if not binary.is_file(): raise EsphomeError(f"ninja binary missing after extraction in {ninja_dir}") binary.chmod(binary.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) diff --git a/esphome/arduino8266/toolchain.py b/esphome/arduino8266/toolchain.py index 5fe297d493..2896a3aba3 100644 --- a/esphome/arduino8266/toolchain.py +++ b/esphome/arduino8266/toolchain.py @@ -146,7 +146,6 @@ def _print_size_summary(build_dir: Path, toolchain_path: Path) -> None: except ValueError: # An omitted section would silently skew the reported totals _LOGGER.warning("Unparsable size output for section %s", parts[0]) - continue ram = sum(sections.get(s, 0) for s in _RAM_SECTIONS) flash = sum(sections.get(s, 0) for s in _FLASH_SECTIONS) print(f"RAM: {format_bar(ram, _MAX_RAM_SIZE)}") diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 1fdcf8943a..b6ed132cf5 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -355,7 +355,7 @@ def generate_ld_scripts( raise EsphomeError(f"Generating the linker script failed:\n{result.stderr}") content = relocate_ratetable(result.stdout) if CORE.testing_mode: - content = apply_testing_memory_patches(content) + content = apply_testing_memory_patches(content, require=("iram1_0_seg",)) write_file_if_changed(output, content) stamp.write_text(stamp_content, encoding="utf-8") @@ -366,7 +366,8 @@ def generate_ld_scripts( write_file_if_changed( ld_dir / f"testing_{flash_ld_name}", apply_testing_memory_patches( - flash_ld.read_text(encoding="utf-8"), require=True + flash_ld.read_text(encoding="utf-8"), + require=("dram0_0_seg", "irom0_0_seg"), ), ) diff --git a/esphome/components/esp8266/build_surgery.py b/esphome/components/esp8266/build_surgery.py index fd4d6e3db1..d8ca64f669 100644 --- a/esphome/components/esp8266/build_surgery.py +++ b/esphome/components/esp8266/build_surgery.py @@ -9,6 +9,7 @@ as plain functions. Keep both in sync when changing either. from __future__ import annotations +from collections.abc import Collection import re # Move the NONOS SDK wifi rate tables from flash to DRAM; see @@ -59,23 +60,24 @@ def _patch_segment_size(content: str, segment_name: str, new_size: str) -> str: return re.sub(pattern, rf"\g<1>{new_size}", content) -def apply_testing_memory_patches(content: str, require: bool = False) -> str: +def apply_testing_memory_patches(content: str, require: Collection[str] = ()) -> str: """Enlarge IRAM/DRAM/flash segments so grouped CI test builds can link. - With ``require``, raise when a segment was not found: a silently - unpatched flash linker script would keep the real 32KB IRAM limits and - fail grouped builds far from the cause. The common linker script has no - MEMORY block, so its caller leaves ``require`` off. + ``require`` names the segments this file must define; a silently + unpatched segment would keep the real memory limits and fail grouped + builds far from the cause. The segments are split across the two linker + scripts (iram1_0_seg in the generated common one, dram0_0_seg and + irom0_0_seg in the flash one), so each caller requires only its own. """ - missing: list[str] = [] + missing = set(require) for segment, size in _TESTING_SEGMENT_SIZES: patched = _patch_segment_size(content, segment, size) - if patched == content: - missing.append(segment) + if patched != content: + missing.discard(segment) content = patched - if require and missing: + if missing: raise RuntimeError( - f"Testing-mode memory patch failed: segment(s) {', '.join(missing)} " + f"Testing-mode memory patch failed: segment(s) {', '.join(sorted(missing))} " "not found (has the Arduino core linker script changed?)" ) return content diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index caaae9d2b7..de77624756 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -349,10 +349,17 @@ def test_build_config_waveform_locked_phase() -> None: _COMMON_LD_H_OUTPUT = """\ +MEMORY +{ + iram1_0_seg : org = 0x40100000, len = 0x8000 +} +SECTIONS +{ .data : ALIGN(4) { _data_start = ABSOLUTE(.); } >dram0_0_seg :dram0_0_phdr +} """ diff --git a/tests/unit_tests/components/esp8266/test_build_surgery.py b/tests/unit_tests/components/esp8266/test_build_surgery.py index 7e56d9c133..9e4640418d 100644 --- a/tests/unit_tests/components/esp8266/test_build_surgery.py +++ b/tests/unit_tests/components/esp8266/test_build_surgery.py @@ -72,11 +72,15 @@ def test_segment_length() -> None: def test_testing_memory_patches_require() -> None: - """With require, a segment the patch could not find raises instead of + """A required segment the patch could not find raises instead of silently keeping the real memory limits.""" - patched = apply_testing_memory_patches(_FLASH_LD_SNIPPET, require=True) + patched = apply_testing_memory_patches( + _FLASH_LD_SNIPPET, require=("dram0_0_seg", "irom0_0_seg") + ) assert "0x2000000" in patched - with pytest.raises(RuntimeError, match="iram1_0_seg"): - apply_testing_memory_patches("MEMORY { }", require=True) - # Without require (the common linker script has no MEMORY block) it is a no-op + with pytest.raises(RuntimeError, match="dram0_0_seg, irom0_0_seg"): + apply_testing_memory_patches( + "MEMORY { }", require=("dram0_0_seg", "irom0_0_seg") + ) + # Segments a file does not require are patched opportunistically only assert apply_testing_memory_patches("MEMORY { }") == "MEMORY { }" diff --git a/tests/unit_tests/test_arduino8266_component.py b/tests/unit_tests/test_arduino8266_component.py index 74de50008b..e18c435fc8 100644 --- a/tests/unit_tests/test_arduino8266_component.py +++ b/tests/unit_tests/test_arduino8266_component.py @@ -98,12 +98,10 @@ def test_resolve_libraries_bundled_and_unknown( ) -> None: framework = _make_framework(tmp_path) _add_library("ESP8266WiFi", None) - _add_library("Updater", None) # known-absent: skipped silently - _add_library("Typoo", None) # unknown: skipped with a warning + _add_library("Typoo", None) # unknown bare name: skipped with a warning libs = component.resolve_libraries(framework) assert [lib.name for lib in libs] == ["ESP8266WiFi"] assert "Typoo" in caplog.text - assert "Updater" not in caplog.text def _converted(name: str, source_dir: Path, data: dict) -> ConvertedLibrary: diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index 4e206a1f62..a0ba28e34f 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -230,12 +230,11 @@ def test_check_ninja_install_mirror_override_skips_checksum(tmp_path: Path) -> N """A mirror override is trusted as configured (no pinned checksum).""" with ( patch("shutil.which", return_value=None), - patch.dict( - os.environ, - { - "ESPHOME_ARDUINO8266_PREFIX": str(tmp_path), - "ESPHOME_ARDUINO8266_NINJA_MIRRORS": "http://mirror/{ARCHIVE}", - }, + patch.dict(os.environ, {"ESPHOME_ARDUINO8266_PREFIX": str(tmp_path)}), + patch.object( + framework, + "ESPHOME_ARDUINO8266_NINJA_MIRRORS", + ["http://mirror/{ARCHIVE}"], ), patch.object(framework, "download_from_mirrors") as mock_download, patch.object(framework, "archive_extract_all", side_effect=_fake_ninja_extract), diff --git a/tests/unit_tests/test_arduino8266_toolchain.py b/tests/unit_tests/test_arduino8266_toolchain.py index 56378d0f1d..ad2211fdf4 100644 --- a/tests/unit_tests/test_arduino8266_toolchain.py +++ b/tests/unit_tests/test_arduino8266_toolchain.py @@ -15,7 +15,7 @@ from esphome.const import ( KEY_CORE, KEY_FRAMEWORK_VERSION, ) -from esphome.core import CORE +from esphome.core import CORE, EsphomeError _SIZE_OUTPUT = """\ firmware.elf : @@ -107,8 +107,6 @@ def test_write_compile_commands(tmp_path: Path) -> None: def test_write_compile_commands_failure_removes_stale_db(tmp_path: Path) -> None: """A failed compdb run must not leave a stale database behind.""" - from esphome.core import EsphomeError - stale = tmp_path / "compile_commands.json" stale.write_text("[]") with (