Merge branch 'esp8266-native-ninja-emission' into esp8266-arduino-toolchain

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
@@ -49,11 +50,7 @@ from esphome.framework_helpers import (
     strip_win_long_path_prefix,
 )
 from esphome.helpers import mkdir_p, write_file_if_changed
-from esphome.platformio.library import (
-    SOURCE_KIND_FOR_SUFFIX,
-    lex_build_flags,
-    raise_on_empty_arg_flags,
-)
+from esphome.platformio.library import SOURCE_KIND_FOR_SUFFIX, lex_build_flags
 
 if TYPE_CHECKING:
     from esphome.arduino8266.framework import InstalledPaths
@@ -286,10 +283,8 @@ def _lexed_build_flags() -> list[str]:
 
     Lex once per build; consumers share the tokens.
     """
-    tokens = lex_build_flags(sorted(CORE.build_flags), "esphome")
-    # Raises for every consumer of the shared token list
-    raise_on_empty_arg_flags(tokens, "build_flags")
-    return tokens
+    # The funnel warns and drops empty glued arguments (-D "") itself
+    return lex_build_flags(sorted(CORE.build_flags), "esphome")
 
 
 def _flag_defines(unflags: set[str], tokens: list[str]) -> dict[str, str]:
@@ -568,8 +563,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 _collect_sources(root: Path, exclude: set[str] = frozenset()) -> list[Path]:
@@ -602,17 +608,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(
@@ -691,13 +701,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(
@@ -709,10 +721,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:

esphome/platformio/library.py

@@ -661,27 +661,14 @@ def lex_build_flags(entries: str | list[str], owner: str) -> list[str]:
 BARE_ARG_FLAGS = frozenset({"-I", "-L", "-l", "-D"})
 
 
-def raise_on_empty_arg_flags(tokens: list[str], owner: str) -> None:
-    """Reject bare ``-I``/``-D``/``-L``/``-l`` tokens left by an empty glued
-    argument (``-D ""``).
-
-    Consumed by the ESP8266 native build generator (later in this chain)
-    for user build_flags; library manifests deliberately stay warn-and-drop.
-
-    Lives next to ``join_flag_args`` because the bare token is its
-    postcondition: a trailing bare flag is warned and dropped there, so a
-    surviving one always means an empty argument. gcc would eat the next
-    flag as the argument (or add the CWD for ``-L``); always a typo.
-    """
-    if empty := sorted({tok for tok in tokens if tok in BARE_ARG_FLAGS}):
-        raise EsphomeError(
-            f"{owner} contain empty-argument flag(s): {', '.join(empty)}"
-        )
-
-
 def join_flag_args(tokens: Iterable[str], owner: str) -> list[str]:
     """Join a bare ``-I``/``-L``/``-l``/``-D`` with its following token,
-    the way PlatformIO's ParseFlags lexes them."""
+    the way PlatformIO's ParseFlags lexes them.
+
+    A trailing or empty argument (``-D ""``) is warned and dropped: the
+    bare flag would make gcc eat the next flag as its argument (or add
+    the CWD for ``-L``); always a typo.
+    """
     out: list[str] = []
     it = iter(tokens)
     for tok in it:
@@ -690,6 +677,11 @@ def join_flag_args(tokens: Iterable[str], owner: str) -> list[str]:
             if arg is None:
                 _LOGGER.warning("Ignoring trailing '%s' in %s build flags", tok, owner)
                 break
+            if not arg:
+                _LOGGER.warning(
+                    "Ignoring '%s' with empty argument in %s build flags", tok, owner
+                )
+                continue
             tok += arg
         out.append(tok)
     return out

tests/unit_tests/build_gen/test_arduino8266.py

@@ -939,12 +939,15 @@ def test_vtables_conflicting_raises() -> None:
         _resolve("-DVTABLES_IN_DRAM", "-DVTABLES_IN_IRAM")
 
 
-def test_empty_lib_flags_raise() -> None:
-    """A bare -L would silently add the CWD to the search path; the shared
-    lex point raises for every consumer."""
-    CORE.build_flags = {'-L ""', '-l ""'}
-    with pytest.raises(EsphomeError, match=r"empty-argument flag\(s\): -L, -l"):
-        arduino8266._lexed_build_flags()
+def test_empty_lib_flags_warned_and_dropped(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """A bare -L would silently add the CWD to the search path; the lex
+    funnel warns and drops it for every consumer."""
+    CORE.build_flags = {'-L ""', '-l ""', "-DFOO"}
+    assert arduino8266._lexed_build_flags() == ["-DFOO"]
+    assert "Ignoring '-L' with empty argument" in caplog.text
+    assert "Ignoring '-l' with empty argument" in caplog.text
 
 
 def test_generate_ld_scripts_surfaces_preprocessor_warnings(
@@ -970,6 +973,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."""
@@ -1079,20 +1114,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."""
@@ -1395,12 +1416,15 @@ def test_board_tables_are_equal() -> None:
     assert set(BOARDS) == set(ESP8266_BOARD_BUILD)
 
 
-def test_bare_include_and_define_raise() -> None:
+def test_bare_include_and_define_dropped(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
     """An empty-argument -I or -D would make gcc eat the next flag as the
-    argument; the shared lex point raises for every consumer."""
+    argument; the lex funnel warns and drops both."""
     CORE.build_flags = {'-I ""', '-D ""'}
-    with pytest.raises(EsphomeError, match=r"empty-argument flag\(s\): -D, -I"):
-        arduino8266._lexed_build_flags()
+    assert arduino8266._lexed_build_flags() == []
+    assert "Ignoring '-I' with empty argument" in caplog.text
+    assert "Ignoring '-D' with empty argument" in caplog.text
 
 
 def test_generate_ld_scripts_gcc_change_invalidates_stamp(tmp_path: Path) -> None:

tests/unit_tests/test_platformio_library.py

@@ -723,11 +723,12 @@ def test_prefetch_wave_unknown_size_falls_back_to_sequential(
     assert "No Content-Length for https://x/b.tar.gz" in caplog.text
 
 
-def test_raise_on_empty_arg_flags() -> None:
-    """A surviving bare flag means an empty glued argument; reject by name."""
-    with pytest.raises(EsphomeError, match=r"build_flags contain empty-argument"):
-        lib.raise_on_empty_arg_flags(["-DFOO", "-D", "-l"], "build_flags")
-    lib.raise_on_empty_arg_flags(["-DFOO", "-Iinc"], "build_flags")
+def test_join_flag_args_empty_argument_warns_and_drops(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """An empty glued argument is dropped: a bare -D would eat the next flag."""
+    assert lib.lex_build_flags('-D "" -DFOO', "build_flags") == ["-DFOO"]
+    assert "Ignoring '-D' with empty argument in build_flags" in caplog.text
 
 
 def test_content_lengths_head_requests(monkeypatch: pytest.MonkeyPatch) -> None: