From eb3a64829f093e86fd8d584ecfe56925aaf2ce18 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 20 Aug 2026 04:59:27 -0500 Subject: [PATCH] Shell-lex -L/-l build_flags entries and generate the compile database before the build --- esphome/arduino8266/component.py | 40 ++++++++++++------- esphome/arduino8266/toolchain.py | 12 ++++-- esphome/build_gen/arduino8266.py | 19 +++++++-- .../unit_tests/build_gen/test_arduino8266.py | 32 ++++++++++++++- .../unit_tests/test_arduino8266_toolchain.py | 4 ++ 5 files changed, 82 insertions(+), 25 deletions(-) diff --git a/esphome/arduino8266/component.py b/esphome/arduino8266/component.py index 42f2b72c77..be9c1c7ee9 100644 --- a/esphome/arduino8266/component.py +++ b/esphome/arduino8266/component.py @@ -13,6 +13,7 @@ include path. from __future__ import annotations +from collections.abc import Iterable from dataclasses import dataclass, field import logging from pathlib import Path @@ -58,6 +59,21 @@ class ArduinoLibrary: link_flags: list[str] = field(default_factory=list) +def join_flag_args(tokens: Iterable[str], owner: str) -> list[str]: + """Join a bare ``-I``/``-L``/``-l`` with its following token (PIO lexing).""" + out: list[str] = [] + it = iter(tokens) + for tok in it: + if tok in ("-I", "-L", "-l"): + arg = next(it, None) + if arg is None: + _LOGGER.warning("Ignoring trailing '%s' in %s build flags", tok, owner) + break + tok += arg + out.append(tok) + return out + + def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: """Resolve one library's sources, include dirs, and flags (PIO semantics).""" build = data.get("build", {}) @@ -74,24 +90,18 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) # PlatformIO shell-lexes each build.flags entry - raw_flags = [ - token - for entry in ensure_list(build.get("flags", [])) - for token in shlex.split(entry) - ] + raw_flags = join_flag_args( + ( + token + for entry in ensure_list(build.get("flags", [])) + for token in shlex.split(entry) + ), + f"library {name}", + ) lib = ArduinoLibrary(name=name) - it = iter(raw_flags) include_flags: list[str] = [] - for tok in it: - if tok in ("-I", "-L", "-l"): - arg = next(it, None) - if arg is None: - _LOGGER.warning( - "Ignoring trailing '%s' in library %s build flags", tok, name - ) - break - tok += arg + for tok in raw_flags: if tok.startswith("-I"): include_flags.append(tok[2:]) elif tok.startswith("-L"): diff --git a/esphome/arduino8266/toolchain.py b/esphome/arduino8266/toolchain.py index 83546a4a24..0d47063b37 100644 --- a/esphome/arduino8266/toolchain.py +++ b/esphome/arduino8266/toolchain.py @@ -65,6 +65,14 @@ def run_compile(config: ConfigType, verbose: bool) -> int: build_dir = get_build_dir() env = framework.get_build_env(paths["toolchain_path"]) + + # The compile database is a pure function of build.ninja (no compilation + # involved), so regenerate it before the build: a failed build can then + # never leave a stale database behind. Skip the ninja spawn plus MBs of + # text on unchanged builds. + if ninja_changed or not (build_dir / "compile_commands.json").is_file(): + _write_compile_commands(paths["ninja_path"], build_dir, env) + cmd = [str(paths["ninja_path"]), "-C", str(build_dir)] if verbose: cmd.append("-v") @@ -76,10 +84,6 @@ def run_compile(config: ConfigType, verbose: bool) -> int: if rc != 0: return rc - # The compile database is a pure function of build.ninja; skip its - # regeneration (a ninja spawn plus MBs of text) on unchanged builds. - if ninja_changed or not (build_dir / "compile_commands.json").is_file(): - _write_compile_commands(paths["ninja_path"], build_dir, env) _print_size_summary(build_dir) get_idedata() return 0 diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 3b5e921593..ba5f4ee1ea 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -18,9 +18,11 @@ from dataclasses import dataclass, field import os from pathlib import Path import re +import shlex import subprocess import sys +from esphome.arduino8266.component import join_flag_args from esphome.components.esp8266 import build_surgery from esphome.components.esp8266.boards import ( BOARDS, @@ -308,10 +310,19 @@ def _project_flags() -> tuple[list[str], list[str], list[Path], list[str]]: for flag in flags: if flag.startswith("-Wl,"): link_flags.append(flag) - elif flag.startswith("-L") and len(flag) > 2: - lib_dirs.append(Path(flag[2:])) - elif flag.startswith("-l") and len(flag) > 2: - libs.append(flag[2:]) + elif flag.startswith(("-L", "-l")): + # Shell-lex only linker entries so forms like "-L /opt/blobs" + # work as they do under PlatformIO. Other entries pass verbatim: + # lexing them would strip the quotes in defines like -DBOARD="...". + for tok in join_flag_args(shlex.split(flag), "esphome"): + if tok.startswith("-L") and len(tok) > 2: + lib_dirs.append(Path(tok[2:])) + elif tok.startswith("-l") and len(tok) > 2: + libs.append(tok[2:]) + elif tok.startswith("-Wl,"): + link_flags.append(tok) + else: + compile_flags.append(tok) else: compile_flags.append(flag) return compile_flags, link_flags, lib_dirs, libs diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index 813743c7fa..2b66b36138 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -224,6 +224,7 @@ def test_write_project_link_line_and_exclusions(tmp_path: Path) -> None: "-Wno-nonnull-compare", "-L/opt/blobs", "-luser_blob", + "-L /spc/blobs -l spaced_blob", ) content = _write_ninja(paths) @@ -256,17 +257,23 @@ def test_write_project_link_line_and_exclusions(tmp_path: Path) -> None: assert "--app $in --flash_mode" in content assert '"$in"' not in content assert '"$out"' not in content - # -L/-l from esphome build_flags reach the link line, not the compiles + # -L/-l from esphome build_flags reach the link line, not the compiles; + # spaced forms ("-L /path") are shell-lexed the way PlatformIO does assert '-L"/opt/blobs"' in content assert "-luser_blob" in content + assert '-L"/spc/blobs"' in content + assert "-lspaced_blob" in content for line in content.splitlines(): if line.split(" = ")[0] in ("cflags", "cxxflags", "asflags"): assert "user_blob" not in line assert "/opt/blobs" not in line + assert "spaced_blob" not in line + assert "/spc/blobs" not in line # System libraries with the selected lwIP variant, in the builder's order assert ( "-lhal -lphy -lpp -lnet80211 -llwip2-1460 -lwpa -lcrypto -lmain -lwps " - "-lbearssl -lespnow -lsmartconfig -lairkiss -lwpa2 -luser_blob " + "-lbearssl -lespnow -lsmartconfig -lairkiss -lwpa2 -lspaced_blob " + "-luser_blob " "-lstdc++ -lm -lc -lgcc" in content ) # Core exclusions: native OTA backend and waveform stubs @@ -560,3 +567,24 @@ def test_write_project_build_unflags_apply_to_framework_flags(tmp_path: Path) -> assert "-fipa-pta" not in line if key == "linkflags": assert "-Wl,--gc-sections" not in line + + +def test_project_flags_trailing_bare_linker_flag_warns( + caplog: pytest.LogCaptureFixture, +) -> None: + _set_flags("-l") + compile_flags, link_flags, lib_dirs, libs = arduino8266._project_flags() + assert "Ignoring trailing '-l'" in caplog.text + assert not libs + assert not lib_dirs + assert "-l" not in compile_flags + assert "-l" not in link_flags + + +def test_project_flags_lexed_entry_scatters_non_linker_tokens() -> None: + _set_flags("-L /d -Wl,-Map=m stray") + compile_flags, link_flags, lib_dirs, libs = arduino8266._project_flags() + assert lib_dirs == [Path("/d")] + assert link_flags == ["-Wl,-Map=m"] + assert "stray" in compile_flags + assert not libs diff --git a/tests/unit_tests/test_arduino8266_toolchain.py b/tests/unit_tests/test_arduino8266_toolchain.py index 82db24e4ff..1a713e6a57 100644 --- a/tests/unit_tests/test_arduino8266_toolchain.py +++ b/tests/unit_tests/test_arduino8266_toolchain.py @@ -66,10 +66,14 @@ def test_run_compile_build_failure(tmp_path: Path) -> None: patch.object( toolchain.subprocess, "run", return_value=MagicMock(returncode=2) ) as mock_run, + patch.object(toolchain, "_write_compile_commands") as mock_compdb, ): assert toolchain.run_compile({CONF_ESPHOME: {}}, verbose=True) == 2 cmd = mock_run.call_args[0][0] assert "-v" in cmd + # The compile database is generated before the build runs, so a failed + # build cannot leave a stale database behind. + mock_compdb.assert_called_once() def test_run_compile_success(tmp_path: Path) -> None: