From 66f6f5c00409fa007648f643cd286c55d2e9b592 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 19 Aug 2026 23:50:16 -0500 Subject: [PATCH] Address review: cover all source suffixes, pass PIOPLATFORM through, link library -Wl flags --- esphome/arduino8266/component.py | 7 +++-- esphome/build_gen/arduino8266.py | 18 ++++++++++++- esphome/espidf/extra_script.py | 26 ++++++++++++++----- .../unit_tests/build_gen/test_arduino8266.py | 14 +++++++++- .../unit_tests/test_arduino8266_component.py | 6 ++++- tests/unit_tests/test_espidf_component.py | 13 ++++++++++ 6 files changed, 73 insertions(+), 11 deletions(-) diff --git a/esphome/arduino8266/component.py b/esphome/arduino8266/component.py index 2b3b5b5da3..21c9225bfc 100644 --- a/esphome/arduino8266/component.py +++ b/esphome/arduino8266/component.py @@ -50,9 +50,10 @@ class ArduinoLibrary: # Extra compile flags private to this library's own sources flags: list[str] = field(default_factory=list) # Link inputs the library contributes (-L dirs / -l libs, e.g. from - # precompiled vendor blobs) + # precompiled vendor blobs) and -Wl, options for the firmware link link_dirs: list[Path] = field(default_factory=list) link_libs: list[str] = field(default_factory=list) + link_flags: list[str] = field(default_factory=list) def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: @@ -84,6 +85,8 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: lib.link_dirs.append((read_path / tok[2:]).resolve()) elif tok.startswith("-l"): lib.link_libs.append(tok[2:]) + elif tok.startswith("-Wl,"): + lib.link_flags.append(tok) else: lib.flags.append(tok) @@ -148,7 +151,7 @@ def resolve_libraries(framework_path: Path) -> list[ArduinoLibrary]: bundled.append(_bundled_library(framework_path, name)) def _emit(component: ConvertedLibrary) -> None: - apply_extra_script(component, "esp8266") + apply_extra_script(component, "esp8266", pio_platform=ESP8266_PLATFORM) converted.append( _library_info( component.get_require_name(), component.source_dir, component.data diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 1ebed13961..3ecb401e6e 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -42,7 +42,22 @@ from esphome.core import CORE, EsphomeError from esphome.framework_helpers import get_project_cxx_compile_flags from esphome.helpers import mkdir_p, write_file_if_changed -_RULE_FOR_SUFFIX = {".c": "cc", ".cpp": "cxx", ".S": "asm"} +# Compile rule per source suffix; keys must cover SRC_FILE_EXTENSIONS so any +# source a library manifest selects has a rule (pinned by a drift test). +_RULE_FOR_SUFFIX = { + ".c": "cc", + ".cpp": "cxx", + ".cc": "cxx", + ".cxx": "cxx", + ".c++": "cxx", + ".S": "asm", + ".spp": "asm", + ".SPP": "asm", + ".sx": "asm", + ".s": "asm", + ".asm": "asm", + ".ASM": "asm", +} # Always excluded from the core build: ESPHome uses its own native OTA # backend, so the Arduino Updater (and its 228-byte global) never links. @@ -437,6 +452,7 @@ def write_project(paths: dict[str, Path]) -> bool: if esp8266_data.get(KEY_SCANF_FLOAT): link_flags += ["-u", "_scanf_float"] link_flags += project_link_flags + link_flags += [flag for lib in libraries for flag in lib.link_flags] flash_ld = f"testing_{flash_ld_name}" if CORE.testing_mode else flash_ld_name link_flags += ["-T", flash_ld] diff --git a/esphome/espidf/extra_script.py b/esphome/espidf/extra_script.py index d9d929d47a..8cbef0fd53 100644 --- a/esphome/espidf/extra_script.py +++ b/esphome/espidf/extra_script.py @@ -42,7 +42,9 @@ _LOGGER = logging.getLogger(__name__) def apply_extra_script( - component: ConvertedLibrary, idf_target: str | Callable[[], str] + component: ConvertedLibrary, + idf_target: str | Callable[[], str], + pio_platform: str = "espressif32", ) -> None: """Run a library's PIO ``extraScript`` and fold its captured env vars into ``component.data["build"]["flags"]`` so the backend's -L/-l/-D extraction @@ -50,6 +52,7 @@ def apply_extra_script( ``idf_target`` may be a callable so a backend whose target lookup needs build state (the esp32 variant) resolves it only when a script will run. + ``pio_platform`` is exposed to the script as PlatformIO's ``PIOPLATFORM``. """ extra_script = component.data.get("build", {}).get("extraScript") if not extra_script: @@ -64,7 +67,10 @@ def apply_extra_script( if callable(idf_target): idf_target = idf_target() result = run_extra_script( - script_path, library_dir=source_path, idf_target=idf_target + script_path, + library_dir=source_path, + idf_target=idf_target, + pio_platform=pio_platform, ) extra_flags = captured_as_build_flags(result, library_dir=source_path) if not extra_flags: @@ -101,10 +107,10 @@ class _FakeSConsEnv: ``AttributeError`` and abort the script. """ - def __init__(self, *, board_mcu: str, pio_env: str) -> None: + def __init__(self, *, board_mcu: str, pio_env: str, pio_platform: str) -> None: self._vars: dict[str, str] = { "BOARD_MCU": board_mcu, - "PIOPLATFORM": "espressif32", + "PIOPLATFORM": pio_platform, "PIOENV": pio_env, } self.result = ExtraScriptResult() @@ -132,7 +138,11 @@ class _FakeSConsEnv: def run_extra_script( - script_path: Path, *, library_dir: Path, idf_target: str + script_path: Path, + *, + library_dir: Path, + idf_target: str, + pio_platform: str = "espressif32", ) -> ExtraScriptResult: """Execute ``script_path`` with a fake SCons env and return captured vars. @@ -147,7 +157,11 @@ def run_extra_script( an empty result — extra-scripts are best-effort, and an unsupported script shouldn't block the build. """ - env = _FakeSConsEnv(board_mcu=idf_target, pio_env=f"esphome_{idf_target}") + env = _FakeSConsEnv( + board_mcu=idf_target, + pio_env=f"esphome_{idf_target}", + pio_platform=pio_platform, + ) code = compile(script_path.read_text(encoding="utf-8"), str(script_path), "exec") old_cwd = Path.cwd() try: diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index f7d2264e73..4aa01a5fdd 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -63,6 +63,13 @@ def test_board_build_covers_every_board() -> None: assert set(BOARDS) <= set(ESP8266_BOARD_BUILD) +def test_rule_map_covers_all_source_suffixes() -> None: + """Every suffix a library manifest can select must map to a ninja rule.""" + from esphome.platformio.library import SRC_FILE_EXTENSIONS + + assert set(arduino8266._RULE_FOR_SUFFIX) == set(SRC_FILE_EXTENSIONS) + + def test_build_config_defaults() -> None: _set_flags() @@ -409,14 +416,16 @@ def test_write_project_libraries_and_variant(tmp_path: Path) -> None: lib_dir = tmp_path / "libsrc" lib_dir.mkdir() (lib_dir / "lib.cpp").write_text("") + (lib_dir / "impl.cc").write_text("") headers_only = ArduinoLibrary(name="HeadersOnly", include_dirs=[lib_dir]) library = ArduinoLibrary( name="MyLib", - sources=[lib_dir / "lib.cpp"], + sources=[lib_dir / "impl.cc", lib_dir / "lib.cpp"], include_dirs=[lib_dir], flags=["-DMYLIB=1"], link_dirs=[lib_dir / "blobs"], link_libs=["algobsec"], + link_flags=["-Wl,--wrap=malloc"], ) _set_flags("-DPIO_FRAMEWORK_ARDUINO_ENABLE_EXCEPTIONS") @@ -430,6 +439,9 @@ def test_write_project_libraries_and_variant(tmp_path: Path) -> None: assert "libHeadersOnly.a" not in content assert " flags = -DMYLIB=1" in content assert "-lalgobsec" in content + # Library link flags reach the firmware link line; .cc compiles as C++ + assert "-Wl,--wrap=malloc" in content + assert "impl.cc.o: cxx" in content assert f'-L"{lib_dir / "blobs"}"' in content # Exceptions knob: -fexceptions and the exception-enabled stdc++ assert "-fexceptions" in content diff --git a/tests/unit_tests/test_arduino8266_component.py b/tests/unit_tests/test_arduino8266_component.py index f0e339c0ec..3c6af9be37 100644 --- a/tests/unit_tests/test_arduino8266_component.py +++ b/tests/unit_tests/test_arduino8266_component.py @@ -65,6 +65,7 @@ def test_library_info_flags_parsing(tmp_path: Path) -> None: "-DFOO=1 -I inc", "-lalgobsec", "-fno-lto", + "-Wl,--wrap=malloc", "-l", "m", "-L", @@ -80,6 +81,7 @@ def test_library_info_flags_parsing(tmp_path: Path) -> None: ] assert lib.link_dirs == [(read_path / "blobs").resolve()] assert lib.link_libs == ["algobsec", "m"] + assert lib.link_flags == ["-Wl,--wrap=malloc"] def test_library_info_no_src_dir(tmp_path: Path) -> None: @@ -144,7 +146,9 @@ def test_resolve_libraries_external_and_bundled_deps(tmp_path: Path) -> None: ): libs = component.resolve_libraries(framework) - mock_extra.assert_called_once_with(converted, "esp8266") + mock_extra.assert_called_once_with( + converted, "esp8266", pio_platform="espressif8266" + ) assert [lib.name for lib in libs] == [ "Wire", "esp32async__ESPAsyncWebServer", diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index c919953076..f168faa1df 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -1254,3 +1254,16 @@ def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None: apply_extra_script(c, "esp8266") assert "flags" not in c.data["build"] assert "skipping" in caplog.text + + +def test_apply_extra_script_pio_platform(tmp_path) -> None: + """The backend's platform token is exposed to the script as PIOPLATFORM.""" + from esphome.espidf.extra_script import apply_extra_script + + script = tmp_path / "extra.py" + script.write_text("env.Append(LIBS=[env.get('PIOPLATFORM')])\n") + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": "extra.py"}} + apply_extra_script(c, "esp8266", pio_platform="espressif8266") + assert c.data["build"]["flags"] == ["-lespressif8266"]