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.
This commit is contained in:
J. Nick Koston
2026-08-23 12:17:43 -05:00
parent 10d82b129a
commit eb982e07a7
2 changed files with 62 additions and 23 deletions
+30 -9
View File
@@ -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:
+32 -14
View File
@@ -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."""