Apply review cleanups to the native ESP8266 toolchain

This commit is contained in:
J. Nick Koston
2026-08-19 23:13:30 -05:00
parent 8811224c0e
commit 3fe9cac1fc
12 changed files with 164 additions and 100 deletions
+2 -2
View File
@@ -93,9 +93,9 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary:
lib.include_dirs.append(path.resolve())
lib.sources = sorted(
Path(f).resolve()
path.resolve()
for f in collect_filtered_files(read_path / src_dir, src_filter)
if Path(f).suffix in SRC_FILE_EXTENSIONS
if (path := Path(f)).suffix in SRC_FILE_EXTENSIONS
)
return lib
+7 -8
View File
@@ -4,7 +4,6 @@ from __future__ import annotations
import logging
from pathlib import Path
import re
import subprocess
from esphome.arduino8266 import framework
@@ -43,7 +42,7 @@ def run_compile(config: ConfigType, verbose: bool) -> int:
from esphome.build_gen import arduino8266 as build_gen
paths = framework.check_and_install(CORE.data[KEY_CORE][KEY_FRAMEWORK_VERSION])
build_gen.write_project(paths)
ninja_changed = build_gen.write_project(paths)
build_dir = get_build_dir()
env = framework.get_build_env(paths["toolchain_path"])
@@ -58,7 +57,10 @@ def run_compile(config: ConfigType, verbose: bool) -> int:
if rc != 0:
return rc
_write_compile_commands(paths["ninja_path"], build_dir, env)
# The compile database is a pure function of build.ninja; skip its
# regeneration (a ninja spawn plus MBs of text) on unchanged builds.
if ninja_changed or not (build_dir / "compile_commands.json").is_file():
_write_compile_commands(paths["ninja_path"], build_dir, env)
_print_size_summary(build_dir, paths["toolchain_path"])
get_idedata()
return 0
@@ -86,16 +88,13 @@ def _write_compile_commands(
def _parse_app_size(build_dir: Path) -> int | None:
"""Read the app flash budget (irom0_0_seg length) from the linker script."""
from esphome.build_gen.arduino8266 import get_flash_ld_path
from esphome.components.esp8266.build_surgery import segment_length
appsize_re = re.compile(r"irom0_0_seg\s*:.+len\s*=\s*(0x[\da-f]+)", re.IGNORECASE)
try:
ld_text = get_flash_ld_path(build_dir).read_text(encoding="utf-8")
except OSError:
return None
for line in ld_text.splitlines():
if match := appsize_re.search(line):
return int(match.group(1), 16)
return None
return segment_length(ld_text, "irom0_0_seg")
def _print_size_summary(build_dir: Path, toolchain_path: Path) -> None:
+7 -3
View File
@@ -373,8 +373,12 @@ def _common_parent(paths: list[Path]) -> Path:
return Path(os.path.commonpath([str(p.parent) for p in paths]))
def write_project(paths: dict[str, Path]) -> None:
"""Write the ninja build for the current configuration."""
def write_project(paths: dict[str, Path]) -> 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.
"""
from esphome.arduino8266.component import resolve_libraries
from esphome.arduino8266.framework import ccache_path
@@ -551,7 +555,7 @@ def write_project(paths: dict[str, Path]) -> None:
lines.append("default firmware.factory.bin firmware.ota.bin")
lines.append("")
write_file_if_changed(build_dir / "build.ninja", "\n".join(lines))
return write_file_if_changed(build_dir / "build.ninja", "\n".join(lines))
def get_flash_ld_path(build_dir: Path) -> Path:
+19 -1
View File
@@ -361,12 +361,30 @@ BOARDS = {
},
}
"""
ESP8266_BOARD_BUILD generate with:
git clone https://github.com/platformio/platform-espressif8266
python3 - <<'EOF'
import json, glob, os
for f in sorted(glob.glob("platform-espressif8266/boards/*.json")):
b = json.load(open(f))["build"]
extra = b["extra_flags"]
extra = extra.split() if isinstance(extra, str) else extra
defines = [
e[2:] for e in extra if e not in ("-DESP8266", "-DARDUINO_ARCH_ESP8266")
]
entries = ", ".join(f'"{d}"' for d in defines) + ("," if len(defines) == 1 else "")
board = os.path.splitext(os.path.basename(f))[0]
print(f' "{board}": {{"variant": "{b["variant"]}", "defines": ({entries})}},')
EOF
"""
# Per-board Arduino core build metadata for the native (PlatformIO-free)
# toolchain: the variant directory (supplies pins_arduino.h) and the
# board-identity defines the PlatformIO builder passes via build.extra_flags.
# -DESP8266 and -DARDUINO_ARCH_ESP8266 are shared by every board and added by
# the generator; only the per-board defines are listed here.
# Generated from platform-espressif8266 boards/*.json (see BOARDS note above).
ESP8266_BOARD_BUILD = {
"agruminolemon": {
"variant": "agruminolemonv4",
@@ -57,3 +57,12 @@ def apply_testing_memory_patches(content: str) -> str:
content = _patch_segment_size(content, "iram1_0_seg", TESTING_IRAM_SIZE)
content = _patch_segment_size(content, "dram0_0_seg", TESTING_DRAM_SIZE)
return _patch_segment_size(content, "irom0_0_seg", TESTING_FLASH_SIZE)
def segment_length(content: str, segment_name: str) -> int | None:
"""Read a memory segment's length from linker script content."""
match = re.search(
rf"{segment_name}\s*:.+len\s*=\s*(0x[\da-fA-F]+)",
content,
)
return int(match.group(1), 16) if match else None
+33 -32
View File
@@ -513,16 +513,31 @@ ESP32_PLATFORMIO_TRIGGER_FILES = frozenset(
)
def _path_or_file_trigger(
files: list[str],
trigger_files: frozenset[str],
trigger_prefixes: tuple[str, ...],
) -> bool:
"""Whether any changed file matches the given infrastructure triggers."""
return any(
file in trigger_files or file.startswith(trigger_prefixes) for file in files
)
@cache
def _changed_components_closure(branch: str | None) -> frozenset[str]:
"""Dependency closure of the changed components (shared by the
per-toolchain narrowing functions and cached per branch)."""
files = changed_files(branch)
component_files = [f for f in files if filter_component_and_test_files(f)]
return frozenset(get_components_with_dependencies(component_files, True))
def _esp32_platformio_path_or_file_trigger(files: list[str]) -> bool:
"""Whether any changed file is a PlatformIO infrastructure / harness trigger."""
for file in files:
if file in ESP32_PLATFORMIO_TRIGGER_FILES:
return True
if any(
file.startswith(prefix) for prefix in ESP32_PLATFORMIO_TRIGGER_PATH_PREFIXES
):
return True
return False
return _path_or_file_trigger(
files, ESP32_PLATFORMIO_TRIGGER_FILES, ESP32_PLATFORMIO_TRIGGER_PATH_PREFIXES
)
# ESP-IDF infra: changes under esphome/espidf/ or to the IDF build generator
@@ -536,14 +551,9 @@ ESP_IDF_INFRA_TRIGGER_FILES = frozenset({"esphome/build_gen/espidf.py"})
def _esp_idf_infra_changed(files: list[str]) -> bool:
"""Whether any changed file is ESP-IDF build/runner infrastructure."""
for file in files:
if file in ESP_IDF_INFRA_TRIGGER_FILES:
return True
if any(
file.startswith(prefix) for prefix in ESP_IDF_INFRA_TRIGGER_PATH_PREFIXES
):
return True
return False
return _path_or_file_trigger(
files, ESP_IDF_INFRA_TRIGGER_FILES, ESP_IDF_INFRA_TRIGGER_PATH_PREFIXES
)
def esp32_platformio_components_to_test(branch: str | None = None) -> list[str]:
@@ -586,10 +596,9 @@ def esp32_platformio_components_to_test(branch: str | None = None) -> list[str]:
if core_changed(files) or _esp32_platformio_path_or_file_trigger(files):
return sorted(ESP32_PLATFORMIO_TEST_COMPONENTS)
component_files = [f for f in files if filter_component_and_test_files(f)]
changed = get_components_with_dependencies(component_files, True)
return sorted(ESP32_PLATFORMIO_TEST_COMPONENTS & set(changed))
return sorted(
ESP32_PLATFORMIO_TEST_COMPONENTS & _changed_components_closure(branch)
)
def should_run_esp32_platformio(branch: str | None = None) -> bool:
@@ -645,14 +654,9 @@ ESP8266_NATIVE_TRIGGER_FILES = frozenset(
def _esp8266_native_path_or_file_trigger(files: list[str]) -> bool:
"""Whether any changed file is native-ESP8266 infrastructure / harness."""
for file in files:
if file in ESP8266_NATIVE_TRIGGER_FILES:
return True
if any(
file.startswith(prefix) for prefix in ESP8266_NATIVE_TRIGGER_PATH_PREFIXES
):
return True
return False
return _path_or_file_trigger(
files, ESP8266_NATIVE_TRIGGER_FILES, ESP8266_NATIVE_TRIGGER_PATH_PREFIXES
)
def esp8266_native_components_to_test(branch: str | None = None) -> list[str]:
@@ -667,10 +671,7 @@ def esp8266_native_components_to_test(branch: str | None = None) -> list[str]:
if core_changed(files) or _esp8266_native_path_or_file_trigger(files):
return sorted(ESP8266_NATIVE_TEST_COMPONENTS)
component_files = [f for f in files if filter_component_and_test_files(f)]
changed = get_components_with_dependencies(component_files, True)
return sorted(ESP8266_NATIVE_TEST_COMPONENTS & set(changed))
return sorted(ESP8266_NATIVE_TEST_COMPONENTS & _changed_components_closure(branch))
def determine_cpp_unit_tests(
+25 -41
View File
@@ -16,7 +16,16 @@ from unittest.mock import MagicMock, patch
import pytest
from esphome.build_gen import arduino8266
from esphome.build_gen.arduino8266 import (
_defines_flags,
_flag_defines,
_flash_size_str,
_resolve_build_config,
get_flash_ld_path,
)
from esphome.components.esp8266.boards import BOARDS, ESP8266_BOARD_BUILD
from esphome.components.esp8266.build_surgery import RATETABLE_RULE
from esphome.components.esp8266.const import (
KEY_BOARD,
KEY_ESP8266,
@@ -55,7 +64,6 @@ def test_board_build_covers_every_board() -> None:
def test_build_config_defaults() -> None:
from esphome.build_gen.arduino8266 import _flag_defines, _resolve_build_config
_set_flags()
config = _resolve_build_config(_flag_defines())
@@ -75,7 +83,6 @@ def test_build_config_defaults() -> None:
def test_build_config_esphome_lwip_knob() -> None:
"""The lwIP variant ESPHome selects maps to the same defines and library
as the PlatformIO builder."""
from esphome.build_gen.arduino8266 import _flag_defines, _resolve_build_config
_set_flags("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH")
config = _resolve_build_config(_flag_defines())
@@ -86,7 +93,6 @@ def test_build_config_esphome_lwip_knob() -> None:
def test_build_config_knobs() -> None:
from esphome.build_gen.arduino8266 import _flag_defines, _resolve_build_config
_set_flags(
"-DPIO_FRAMEWORK_ARDUINO_ESPRESSIF_SDK305",
@@ -102,7 +108,6 @@ def test_build_config_knobs() -> None:
def test_build_config_mmu_custom_requires_sizes() -> None:
from esphome.build_gen.arduino8266 import _flag_defines, _resolve_build_config
_set_flags("-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM")
with pytest.raises(EsphomeError, match="MMU_IRAM_SIZE"):
@@ -122,11 +127,6 @@ def test_build_config_mmu_custom_requires_sizes() -> None:
def test_defines_match_platformio_builder() -> None:
"""The exact define set the PlatformIO builder passes for nodemcuv2/dout."""
from esphome.build_gen.arduino8266 import (
_defines_flags,
_flag_defines,
_resolve_build_config,
)
_set_flags("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH")
assert _defines_flags(
@@ -183,9 +183,11 @@ def _make_framework(tmp_path: Path) -> dict[str, Path]:
}
def _write_ninja(paths: dict[str, Path]) -> str:
from esphome.build_gen import arduino8266
def _write_ninja(
paths: dict[str, Path],
libraries: list | None = None,
ccache: str | None = None,
) -> str:
src = CORE.relative_src_path()
(src / "esphome" / "components" / "esp8266").mkdir(parents=True, exist_ok=True)
(src / "main.cpp").write_text("")
@@ -193,7 +195,11 @@ def _write_ninja(paths: dict[str, Path]) -> str:
with (
patch.object(arduino8266, "generate_ld_scripts"),
patch("esphome.arduino8266.framework.ccache_path", return_value=None),
patch(
"esphome.arduino8266.component.resolve_libraries",
return_value=libraries or [],
),
patch("esphome.arduino8266.framework.ccache_path", return_value=ccache),
):
arduino8266.write_project(paths)
return (CORE.relative_pioenvs_path(CORE.name) / "build.ninja").read_text()
@@ -284,7 +290,6 @@ def test_build_config_lwip_variants(
knob: str, lib: str, mss: int, features: int, ipv6: int
) -> None:
"""Every lwIP knob maps to the same defines and library as the PIO builder."""
from esphome.build_gen.arduino8266 import _flag_defines, _resolve_build_config
_set_flags(f"-D{knob}")
config = _resolve_build_config(_flag_defines())
@@ -321,14 +326,12 @@ def test_build_config_lwip_variants(
],
)
def test_build_config_mmu_variants(knob: str, expected: list[str]) -> None:
from esphome.build_gen.arduino8266 import _flag_defines, _resolve_build_config
_set_flags(f"-D{knob}")
assert _resolve_build_config(_flag_defines()).mmu_defines == expected
def test_build_config_waveform_locked_phase() -> None:
from esphome.build_gen.arduino8266 import _flag_defines, _resolve_build_config
_set_flags("-DPIO_FRAMEWORK_ARDUINO_WAVEFORM_LOCKED_PHASE", "-DFP_IN_IROM")
config = _resolve_build_config(_flag_defines())
@@ -345,16 +348,13 @@ _COMMON_LD_H_OUTPUT = """\
def _run_generate_ld_scripts(paths: dict[str, Path]) -> Path:
from esphome.build_gen import arduino8266
config = arduino8266._resolve_build_config(arduino8266._flag_defines())
config = _resolve_build_config(_flag_defines())
arduino8266.generate_ld_scripts(paths, config, "eagle.flash.4m.ld")
return CORE.relative_pioenvs_path(CORE.name, "ld")
def test_generate_ld_scripts(tmp_path: Path) -> None:
from esphome.build_gen import arduino8266
from esphome.components.esp8266.build_surgery import RATETABLE_RULE
paths = _make_framework(tmp_path)
_set_flags("-DFP_IN_IROM")
@@ -375,7 +375,6 @@ def test_generate_ld_scripts(tmp_path: Path) -> None:
def test_generate_ld_scripts_failure(tmp_path: Path) -> None:
from esphome.build_gen import arduino8266
paths = _make_framework(tmp_path)
result = MagicMock(returncode=1, stderr="nope")
@@ -387,7 +386,6 @@ def test_generate_ld_scripts_failure(tmp_path: Path) -> None:
def test_generate_ld_scripts_testing_mode(tmp_path: Path) -> None:
from esphome.build_gen import arduino8266
paths = _make_framework(tmp_path)
(paths["framework_path"] / "tools" / "sdk" / "ld" / "eagle.flash.4m.ld").write_text(
@@ -403,7 +401,6 @@ def test_generate_ld_scripts_testing_mode(tmp_path: Path) -> None:
def test_write_project_libraries_and_variant(tmp_path: Path) -> None:
from esphome.arduino8266.component import ArduinoLibrary
from esphome.build_gen import arduino8266
paths = _make_framework(tmp_path)
variant_src = paths["framework_path"] / "variants" / "nodemcu" / "variant.cpp"
@@ -423,20 +420,9 @@ def test_write_project_libraries_and_variant(tmp_path: Path) -> None:
)
_set_flags("-DPIO_FRAMEWORK_ARDUINO_ENABLE_EXCEPTIONS")
src = CORE.relative_src_path()
(src / "esphome" / "components" / "esp8266").mkdir(parents=True, exist_ok=True)
(src / "main.cpp").write_text("")
with (
patch.object(arduino8266, "generate_ld_scripts"),
patch(
"esphome.arduino8266.component.resolve_libraries",
return_value=[library, headers_only],
),
patch("esphome.arduino8266.framework.ccache_path", return_value="/cc/ccache"),
):
arduino8266.write_project(paths)
content = (CORE.relative_pioenvs_path(CORE.name) / "build.ninja").read_text()
content = _write_ninja(
paths, libraries=[library, headers_only], ccache="/cc/ccache"
)
assert "build libFrameworkArduinoVariant.a: ar" in content
assert "build libMyLib.a: ar" in content
@@ -452,10 +438,9 @@ def test_write_project_libraries_and_variant(tmp_path: Path) -> None:
def test_get_flash_ld_path(tmp_path: Path) -> None:
from esphome.build_gen import arduino8266
CORE.testing_mode = True
assert arduino8266.get_flash_ld_path(tmp_path) == (
assert get_flash_ld_path(tmp_path) == (
tmp_path / "ld" / "testing_eagle.flash.4m.ld"
)
@@ -470,13 +455,12 @@ def test_get_flash_ld_path(tmp_path: Path) -> None:
return_value="3.30102.0",
),
):
assert arduino8266.get_flash_ld_path(tmp_path) == (
assert get_flash_ld_path(tmp_path) == (
tmp_path / "framework" / "tools" / "sdk" / "ld" / "eagle.flash.4m.ld"
)
def test_flash_size_str() -> None:
from esphome.build_gen.arduino8266 import _flash_size_str
assert _flash_size_str("eagle.flash.4m.ld") == "4M"
assert _flash_size_str("eagle.flash.512k.ld") == "512K"
@@ -62,3 +62,10 @@ def test_testing_memory_patches_enlarge_segments() -> None:
"irom0_0_seg : org = 0x40201010, len = 0x2000000"
in patched
)
def test_segment_length() -> None:
from esphome.components.esp8266.build_surgery import segment_length
assert segment_length(_FLASH_LD_SNIPPET, "irom0_0_seg") == 0xFEFF0
assert segment_length(_FLASH_LD_SNIPPET, "missing_seg") is None
@@ -245,12 +245,9 @@ def test_ccache_path_disabled_by_env() -> None:
assert framework.ccache_path() is None
def test_ccache_path_no_binary() -> None:
with (
patch.dict(os.environ, {}, clear=False),
patch("shutil.which", return_value=None),
):
os.environ.pop("ESPHOME_CCACHE_ENABLE", 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
@@ -263,12 +260,12 @@ def test_ccache_path_probe_failure() -> None:
assert framework.ccache_path() is None
def test_ccache_path_ok() -> 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"),
):
os.environ.pop("ESPHOME_CCACHE_ENABLE", None)
assert framework.ccache_path() == "/usr/bin/ccache"
@@ -184,3 +184,30 @@ def test_get_idedata_delegates(tmp_path: Path) -> None:
assert compile_commands.name == "compile_commands.json"
assert elf.name == "firmware.elf"
assert cache.name == "test8266.json"
def test_run_compile_skips_compdb_when_ninja_unchanged(tmp_path: Path) -> None:
"""An unchanged build.ninja means the compile DB is already current."""
build_dir = toolchain.get_build_dir()
build_dir.mkdir(parents=True)
def run(regenerate_expected: bool) -> None:
with (
patch.object(framework, "check_and_install", return_value=_paths(tmp_path)),
patch.object(framework, "get_build_env", return_value={}),
patch("esphome.build_gen.arduino8266.write_project", return_value=False),
patch.object(
toolchain.subprocess, "run", return_value=MagicMock(returncode=0)
),
patch.object(toolchain, "_write_compile_commands") as mock_compdb,
patch.object(toolchain, "_print_size_summary"),
patch.object(toolchain, "get_idedata"),
):
assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=False) == 0
assert mock_compdb.called == regenerate_expected
# Missing compile DB: regenerated even though build.ninja is unchanged
run(regenerate_expected=True)
# Present compile DB + unchanged build.ninja: skipped
(build_dir / "compile_commands.json").write_text("[]")
run(regenerate_expected=False)
+5 -5
View File
@@ -499,7 +499,7 @@ def test_parse_library_json(tmp_path):
assert result["name"] == "test"
def testparse_library_properties(tmp_path):
def test_parse_library_properties(tmp_path):
f = tmp_path / "library.properties"
f.write_text(
"""
@@ -679,22 +679,22 @@ def test_node_key_registry_bare_name():
assert (key, kind, locator) == ("bar", "registry", (None, "bar"))
def testnormalize_dependencies_none():
def test_normalize_dependencies_none():
assert normalize_dependencies(None) == []
def testnormalize_dependencies_list_form():
def test_normalize_dependencies_list_form():
deps = [{"name": "foo", "version": "1.0"}]
assert normalize_dependencies(deps) == [{"name": "foo", "version": "1.0"}]
def testnormalize_dependencies_dict_form():
def test_normalize_dependencies_dict_form():
out = normalize_dependencies({"nanopb/Nanopb": "^0.4.91", "BareName": "1.2.3"})
assert {"name": "Nanopb", "owner": "nanopb", "version": "^0.4.91"} in out
assert {"name": "BareName", "owner": None, "version": "1.2.3"} in out
def testnormalize_dependencies_dict_form_nested_spec():
def test_normalize_dependencies_dict_form_nested_spec():
out = normalize_dependencies(
{"nanopb/Nanopb": {"version": "^0.4.91", "platforms": "espidf"}}
)
+18
View File
@@ -7168,3 +7168,21 @@ def test_write_cpp_file_arduino_toolchain_writes_no_project(tmp_path: Path) -> N
mock_write_cpp.assert_called_once()
mock_pio_project.assert_not_called()
def test_write_cpp_file_platformio_toolchain_writes_project(tmp_path: Path) -> None:
"""The default toolchain writes the PlatformIO project files."""
setup_core(platform=PLATFORM_ESP8266, tmp_path=tmp_path, name="test")
with (
patch("esphome.writer.write_cpp") as mock_write_cpp,
patch("esphome.build_gen.platformio.write_project") as mock_pio_project,
patch.object(
type(CORE), "cpp_main_section", new_callable=PropertyMock
) as mock_section,
):
mock_section.return_value = ""
assert main.write_cpp_file() == 0
mock_write_cpp.assert_called_once()
mock_pio_project.assert_called_once()