Address review: cover all source suffixes, pass PIOPLATFORM through, link library -Wl flags

This commit is contained in:
J. Nick Koston
2026-08-19 23:50:16 -05:00
parent 9429243d2b
commit 66f6f5c004
6 changed files with 73 additions and 11 deletions
+5 -2
View File
@@ -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
+17 -1
View File
@@ -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]
+20 -6
View File
@@ -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:
+13 -1
View File
@@ -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
@@ -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",
+13
View File
@@ -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"]