From 03dba8b63e2248b76578ae3d4793237eca9d3e47 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 19:13:41 -0500 Subject: [PATCH 1/2] Raise on empty-argument build flags, pin the preprocessor decode, soften bookkeeping writes --- esphome/build_gen/arduino8266.py | 64 +++++++++++-------- .../unit_tests/build_gen/test_arduino8266.py | 50 ++++++--------- 2 files changed, 58 insertions(+), 56 deletions(-) diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 6a07d651f5..83018b5d1f 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -88,6 +88,9 @@ _MMU_VARIANTS = ( ("MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000", "MMU_EXTERNAL_HEAP=128"), ), ( + # Upstream really does cap the 1024K option's heap knob at 256 + # (platformio-build.py's MMU_EXTERNAL_1024K branch); transliterated + # verbatim "PIO_FRAMEWORK_ARDUINO_MMU_EXTERNAL_1024K", ("MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000", "MMU_EXTERNAL_HEAP=256"), ), @@ -180,14 +183,22 @@ def _lexed_build_flags() -> list[str]: Lex once per build; consumers share the tokens. """ - return lex_build_flags(sorted(CORE.build_flags), "esphome") + tokens = lex_build_flags(sorted(CORE.build_flags), "esphome") + # The lexer glues '-D ""' to a bare "-D"; gcc would eat the next flag + # as its argument (or add the CWD for -L). Always a typo, so raise for + # every consumer of the shared token list. + if empty := sorted({tok for tok in tokens if tok in ("-I", "-D", "-L", "-l")}): + raise EsphomeError( + f"build_flags contain empty-argument flag(s): {', '.join(empty)}" + ) + return tokens def _flag_defines(unflags: set[str], tokens: list[str]) -> dict[str, str]: """Map define name -> full ``NAME[=VALUE]`` for every -D build flag. ``tokens`` comes from one ``_lexed_build_flags()`` call shared with - ``_project_flags`` so a malformed entry warns once, structurally. + ``_project_flags``, which already raised on any bare "-D". """ defines: dict[str, str] = {} for tok in tokens: @@ -195,9 +206,7 @@ def _flag_defines(unflags: set[str], tokens: list[str]) -> dict[str, str]: # being absent from the compile line if tok in unflags: continue - # A bare "-D" is skipped here and warned about in _project_flags, - # which sees the same token list - if tok.startswith("-D") and len(tok) > 2: + if tok.startswith("-D"): body = tok[2:] defines[body.split("=", 1)[0]] = body return defines @@ -299,13 +308,11 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: def _pio_option(key: str, default: str) -> str: """A platformio_options value the native build honors (str-normalized). - Routed into ``CORE.platformio_options`` by core/config.py under the - arduino toolchain; a repeated option accumulates as a list, where the - last value wins like a later platformio.ini line. + core/config.py routes these into ``CORE.platformio_options`` under the + arduino toolchain and already collapses a repeated option to its last + value (like a later platformio.ini line), so a scalar always arrives. """ value = CORE.platformio_options.get(key) - if isinstance(value, list): - value = value[-1] if value else "" if value is None: return default value = str(value).strip() @@ -382,23 +389,12 @@ def _project_flags( for tok in tokens: if tok in unflags: continue - if tok in ("-I", "-D"): - # A bare form from '-I ""' would make gcc eat the next flag as - # its argument (silently, for a nonexistent include dir) - _LOGGER.warning("Ignoring empty %s in build_flags", tok) - continue + # _lexed_build_flags raised on any bare -I/-D/-L/-l if tok.startswith("-Wl,"): link_flags.append(_shell_token(tok)) elif tok.startswith("-L"): - if len(tok) == 2: - # Path("") is the CWD; never add it silently - _LOGGER.warning("Ignoring empty -L in build_flags") - continue lib_dirs.append(Path(tok[2:])) elif tok.startswith("-l"): - if len(tok) == 2: - _LOGGER.warning("Ignoring empty -l in build_flags") - continue libs.append(tok[2:]) else: if tok in _PLAIN_LINKER_FLAGS or tok.startswith(_PLAIN_LINKER_PREFIXES): @@ -438,6 +434,15 @@ def _stat_sig(path: Path) -> str: return f"unreadable:{os.urandom(8).hex()}" +def _write_note(path: Path, text: str) -> None: + """Best-effort bookkeeping write; a failure only costs a cache miss or + a lost re-emitted warning, never the build.""" + try: + path.write_text(text, encoding="utf-8") + except OSError as err: + _LOGGER.debug("Could not write %s: %s", path, err) + + def _write_generated(path: Path, content: str) -> None: """write_file_if_changed, replacing an unreadable existing copy. @@ -515,7 +520,14 @@ def generate_ld_scripts( if not _cached_ld_is_valid(): try: result = subprocess.run( - cmd, capture_output=True, text=True, check=False, close_fds=False + cmd, + capture_output=True, + # Localized gcc diagnostics on a non-UTF-8 console must + # degrade, not UnicodeDecodeError the build + encoding="utf-8", + errors="replace", + check=False, + close_fds=False, ) except OSError as err: # A half-extracted or half-deleted toolchain cache reaches here @@ -528,7 +540,7 @@ def generate_ld_scripts( # 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") + _write_note(stderr_note, result.stderr.strip()) else: stderr_note.unlink(missing_ok=True) if "SECTIONS" not in result.stdout: @@ -552,9 +564,9 @@ def generate_ld_scripts( # Same changed-linker-script failure class as the ratetable raise EsphomeError(str(err)) from err _write_generated(output, content) - stamp.write_text( + _write_note( + stamp, f"{stamp_content} content={hashlib.sha256(content.encode('utf-8')).hexdigest()}", - encoding="utf-8", ) elif stderr_note.is_file(): # Re-emit cached preprocessor warnings on cache hits diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index e7c2d50323..8c55efcf92 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -597,18 +597,12 @@ def test_vtables_conflicting_raises() -> None: _resolve("-DVTABLES_IN_DRAM", "-DVTABLES_IN_IRAM") -def test_project_flags_empty_lib_flags_warn( - caplog: pytest.LogCaptureFixture, -) -> None: - """A bare -L must not silently add the CWD to the search path.""" +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 ""'} - _c, _l, lib_dirs, libs = arduino8266._project_flags( - set(), arduino8266._lexed_build_flags() - ) - assert lib_dirs == [] - assert libs == [] - assert "Ignoring empty -L" in caplog.text - assert "Ignoring empty -l" in caplog.text + with pytest.raises(EsphomeError, match=r"empty-argument flag\(s\): -L, -l"): + arduino8266._lexed_build_flags() def test_generate_ld_scripts_surfaces_preprocessor_warnings( @@ -800,15 +794,22 @@ def test_generate_ld_scripts_unreadable_note_still_warns( assert "could not be read" in caplog.text +def test_write_note_failure_is_best_effort( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """A failed stamp or stderr-note write costs a cache miss, never the + build.""" + caplog.set_level("DEBUG") + arduino8266._write_note(tmp_path / "missing" / "stamp", "x") + assert "Could not write" in caplog.text + + def test_pio_option_blank_value_raises() -> None: """An empty or blank platformio_options value is a config error, not a silent fallback to the default.""" CORE.platformio_options = {"board_build.f_cpu": " "} with pytest.raises(EsphomeError, match="board_build.f_cpu is empty"): arduino8266._pio_option("board_build.f_cpu", "80000000L") - CORE.platformio_options = {"board_build.f_cpu": []} - with pytest.raises(EsphomeError, match="board_build.f_cpu is empty"): - arduino8266._pio_option("board_build.f_cpu", "80000000L") def test_defines_flags_invalid_f_cpu_raises() -> None: @@ -937,19 +938,12 @@ def test_generate_ld_scripts_unreadable_header_forces_regeneration( mock_run.assert_called_once() -def test_project_flags_warns_on_bare_include_and_define( - caplog: pytest.LogCaptureFixture, -) -> None: - """An empty-argument -I or -D must not reach gcc, which would eat the - next flag as the argument.""" +def test_bare_include_and_define_raise() -> 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.""" CORE.build_flags = {'-I ""', '-D ""'} - compile_flags, _l, _d, _libs = arduino8266._project_flags( - set(), arduino8266._lexed_build_flags() - ) - assert "-I" not in compile_flags - assert "-D" not in compile_flags - assert "Ignoring empty -I in build_flags" in caplog.text - assert "Ignoring empty -D in build_flags" in caplog.text + with pytest.raises(EsphomeError, match=r"empty-argument flag\(s\): -D, -I"): + arduino8266._lexed_build_flags() def test_generate_ld_scripts_gcc_change_invalidates_stamp(tmp_path: Path) -> None: @@ -978,7 +972,3 @@ def test_defines_flags_honors_f_cpu_override() -> None: CORE.platformio_options = {"board_build.f_cpu": "160000000L"} defines = _defines_flags(config, "dout", "nodemcuv2", board_build["defines"]) assert "-DF_CPU=160000000L" in defines - # A repeated option accumulates as a list; the last value wins - CORE.platformio_options = {"board_build.f_cpu": ["80000000L", "160000000L"]} - defines = _defines_flags(config, "dout", "nodemcuv2", board_build["defines"]) - assert "-DF_CPU=160000000L" in defines From 82b20313b0817b00754369d2d9c14a0515e6589a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 22 Aug 2026 19:14:32 -0500 Subject: [PATCH 2/2] Export the consumed platformio_options set so the ignored-option warning cannot drift --- esphome/core/config.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/esphome/core/config.py b/esphome/core/config.py index b1b234adde..6f9b271dca 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -559,6 +559,10 @@ def _add_library_str(lib: str) -> None: # in this chain) will honor; its ignored-option warning will consume the same # list so the two cannot drift NATIVE_ARDUINO_PIO_OPTIONS = frozenset({"board_build.f_cpu", "board_build.ldscript"}) +# The full set that survives into CORE.platformio_options under the native +# arduino toolchain: lib_ignore is the only specially-translated key below +# that is stored rather than translated away +NATIVE_ARDUINO_CONSUMED_PIO_OPTIONS = NATIVE_ARDUINO_PIO_OPTIONS | {"lib_ignore"} @coroutine_with_priority(CoroPriority.FINAL)