diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index 8260f17c52..a57bc7c5a3 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -17,6 +17,7 @@ from dataclasses import dataclass, field import functools import logging from pathlib import Path +import re from esphome.core import CORE, EsphomeError, Library from esphome.helpers import walk_files @@ -63,16 +64,15 @@ class ArduinoLibrary: link_flags: list[str] = field(default_factory=list) +# Filename-plain names only: leading alnum/underscore, then word chars, +# dot, space, plus, or hyphen. An allowlist excludes separators, drive +# colons, and dot-only names by shape instead of enumerating them. +_SAFE_LIBRARY_NAME_RE = re.compile(r"[A-Za-z0-9_][A-Za-z0-9_. +-]*\Z") + + def _is_safe_library_name(name: object) -> bool: """Whether a name may be joined under the framework's libraries dir.""" - return ( - isinstance(name, str) - and bool(name) - and "/" not in name - and "\\" not in name - and ":" not in name # a Windows drive-relative name escapes the tree - and name not in (".", "..") - ) + return isinstance(name, str) and _SAFE_LIBRARY_NAME_RE.fullmatch(name) is not None def _manifest_build(name: str, data: object) -> dict: @@ -84,24 +84,21 @@ def _manifest_build(name: str, data: object) -> dict: return build -def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: - """Resolve one library's sources, include dirs, and flags (PIO semantics).""" - build = _manifest_build(name, data) +def _resolve_src_dir(name: str, read_path: Path, build: dict) -> str: + """Resolve PIO's source dir: manifest srcDir, else src/Src, else the root.""" + if "srcDir" not in build: + return next((d for d in ("src", "Src") if (read_path / d).is_dir()), ".") + # A declared srcDir (falsy included) that does not resolve is a + # manifest error + src_dir = build["srcDir"] + if not (isinstance(src_dir, str) and src_dir and (read_path / src_dir).is_dir()): + raise EsphomeError( + f"Library {name} declares srcDir {src_dir!r} which does not exist" + ) + return src_dir - # PIO's source-dir resolution: manifest srcDir, else src/Src, else the root - if "srcDir" in build: - # A declared srcDir (falsy included) that does not resolve is a - # manifest error - src_dir = build["srcDir"] - if not ( - isinstance(src_dir, str) and src_dir and (read_path / src_dir).is_dir() - ): - raise EsphomeError( - f"Library {name} declares srcDir {src_dir!r} which does not exist" - ) - else: - src_dir = next((d for d in ("src", "Src") if (read_path / d).is_dir()), ".") +def _warn_dropped_link_fields(name: str, data: dict) -> None: for dropped_key in ("precompiled", "ldflags"): if data.get(dropped_key): # PIO's Arduino lib builder honors these; building without them @@ -111,15 +108,14 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: name, dropped_key, ) - src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) - if not all(isinstance(entry, str) for entry in src_filter): - raise EsphomeError(f"Library {name} has a malformed srcFilter") - # PlatformIO shell-lexes each build.flags entry - flag_tokens = lex_build_flags(build.get("flags", []), f"library {name}") - # dot_a_linkage (Arduino IDE's property, ignored by PIO) is a deliberate - # extra. Strict parse: bool("false") is True. - def _parse_archive(key: str, raw: object) -> bool: + +def _resolve_lib_archive(name: str, data: dict, build: dict) -> bool: + """build.libArchive, else dot_a_linkage (Arduino IDE's property, ignored + by PIO -- a deliberate extra), else archive.""" + + # Strict parse: bool("false") is True + def _parse(key: str, raw: object) -> bool: if isinstance(raw, bool): return raw value = str(raw).strip().lower() @@ -128,12 +124,19 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: raise EsphomeError(f"Library {name} has a malformed {key} value {raw!r}") if "libArchive" in build: - lib_archive = _parse_archive("libArchive", build["libArchive"]) - elif "dot_a_linkage" in data: - lib_archive = _parse_archive("dot_a_linkage", data["dot_a_linkage"]) - else: - lib_archive = True - lib = ArduinoLibrary(name=name, lib_archive=lib_archive) + return _parse("libArchive", build["libArchive"]) + if "dot_a_linkage" in data: + return _parse("dot_a_linkage", data["dot_a_linkage"]) + return True + + +def _classify_build_flags( + name: str, read_path: Path, lib: ArduinoLibrary, flag_tokens: list[str] +) -> list[str]: + """Route the lexed build.flags into the library's flag lists. + + Returns the ``-I`` arguments for the include-dir resolution. + """ include_flags: list[str] = [] for tok in flag_tokens: if tok.startswith("-I"): @@ -155,13 +158,23 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: lib.link_flags.append(tok) else: lib.flags.append(tok) + return include_flags + +def _resolve_include_dirs( + name: str, + read_path: Path, + lib: ArduinoLibrary, + build: dict, + src_dir: str, + include_flags: list[str], +) -> None: include_dir = build.get("includeDir", DEFAULT_BUILD_INCLUDE_DIR) if not isinstance(include_dir, str): raise EsphomeError(f"Library {name} has a malformed includeDir") for d, explicit in [ (include_dir, "includeDir" in build), - (src_dir, False), # the srcDir guard above already validated it + (src_dir, False), # _resolve_src_dir already validated it *((flag, True) for flag in include_flags), ]: if (path := (read_path / d)).is_dir(): @@ -174,6 +187,15 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: "Library %s declares include dir %s which does not exist", name, d ) + +def _collect_lib_sources( + name: str, + read_path: Path, + lib: ArduinoLibrary, + build: dict, + src_dir: str, + src_filter: list[str], +) -> None: matched = collect_filtered_files(read_path / src_dir, src_filter) lib.sources = sorted( path.resolve() @@ -204,6 +226,23 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: "Library %s declares srcFilter/srcDir but no source files matched", name, ) + + +def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: + """Resolve one library's sources, include dirs, and flags (PIO semantics).""" + build = _manifest_build(name, data) + _warn_dropped_link_fields(name, data) + src_dir = _resolve_src_dir(name, read_path, build) + src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) + if not all(isinstance(entry, str) for entry in src_filter): + raise EsphomeError(f"Library {name} has a malformed srcFilter") + lib = ArduinoLibrary(name=name, lib_archive=_resolve_lib_archive(name, data, build)) + # PlatformIO shell-lexes each build.flags entry + include_flags = _classify_build_flags( + name, read_path, lib, lex_build_flags(build.get("flags", []), f"library {name}") + ) + _resolve_include_dirs(name, read_path, lib, build, src_dir, include_flags) + _collect_lib_sources(name, read_path, lib, build, src_dir, src_filter) return lib diff --git a/esphome/build_gen/build_tool.py b/esphome/build_gen/build_tool.py index c19c4dc220..804b623d9d 100644 --- a/esphome/build_gen/build_tool.py +++ b/esphome/build_gen/build_tool.py @@ -17,50 +17,62 @@ import subprocess import sys +def _read_rspfile(rspfile: str) -> list[str]: + r"""The object paths listed in ``rspfile``, unquoted. + + GNU ar treats backslashes in response files as escapes (corrupts + Windows paths), so the caller expands the list into argv; strip the + simple surrounding quote ninja adds to special paths, then undo + ninja's POSIX escape for an embedded quote ('a'\\''b.o' -> a'b.o). + """ + return [ + line[1:-1].replace("'\\''", "'") + if len(line) >= 2 and line[0] == line[-1] and line[0] in "'\"" + else line + for line in Path(rspfile).read_text(encoding="utf-8").splitlines() + if line + ] + + +def _run_ar(ar: str, archive: str, rspfile: str) -> int: + # Remove first: ``ar rc`` replaces members but never drops ones whose + # source was removed from the build, which would leak stale objects. + Path(archive).unlink(missing_ok=True) + objects = _read_rspfile(rspfile) + if not objects: + # An empty archive would "succeed" here and fail far away at link + print(f"ar: no objects listed in {rspfile} for {archive}", file=sys.stderr) + return 1 + # Batch by argv length: expanding the rspfile gives back the Windows + # 32767-char command-line limit it existed to avoid. "rc" creates, + # "q" appends the remainder. + op = "rc" + while objects: + batch = [objects.pop(0)] + batch_len = len(batch[0]) + while objects and batch_len + len(objects[0]) < 25000: + batch_len += len(objects[0]) + 1 + batch.append(objects.pop(0)) + rc = subprocess.run( + [ar, op, archive, *batch], check=False, close_fds=False + ).returncode + if rc != 0: + return rc + op = "q" + return 0 + + +def _run_copy(src: str, dst: str) -> int: + shutil.copyfile(src, dst) + return 0 + + def main() -> int: mode = sys.argv[1] if mode == "ar": - ar, archive, rspfile = sys.argv[2:5] - # Remove first: ``ar rc`` replaces members but never drops ones whose - # source was removed from the build, which would leak stale objects. - Path(archive).unlink(missing_ok=True) - # GNU ar treats backslashes in response files as escapes (corrupts - # Windows paths), so expand the rspfile into argv, stripping the - # simple surrounding quote ninja adds to special paths. - # After stripping the outer pair, undo ninja's POSIX escape for an - # embedded quote ('a'\''b.o' -> a'b.o) - objects = [ - line[1:-1].replace("'\\''", "'") - if len(line) >= 2 and line[0] == line[-1] and line[0] in "'\"" - else line - for line in Path(rspfile).read_text(encoding="utf-8").splitlines() - if line - ] - if not objects: - # An empty archive would "succeed" here and fail far away at link - print(f"ar: no objects listed in {rspfile} for {archive}", file=sys.stderr) - return 1 - # Batch by argv length: expanding the rspfile gives back the Windows - # 32767-char command-line limit it existed to avoid. "rc" creates, - # "q" appends the remainder. - op = "rc" - while objects: - batch = [objects.pop(0)] - batch_len = len(batch[0]) - while objects and batch_len + len(objects[0]) < 25000: - batch_len += len(objects[0]) + 1 - batch.append(objects.pop(0)) - rc = subprocess.run( - [ar, op, archive, *batch], check=False, close_fds=False - ).returncode - if rc != 0: - return rc - op = "q" - return 0 + return _run_ar(*sys.argv[2:5]) if mode == "copy": - src, dst = sys.argv[2:4] - shutil.copyfile(src, dst) - return 0 + return _run_copy(*sys.argv[2:4]) print(f"unknown build_tool mode: {mode}", file=sys.stderr) return 1 diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 8804e5028d..dfceaabf1c 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -11,6 +11,10 @@ from esphome.framework_helpers import strip_win_long_path_prefix, tool_version_r _LOGGER = logging.getLogger(__name__) +# esphome cv.boolean's spelling tables plus the 1/0 env convention +TRUTHY_ENV_STRINGS = frozenset({"1", "true", "yes", "on", "enable"}) +FALSY_ENV_STRINGS = frozenset({"0", "false", "no", "off", "disable"}) + def _ccache_runs(ccache: str) -> bool: """Return True when the ``ccache`` found on PATH actually runs.""" @@ -31,9 +35,9 @@ def parse_enable_env(name: str) -> bool | None: if raw is None: return None lowered = raw.strip().lower() - if lowered in ("1", "true", "yes", "on"): + if lowered in TRUTHY_ENV_STRINGS: return True - if lowered in ("0", "false", "no", "off"): + if lowered in FALSY_ENV_STRINGS: return False _LOGGER.warning("Ignoring unrecognized %s=%r; use 1 or 0", name, raw) return None diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 6c0b0d09cf..8c683ba540 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -859,6 +859,42 @@ def is_lib_ignored(name: str | None, lib_ignore: set[str]) -> bool: ) +def _warn_unsatisfied_versionless( + skipped_versionless: list[tuple[Any, Any, str]], + components: dict[str, ConvertedLibrary], + backend: LibraryBackend, +) -> None: + """Warn for version-less deps nothing satisfied (request key, manifest + name, or backend provides()); a silent drop surfaces as link errors far + from the cause.""" + resolved_manifest_names = {c.data.get("name") for c in components.values()} + warned: set[str] = set() + for dep_name, dep_owner, requester in skipped_versionless: + if not isinstance(dep_name, str) or not dep_name or dep_name in warned: + continue + if dep_name in components: + # A version-less dep's request key is the name itself + continue + if dep_name in resolved_manifest_names: + continue + if ( + not dep_owner + and backend.provides is not None + and backend.provides(dep_name) + ): + # provides() only satisfies owner-less names: the walk's + # backend-provided skip has the same owner guard, so an + # owner-qualified version-less dep was added by nobody + continue + warned.add(dep_name) + _LOGGER.warning( + "Dependency %s of %s has no version to resolve and nothing " + "provides it; skipping", + dep_name, + requester, + ) + + def convert_libraries( libraries: list[Library], backend: LibraryBackend ) -> list[ConvertedLibrary]: @@ -1161,34 +1197,6 @@ def convert_libraries( for component in components.values(): backend.emit(component) - # Warn for version-less deps nothing satisfied (request key, manifest - # name, or backend provides()); a silent drop surfaces as link errors - # far from the cause. - resolved_manifest_names = {c.data.get("name") for c in components.values()} - warned: set[str] = set() - for dep_name, dep_owner, requester in skipped_versionless: - if not isinstance(dep_name, str) or not dep_name or dep_name in warned: - continue - if dep_name in components: - # A version-less dep's request key is the name itself - continue - if dep_name in resolved_manifest_names: - continue - if ( - not dep_owner - and backend.provides is not None - and backend.provides(dep_name) - ): - # provides() only satisfies owner-less names: the walk's - # backend-provided skip has the same owner guard, so an - # owner-qualified version-less dep was added by nobody - continue - warned.add(dep_name) - _LOGGER.warning( - "Dependency %s of %s has no version to resolve and nothing " - "provides it; skipping", - dep_name, - requester, - ) + _warn_unsatisfied_versionless(skipped_versionless, components, backend) return [components[key] for key in top_level if key in components] diff --git a/tests/unit_tests/build_helpers/test_ccache.py b/tests/unit_tests/build_helpers/test_ccache.py index c612f79554..619a1a3476 100644 --- a/tests/unit_tests/build_helpers/test_ccache.py +++ b/tests/unit_tests/build_helpers/test_ccache.py @@ -97,3 +97,23 @@ def test_resolve_unrecognized_value_warns_and_probes( assert ccache.resolve_ccache_path() is None mock_probe.assert_called_once() assert "unrecognized ESPHOME_CCACHE_ENABLE" in caplog.text + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("1", True), + ("enable", True), + ("ON", True), + ("0", False), + ("disable", False), + ("Off", False), + ("maybe", None), + ], +) +def test_parse_enable_env_spelling_tables( + monkeypatch: pytest.MonkeyPatch, raw: str, expected: bool | None +) -> None: + """cv.boolean's spelling tables plus the 1/0 env convention.""" + monkeypatch.setenv("ESPHOME_CCACHE_ENABLE", raw) + assert ccache.parse_enable_env("ESPHOME_CCACHE_ENABLE") is expected diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index f46f4e4fa2..50c2a14405 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -603,30 +603,6 @@ def test_lex_build_flags_dangling_flag_does_not_cross_entries( assert "Ignoring trailing '-I'" in caplog.text -def test_split_flag_entry_non_string_is_clean() -> None: - """A dict or number from a third-party manifest fails naming the entry, - not with an opaque shlex traceback.""" - - with pytest.raises(EsphomeError, match="Malformed build flag"): - split_flag_entry({"esp32": ["-DX"]}, "lib x") - with pytest.raises(EsphomeError, match="Malformed build flag 5"): - split_flag_entry(5, "lib x") - - -def test_source_kind_map_shape() -> None: - """The kind values the native compile rules key on, and the deliberate - AS/ASPP merge (.s and .S both map to asm).""" - - assert set(SOURCE_KIND_FOR_SUFFIX.values()) == {"c", "cxx", "asm"} - assert SOURCE_KIND_FOR_SUFFIX[".s"] == "asm" - assert SOURCE_KIND_FOR_SUFFIX[".S"] == "asm" - assert SOURCE_KIND_FOR_SUFFIX[".c"] == "c" - assert SOURCE_KIND_FOR_SUFFIX[".cpp"] == "cxx" - # SCons's case-sensitive C++ suffixes: PIO compiles .C as C++ - assert SOURCE_KIND_FOR_SUFFIX[".C"] == "cxx" - assert SOURCE_KIND_FOR_SUFFIX[".C++"] == "cxx" - - def test_normalize_dependencies_forms(caplog) -> None: """Every PIO-legal spelling normalizes; unrecognizable entries warn.""" from esphome.platformio.library import normalize_dependencies @@ -684,6 +660,54 @@ def test_walk_warns_for_properties_only_depends( assert "declares dependencies via library.properties" in caplog.text +def test_walk_warns_for_nonplatform_invalid_library( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A dependency dropped for any cause other than the routine platform + filter is visible in every backend.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + {"esphome/A": {"name": "A", "dependencies": [{"name": "B", "version": "1.0"}]}}, + ) + calls = {"n": 0} + real = lib.check_library_data + + def flaky(data, platform, framework): + calls["n"] += 1 + if calls["n"] > 1: + raise InvalidLibrary("manifest is corrupt") + return real(data, platform, framework) + + monkeypatch.setattr(lib, "check_library_data", flaky) + convert_libraries([Library("esphome/A", None, None)], _backend()) + assert "Skipping dependency B of esphome/A: manifest is corrupt" in caplog.text + + +def test_split_flag_entry_non_string_is_clean() -> None: + """A dict or number from a third-party manifest fails naming the entry, + not with an opaque shlex traceback.""" + + with pytest.raises(EsphomeError, match="Malformed build flag"): + split_flag_entry({"esp32": ["-DX"]}, "lib x") + with pytest.raises(EsphomeError, match="Malformed build flag 5"): + split_flag_entry(5, "lib x") + + +def test_source_kind_map_shape() -> None: + """The kind values the native compile rules key on, and the deliberate + AS/ASPP merge (.s and .S both map to asm).""" + + assert set(SOURCE_KIND_FOR_SUFFIX.values()) == {"c", "cxx", "asm"} + assert SOURCE_KIND_FOR_SUFFIX[".s"] == "asm" + assert SOURCE_KIND_FOR_SUFFIX[".S"] == "asm" + assert SOURCE_KIND_FOR_SUFFIX[".c"] == "c" + assert SOURCE_KIND_FOR_SUFFIX[".cpp"] == "cxx" + # SCons's case-sensitive C++ suffixes: PIO compiles .C as C++ + assert SOURCE_KIND_FOR_SUFFIX[".C"] == "cxx" + assert SOURCE_KIND_FOR_SUFFIX[".C++"] == "cxx" + + def test_versionless_platform_filtered_dependency_stays_quiet( tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture ) -> None: @@ -743,30 +767,6 @@ def test_versionless_dependency_without_provider_warns( ) -def test_walk_warns_for_nonplatform_invalid_library( - tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture -) -> None: - """A dependency dropped for any cause other than the routine platform - filter is visible in every backend.""" - _patch_download_with_manifests( - monkeypatch, - tmp_path, - {"esphome/A": {"name": "A", "dependencies": [{"name": "B", "version": "1.0"}]}}, - ) - calls = {"n": 0} - real = lib.check_library_data - - def flaky(data, platform, framework): - calls["n"] += 1 - if calls["n"] > 1: - raise InvalidLibrary("manifest is corrupt") - return real(data, platform, framework) - - monkeypatch.setattr(lib, "check_library_data", flaky) - convert_libraries([Library("esphome/A", None, None)], _backend()) - assert "Skipping dependency B of esphome/A: manifest is corrupt" in caplog.text - - def test_versionless_owner_qualified_dependency_warns_despite_provides( tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture ) -> None: