From eb982e07a776578a3b70d61b093458524cc5286c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 23 Aug 2026 12:17:43 -0500 Subject: [PATCH] Veto the ld-script cache stamp when the warn note is lost, widen the linker denylist A failed stderr-sidecar write no longer stamps the cache: the next build re-runs -E and re-derives the preprocessor diagnostic honestly instead of losing it for the life of the build dir. The sidecar unlink joins the best-effort contract. The plain-linker denylist gains the -nodefaultlibs/-nostdlib/-rdynamic family and the -fuse-ld=/--specs= prefixes, with a comment naming it best-effort rather than complete. The damaged-file helper test now lives only in test_helpers.py beside the OSError-branch coverage. --- esphome/build_gen/arduino8266.py | 39 ++++++++++++---- .../unit_tests/build_gen/test_arduino8266.py | 46 +++++++++++++------ 2 files changed, 62 insertions(+), 23 deletions(-) diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 043c18f989..6d6ae49b10 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -14,6 +14,7 @@ from the build flags with the same precedence as the PlatformIO builder. from __future__ import annotations +from contextlib import suppress from dataclasses import dataclass import hashlib import logging @@ -495,8 +496,19 @@ def _project_flags( # Plain-form linker flags rejected by _project_flags: inert on a -c compile # line, so the firmware would silently lack the requested link behavior -_PLAIN_LINKER_FLAGS = ("-u", "-e", "-s", "-static", "-nostartfiles") -_PLAIN_LINKER_PREFIXES = ("-T", "-Xlinker") +# Best-effort, not exhaustive: an unlisted link-only spelling still falls +# through to the compile line +_PLAIN_LINKER_FLAGS = ( + "-u", + "-e", + "-s", + "-static", + "-nostartfiles", + "-nodefaultlibs", + "-nostdlib", + "-rdynamic", +) +_PLAIN_LINKER_PREFIXES = ("-T", "-Xlinker", "-fuse-ld=", "--specs=") def _stat_sig(path: Path) -> str: @@ -521,17 +533,21 @@ def _stat_sig(path: Path) -> str: return f"unreadable:{os.urandom(8).hex()}" -def _write_note(path: Path, text: str, *, warn: bool = False) -> None: +def _write_note(path: Path, text: str, *, warn: bool = False) -> bool: """Best-effort bookkeeping write; a failure never fails the build. ``warn`` marks notes whose loss drops a diagnostic on later cached builds; a lost stamp only costs a cache miss and stays at debug. + Returns whether the write persisted, so a lost warn note can veto + the cache stamp and keep the diagnostic re-derivable. """ try: path.write_text(text, encoding="utf-8") except OSError as err: log = _LOGGER.warning if warn else _LOGGER.debug log("Could not write %s: %s", path, err) + return False + return True def generate_ld_scripts( @@ -610,13 +626,15 @@ def generate_ld_scripts( raise EsphomeError(f"Could not run {gcc}: {err}; {_CLEAN_HINT}") from err if result.returncode != 0: raise EsphomeError(f"Generating the linker script failed:\n{result.stderr}") + note_persisted = True 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()) - _write_note(stderr_note, result.stderr.strip(), warn=True) + note_persisted = _write_note(stderr_note, result.stderr.strip(), warn=True) else: - stderr_note.unlink(missing_ok=True) + with suppress(OSError): + 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( @@ -628,10 +646,13 @@ def generate_ld_scripts( build_surgery.apply_testing_memory_patches, content, ("iram1_0_seg",) ) write_file_if_changed(output, content) - _write_note( - stamp, - f"{stamp_content} content={hashlib.sha256(content.encode('utf-8')).hexdigest()}", - ) + if note_persisted: + # An unstamped cache re-runs -E next build, re-deriving the + # diagnostic the lost note would have re-emitted + _write_note( + stamp, + f"{stamp_content} content={hashlib.sha256(content.encode('utf-8')).hexdigest()}", + ) elif stderr_note.is_file(): # Re-emit cached preprocessor warnings on cache hits try: diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index 41f506f713..810c1bad12 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -625,6 +625,38 @@ def test_generate_ld_scripts_surfaces_preprocessor_warnings( _run_generate_ld_scripts(paths) +def test_generate_ld_scripts_lost_warn_note_vetoes_the_stamp( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A warn note that could not persist skips the stamp, so the next build + re-runs -E and re-derives the diagnostic instead of losing it.""" + paths = _make_framework(tmp_path) + _set_flags() + result = MagicMock( + returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="warning: something" + ) + real_write_text = Path.write_text + + def fail_note_writes(self: Path, text: str, encoding: str = "utf-8") -> int: + if self.name.endswith(".stderr"): + raise OSError("read-only build dir") + return real_write_text(self, text, encoding=encoding) + + with ( + patch.object(arduino8266.subprocess, "run", return_value=result) as run1, + patch.object(Path, "write_text", fail_note_writes), + ): + _run_generate_ld_scripts(paths) + run1.assert_called_once() + assert "Could not write" in caplog.text + + # Unstamped: the second build re-runs the preprocessor + with patch.object(arduino8266.subprocess, "run", return_value=result) as run2: + _run_generate_ld_scripts(paths) + run2.assert_called_once() + assert caplog.text.count("Linker-script preprocessor: warning: something") == 2 + + 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.""" @@ -734,20 +766,6 @@ def test_generate_ld_scripts_invalid_flash_ld_name_raises(tmp_path: Path) -> Non arduino8266.generate_ld_scripts(paths, config, "../evil.ld") -def test_write_generated_replaces_damaged_file( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: - """A non-UTF-8 existing copy is logged and overwritten; the write path - still raises for real failures (now the shared helper's contract).""" - from esphome.helpers import write_file_if_changed - - target = tmp_path / "gen.ld" - target.write_bytes(b"\xff\xfe") - write_file_if_changed(target, "SECTIONS { }") - assert target.read_text(encoding="utf-8") == "SECTIONS { }" - assert "Replacing damaged file" in caplog.text - - def test_generate_ld_scripts_edited_output_regenerates(tmp_path: Path) -> None: """The stamp records the content hash, so an externally edited cached script regenerates instead of linking untrusted content."""