From 6cfbe5b166268cd3289c372ceff4faf5430b0272 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 23 Aug 2026 17:15:29 -0500 Subject: [PATCH] Normalize captured CPPDEFINES to a CppDefine named tuple at capture time --- esphome/platformio/extra_script.py | 67 ++++++++++++------- .../test_platformio_extra_script.py | 7 +- 2 files changed, 45 insertions(+), 29 deletions(-) diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py index 8ffd6ee24b..e60a50d746 100644 --- a/esphome/platformio/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -14,7 +14,7 @@ import logging import os from pathlib import Path import shlex -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, NamedTuple from esphome.core import EsphomeError from esphome.platformio.library import ESPHOME_DATA_KEY, ESPHOME_DATA_LINK_FLAGS_KEY @@ -99,20 +99,47 @@ class ExtraScriptResult: cpppath: list[str] = field(default_factory=list) libpath: list[str] = field(default_factory=list) libs: list[str] = field(default_factory=list) - cppdefines: list[str | tuple[str, str]] = field(default_factory=list) + cppdefines: list[CppDefine] = field(default_factory=list) linkflags: list[str] = field(default_factory=list) cppflags: list[str] = field(default_factory=list) -def _cppdefines_items(value: Any) -> list: - """Normalize SCons ``processDefines`` spellings: a bare 2-tuple is one - ``name=value`` pair, a dict maps names to values, a list is - element-wise.""" +class CppDefine(NamedTuple): + """One normalized CPPDEFINES entry; a ``value`` of None is a bare -DNAME.""" + + name: str + value: str | None = None + + +def _cppdefine(entry: Any) -> CppDefine | None: + """Normalize one CPPDEFINES element, or warn and drop an unsupported + shape; formatting those blind would hand the compiler garbage like + ``-D{'FOO': '1'}``.""" + if isinstance(entry, str): + return CppDefine(entry) + if ( + isinstance(entry, (tuple, list)) + and len(entry) == 2 + and isinstance(entry[0], (str, int)) + and isinstance(entry[1], (str, int, type(None))) + ): + value = entry[1] + return CppDefine(str(entry[0]), None if value is None else str(value)) + _LOGGER.warning("Ignoring unsupported CPPDEFINES entry %r", entry) + return None + + +def _cppdefines_items(value: Any) -> list[CppDefine]: + """Normalize SCons ``processDefines`` spellings into ``CppDefine``s: a + bare 2-tuple is one ``name=value`` pair, a dict maps names to values, a + list is element-wise.""" if isinstance(value, tuple) and len(value) == 2: - return [value] - if isinstance(value, dict): - return list(value.items()) - return list(value) if isinstance(value, (list, tuple)) else [value] + elements: list[Any] = [value] + elif isinstance(value, dict): + elements = list(value.items()) + else: + elements = list(value) if isinstance(value, (list, tuple)) else [value] + return [d for e in elements if (d := _cppdefine(e)) is not None] class _FakeSConsEnv: @@ -316,23 +343,11 @@ def captured_as_build_flags( ) flags.extend(f"-l{shlex.quote(lib)}" for lib in _str_entries(result.libs, "LIBS")) for define in result.cppdefines: - # SCons also accepts nested containers; formatting those blind - # would hand the compiler garbage like -D{'FOO': '1'} - if ( - isinstance(define, (tuple, list)) - and len(define) == 2 - and isinstance(define[0], (str, int)) - and isinstance(define[1], (str, int, type(None))) - ): - if define[1] is None: - # {"FOO": None} / ("FOO", None) is a bare -DFOO in SCons - flags.append(shlex.quote(f"-D{define[0]}")) - else: - flags.append(shlex.quote(f"-D{define[0]}={define[1]}")) - elif isinstance(define, str): - flags.append(shlex.quote(f"-D{define}")) + if define.value is None: + # {"FOO": None} / ("FOO", None) is a bare -DFOO in SCons + flags.append(shlex.quote(f"-D{define.name}")) else: - _LOGGER.warning("Ignoring unsupported CPPDEFINES entry %r", define) + flags.append(shlex.quote(f"-D{define.name}={define.value}")) # Each captured entry is one argv token in SCons; quote so the # lex_build_flags round-trip cannot split a spaced value into two. # LINKFLAGS are deliberately absent: they travel via diff --git a/tests/unit_tests/test_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py index 90ae4aa43b..980ce29ccb 100644 --- a/tests/unit_tests/test_platformio_extra_script.py +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -11,6 +11,7 @@ import pytest from esphome.core import EsphomeError from esphome.platformio.extra_script import ( + CppDefine, ExtraScriptResult, _FakeSConsEnv, apply_extra_script, @@ -51,8 +52,8 @@ def test_extra_script_captures_libpath_libs_and_defines(tmp_path): assert result.libpath == [str(Path("src") / "esp32")] assert result.libs == ["algobsec"] - assert ("BAR", "1") in result.cppdefines - assert "FOO" in result.cppdefines + assert CppDefine("BAR", "1") in result.cppdefines + assert CppDefine("FOO") in result.cppdefines assert result.linkflags == ["-Wl,--gc-sections"] # Lex like the consumer does: quoting makes raw strings platform-varying @@ -476,7 +477,7 @@ def test_spaced_cppflag_survives_relexing(tmp_path) -> None: """A captured argv token with a space stays one token after lexing.""" result = ExtraScriptResult( cppflags=["-include my hdr.h"], - cppdefines=[("MSG", '"hello world"'), "PLAIN"], + cppdefines=[CppDefine("MSG", '"hello world"'), CppDefine("PLAIN")], ) flags = captured_as_build_flags(result, library_dir=tmp_path) assert lex_build_flags(flags, "test") == [