diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 97b4c5ccb5..295d894cd3 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -44,11 +44,7 @@ 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.helpers import mkdir_p, write_file_if_changed -from esphome.platformio.library import ( - SOURCE_KIND_FOR_SUFFIX, - join_flag_args, - split_flag_entry, -) +from esphome.platformio.library import SOURCE_KIND_FOR_SUFFIX, lex_build_flags if TYPE_CHECKING: from esphome.arduino8266.framework import InstalledPaths @@ -218,11 +214,7 @@ def _lexed_build_flags() -> list[str]: stamp). Lex once per build and pass the result to ``_flag_defines`` and ``_project_flags`` so a malformed entry warns once, not per consumer. """ - return [ - tok - for flag in sorted(CORE.build_flags) - for tok in join_flag_args(split_flag_entry(flag, "esphome"), "esphome") - ] + return lex_build_flags(sorted(CORE.build_flags), "esphome") def _flag_defines(unflags: set[str], tokens: list[str] | None = None) -> dict[str, str]: @@ -290,10 +282,13 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: if mmu_knob is not None: if raw := sorted(n for n in defines if n.startswith("MMU_")): # Same compile-line/linker-script split as the no-knob case below - raise EsphomeError( - f"{', '.join(raw)} conflict with {mmu_knob}; drop the raw MMU_* " - "build flags or use PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" + fix = ( + f"drop {mmu_knob} to use the custom sizes" + if "PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" in defines + else "drop the raw MMU_* build flags or use " + "PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" ) + raise EsphomeError(f"{', '.join(raw)} conflict with {mmu_knob}; {fix}") mmu = list(dict(_MMU_VARIANTS)[mmu_knob]) elif "PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" in defines: if "MMU_IRAM_SIZE" not in defines or "MMU_ICACHE_SIZE" not in defines: @@ -370,17 +365,10 @@ def _defines_flags( def _unflag_tokens() -> set[str]: """``build_unflags`` entries shell-lexed to tokens, as PlatformIO matches.""" - # Joined like _project_flags reads build_flags, so "-D FOO" removes + # Lexed like _lexed_build_flags reads build_flags, so "-D FOO" removes # -DFOO in both spellings (PlatformIO's ProcessUnFlags parses the same # way) and no bare half can collaterally drop an unrelated token - return { - tok - for entry in CORE.build_unflags - for tok in join_flag_args( - split_flag_entry(entry, "esphome build_unflags"), - "esphome build_unflags", - ) - } + return set(lex_build_flags(list(CORE.build_unflags), "esphome build_unflags")) def _project_flags( @@ -418,11 +406,11 @@ def _project_flags( libs.append(tok[2:]) else: if tok == "-u" or tok.startswith(("-T", "-Xlinker")): - # Inert on the compile line; the user expects it to link - _LOGGER.warning( - "Linker flag %s in build_flags is not routed to the link " - "line; use the -Wl, form", - tok, + # Inert on the -c compile line; the firmware would silently + # lack the requested link behavior + raise EsphomeError( + f"Linker flag {tok} in build_flags is not routed to the " + "link line; use the -Wl, form" ) compile_flags.append(_shell_token(tok)) return compile_flags, link_flags, lib_dirs, libs @@ -470,8 +458,13 @@ def generate_ld_scripts( try: header_stat = header.stat() header_sig = f"{header_stat.st_size}:{header_stat.st_mtime_ns}" - except OSError: + except FileNotFoundError: header_sig = "missing" # the preprocessor spawn below names it + except OSError as err: + # An unreadable header must force a cache miss every run, not pin + # the stamp to a constant that can never notice a later edit + _LOGGER.debug("Could not stat %s: %s", header, err) + header_sig = f"unreadable:{os.urandom(8).hex()}" stamp_content = ( " ".join(cmd) + f" testing={CORE.testing_mode}" @@ -496,6 +489,7 @@ def generate_ld_scripts( except (OSError, UnicodeDecodeError): return False + stderr_note = ld_dir / ".local.eagle.app.v6.common.ld.stderr" if not _cached_ld_is_valid(): try: result = subprocess.run( @@ -510,7 +504,11 @@ def generate_ld_scripts( raise EsphomeError(f"Generating the linker script failed:\n{result.stderr}") if result.stderr.strip(): # Preprocessor warnings on the success path must reach the user + # on this and every later cached build (see the re-emit below) _LOGGER.warning("Linker-script preprocessor: %s", result.stderr.strip()) + stderr_note.write_text(result.stderr.strip(), encoding="utf-8") + else: + stderr_note.unlink(missing_ok=True) if "SECTIONS" not in result.stdout: # A degenerate zero-exit run must not be stamped as a good cache raise EsphomeError( @@ -524,11 +522,22 @@ def generate_ld_scripts( # traceback, and never a silently unrelocated rate table raise EsphomeError(str(err)) from err if CORE.testing_mode: - content = build_surgery.apply_testing_memory_patches( - content, ("iram1_0_seg",) - ) + try: + content = build_surgery.apply_testing_memory_patches( + content, ("iram1_0_seg",) + ) + except RuntimeError as err: + # Same changed-linker-script failure class as the ratetable + raise EsphomeError(str(err)) from err write_file_if_changed(output, content) stamp.write_text(stamp_content, encoding="utf-8") + elif stderr_note.is_file(): + # The diagnostic must not vanish for the life of the build dir just + # because the script is cached + _LOGGER.warning( + "Linker-script preprocessor: %s", + stderr_note.read_text(encoding="utf-8"), + ) if CORE.testing_mode: # A patched copy of the flash ld in the build dir; resolved through @@ -541,13 +550,15 @@ def generate_ld_scripts( raise EsphomeError( f"Could not read {flash_ld}: {err}; run 'esphome clean-all' and retry" ) from err - write_file_if_changed( - ld_dir / f"testing_{flash_ld_name}", - build_surgery.apply_testing_memory_patches( + try: + patched_flash_ld = build_surgery.apply_testing_memory_patches( flash_ld_text, ("dram0_0_seg", "irom0_0_seg"), - ), - ) + ) + except RuntimeError as err: + # Same changed-linker-script failure class as the ratetable + raise EsphomeError(str(err)) from err + write_file_if_changed(ld_dir / f"testing_{flash_ld_name}", patched_flash_ld) def _ninja_compile_edges( @@ -760,6 +771,10 @@ def write_project(paths: InstalledPaths) -> bool: " rspfile_content = $in_newline", " description = LINK $out", "rule elf2bin", + # --flash_freq 40: upstream derives this from the board JSON's + # f_flash, but all 45 supported boards ship 40 MHz (audited against + # platform-espressif8266); re-check if a future platform bump adds + # a board with a different f_flash f" command = $python {_q(framework / 'tools' / 'elf2bin.py')} --eboot {_q(framework / 'bootloaders' / 'eboot' / 'eboot.elf')} --app $in --flash_mode {esp8266_data[KEY_FLASH_MODE]} --flash_freq 40 --flash_size {_flash_size_str(BOARDS[board][KEY_FLASH_SIZE])} --path {_q(toolchain_bin)} --out $out", " description = BIN $out", "rule copy", diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index b54c023864..6f2eba4043 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -1,11 +1,14 @@ """Drift tests for the native ESP8266 Arduino build generator. -These pin the build spec transliterated from the PlatformIO builder +These pin the ESPHome side of the transliteration (the knob-define +precedence, the define/flag sets, and the linker-script generation) against +literals audited from the PlatformIO builder (framework-arduinoespressif8266/tools/platformio-build.py and -platform-espressif8266/builder/main.py) so a change on either side of the -toolchain seam is caught: the knob-define precedence, the define/flag sets, -the link line, and the core source exclusions must keep matching what the -PlatformIO toolchain produces for the same configuration. +platform-espressif8266/builder/main.py): the knob-define precedence, the +define/flag sets, the link line, and the core source exclusions. They catch +an accidental edit on this side; an upstream change in a new framework +release is caught by the A/B byte-identical build check on a version bump, +not by these tests. """ from __future__ import annotations @@ -1024,16 +1027,20 @@ def test_lexed_build_flags_shared_between_consumers( ) -def test_project_flags_warns_on_plain_linker_forms( - caplog: pytest.LogCaptureFixture, -) -> None: - """A plain-form linker flag lands on the compile line where it is inert; - the user must be told to use the -Wl, form.""" - _set_flags("-Tcustom.ld", "-Xlinker", "-u", "-Os") +@pytest.mark.parametrize("tok", ["-Tcustom.ld", "-Xlinker", "-u"]) +def test_project_flags_rejects_plain_linker_forms(tok: str) -> None: + """A plain-form linker flag would land on the -c compile line where it + is inert; refuse naming the -Wl, form instead of shipping firmware that + silently lacks the requested link behavior.""" + _set_flags(tok) + with pytest.raises(EsphomeError, match="use the -Wl, form"): + arduino8266._project_flags(set()) + + +def test_project_flags_plain_compile_flags_pass() -> None: + _set_flags("-Os") compile_flags, _l, _d, _libs = arduino8266._project_flags(set()) assert "-Os" in compile_flags - for tok in ("-Tcustom.ld", "-Xlinker", "-u"): - assert f"Linker flag {tok} in build_flags is not routed" in caplog.text def test_generate_ld_scripts_header_change_invalidates_stamp( @@ -1099,3 +1106,114 @@ def test_write_project_lexes_build_flags_once( _set_flags("-DFOO=1 -l") _write_ninja(paths) assert caplog.text.count("Ignoring trailing '-l'") == 1 + + +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())) + + +def test_generate_ld_scripts_testing_surgery_failure_is_named( + tmp_path: Path, +) -> None: + """A testing-mode segment patch failing on a changed linker script is a + named error, like the ratetable surgery.""" + paths = _make_framework(tmp_path) + CORE.testing_mode = True + result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="") + with ( + patch.object(arduino8266.subprocess, "run", return_value=result), + patch.object( + arduino8266.build_surgery, + "apply_testing_memory_patches", + side_effect=RuntimeError("iram1_0_seg not found"), + ), + pytest.raises(EsphomeError, match="iram1_0_seg not found"), + ): + _run_generate_ld_scripts(paths) + + +def test_generate_ld_scripts_testing_flash_ld_surgery_failure_is_named( + tmp_path: Path, +) -> None: + """The flash-ld segment patch gets the same named-error wrap.""" + paths = _make_framework(tmp_path) + (paths.framework / "tools" / "sdk" / "ld" / "eagle.flash.4m.ld").write_text( + "MEMORY { }" + ) + CORE.testing_mode = True + result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="") + with ( + patch.object(arduino8266.subprocess, "run", return_value=result), + patch.object( + arduino8266.build_surgery, + "apply_testing_memory_patches", + side_effect=["patched common", RuntimeError("dram0_0_seg mismatch")], + ), + pytest.raises(EsphomeError, match="dram0_0_seg mismatch"), + ): + _run_generate_ld_scripts(paths) + + +def test_generate_ld_scripts_reemits_cached_preprocessor_warning( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A preprocessor diagnostic survives cache hits instead of appearing + once and vanishing for the life of the build dir.""" + paths = _make_framework(tmp_path) + result = MagicMock( + returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="warning: something odd" + ) + with patch.object(arduino8266.subprocess, "run", return_value=result): + _run_generate_ld_scripts(paths) + assert caplog.text.count("warning: something odd") == 1 + with patch.object(arduino8266.subprocess, "run") as mock_run: + _run_generate_ld_scripts(paths) + mock_run.assert_not_called() + assert caplog.text.count("warning: something odd") == 2 + + +def test_generate_ld_scripts_unreadable_header_forces_regeneration( + tmp_path: Path, +) -> None: + """A stat failure other than absence must miss the cache every run, not + pin the stamp to a constant that can never notice a later edit.""" + paths = _make_framework(tmp_path) + header_name = "eagle.app.v6.common.ld.h" + (paths.framework / "tools" / "sdk" / "ld" / header_name).write_text("v1") + real_stat = Path.stat + + def fake_stat(self: Path, **kwargs: object): + if self.name == header_name: + raise PermissionError(13, "denied") + return real_stat(self, **kwargs) + + result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="") + with patch.object(Path, "stat", fake_stat): + with patch.object( + arduino8266.subprocess, "run", return_value=result + ) as mock_run: + _run_generate_ld_scripts(paths) + mock_run.assert_called_once() + with patch.object( + arduino8266.subprocess, "run", return_value=result + ) as mock_run: + _run_generate_ld_scripts(paths) + mock_run.assert_called_once() + + +def test_board_tables_are_equal() -> None: + """write_project rejects a board missing from either table, so the two + must stay exactly in sync (the build-surgery test only checks the + subset direction, which is how d1_wroom_02 went missing).""" + from esphome.components.esp8266.boards import BOARDS + + assert set(BOARDS) == set(ESP8266_BOARD_BUILD)