diff --git a/esphome/__main__.py b/esphome/__main__.py index c8990f07ca..4149428342 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -762,10 +762,8 @@ def _wrap_to_code(name, comp, yaml_util): async def wrapped(conf): cg.add(cg.LineComment(f"{name}:")) if comp.config_schema is not None: - # sort_keys: voluptuous fills schema defaults in set-iteration - # order, so the validated dict's key order changes with the - # process hash seed; an unsorted dump would churn main.cpp and - # relink the firmware on every run + # sort_keys: voluptuous fills defaults in set order, so an + # unsorted dump would churn main.cpp and relink every run conf_str = yaml_util.dump(conf, sort_keys=True) conf_str = conf_str.replace("//", "") # remove tailing \ to avoid multi-line comment warning diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 00a890e087..e81c5ccc1a 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -150,9 +150,7 @@ def parse_entry( raw = os.path.normpath(directory / raw) return raw.replace("\\", "/") - # A launcher-wrapped command ("ccache g++ ...") names the compiler - # second. The caller passes the exact launcher it configured into the - # build, so this is a comparison, not a guess by name. + # A launcher-wrapped command ("ccache g++ ...") names the compiler second if launcher is not None and tokens[0] == launcher: tokens = tokens[1:] if _is_launcher(tokens[0]) and len(tokens) > 1 and not tokens[1].startswith("-"): @@ -268,10 +266,8 @@ def load_or_build_idedata( # look like unexplained slow builds _LOGGER.warning("Discarding unreadable idedata cache %s: %s", cache, err) else: - # Caches written before cc_path was emitted stay newer than - # compile_commands.json forever, so rebuild them on the field rather - # than on the timestamp. Check the type too: a corrupted cache can - # still be valid JSON, and "in" would match a substring of a string. + # Rebuild pre-cc_path caches on the field, not the timestamp; + # the type check keeps "in" from substring-matching a string if isinstance(cached, dict) and "cc_path" in cached: return cached diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py index e311e8729b..4c4c716009 100644 --- a/esphome/platformio/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -1,29 +1,9 @@ -"""Run a PlatformIO ``extraScript`` against a captured SCons-env stand-in. +"""Run a PlatformIO library ``extraScript`` against a fake SCons env. -PlatformIO libraries occasionally configure per-target link/build state -via a Python ``extraScript`` declared in ``library.json``'s ``build`` -section instead of static fields. The script runs under SCons during -PIO's build and mutates the active ``Environment`` (``env.Append``, -``env.Replace``, …) — chiefly to set ``LIBPATH``/``LIBS`` per chip MCU. - -ESPHome's PIO→IDF converter doesn't run SCons, so these scripts were -previously ignored and any library -relying on them failed to link under ``toolchain: esp-idf``. This -module provides a small shim that ``exec``s an extra-script with a -fake ``env`` object, captures the common ``env.Append(...)`` calls, -and returns the captured vars so the caller can fold them back into -the library's generated CMakeLists. - -Caveats -------- -* Only the ``env.Append`` API is captured. ``env.Replace``, - ``env.Prepend``, ``env.AddPreAction``, SCons file generators, and any - arbitrary I/O are no-ops, logged once per method. Scripts that depend - on those will produce incomplete output. -* Running arbitrary Python from third-party libraries is a non-trivial - trust decision. The shim does no sandboxing — anything in the - script's process can run. Use only with libraries whose source you - trust. +The shim execs the script with a stand-in ``env``, captures ``env.Append`` +calls (everything else is a logged no-op), and folds the result into the +library's build flags. No sandboxing: the script runs with full process +access, so it carries the same trust as the library's own source. """ from __future__ import annotations @@ -48,13 +28,10 @@ def apply_extra_script( board_mcu: Callable[[], str], pio_platform: str, ) -> 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 - picks them up. Shared by the ESP-IDF and ESP8266 Arduino backends. + """Run a library's ``extraScript`` and fold its captured env vars into + ``component.data["build"]["flags"]``. - ``board_mcu`` is 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``. + ``board_mcu`` is a callable so its lookup runs only when a script will. """ extra_script = component.data.get("build", {}).get("extraScript") if not extra_script: @@ -115,13 +92,8 @@ class ExtraScriptResult: class _FakeSConsEnv: - """Minimal stand-in for SCons ``Environment`` exposed to extra-scripts. - - Implements just enough surface area to let scripts query ``BOARD_MCU`` - / ``PIOENV`` and call ``env.Append(LIBPATH=…, LIBS=…, …)``. Every - other env method swallows silently so unrelated calls don't raise - ``AttributeError`` and abort the script. - """ + """Minimal SCons ``Environment`` stand-in: ``get`` and ``Append`` work; + every other method is a swallowed no-op so scripts don't abort.""" def __init__(self, *, board_mcu: str, pio_env: str, pio_platform: str) -> None: self._vars: dict[str, str] = { @@ -177,17 +149,9 @@ def run_extra_script( ) -> ExtraScriptResult: """Execute ``script_path`` with a fake SCons env and return captured vars. - ``board_mcu`` is the active MCU name (e.g. ``esp32``, - ``esp32s3``); it's exposed to the script as PlatformIO's - ``BOARD_MCU`` so chip-conditional logic resolves the same way it - would under PIO. The script runs with ``library_dir`` as the - process CWD so relative-path lookups (``join``, ``realpath``, - ``open``) resolve against the library tree. - - On any exception inside the script we warn and return an empty result - (never a partial capture, which could build wrong-output firmware) — - extra-scripts are best-effort, and an unsupported script shouldn't - block the build. + Runs with ``library_dir`` as CWD so relative lookups resolve against + the library tree. A crashed script warns and returns an empty result, + never a partial capture. """ env = _FakeSConsEnv( board_mcu=board_mcu, @@ -254,20 +218,16 @@ def run_extra_script( def captured_as_build_flags( result: ExtraScriptResult, *, library_dir: Path ) -> list[str]: - """Translate captured env vars into the ``-L`` / ``-l`` / ``-D`` / - raw-flag form ``_generate_cmakelists_txt`` already knows how to consume. + """Translate captured env vars into -L/-l/-D/raw build flags. ``LIBPATH`` entries are made relative to ``library_dir`` so the - generated CMakeLists is portable; absolute paths outside the library - tree are kept as-is (CMake handles absolute paths in - ``target_link_directories`` fine). + generated build files stay portable. """ flags: list[str] = [] library_root = library_dir.resolve() for path in result.libpath: - # Anchor relative paths to library_dir (not the current CWD, which - # has been restored by the time we get here). Joining an absolute - # path against library_dir returns the absolute path unchanged. + # Anchor relative paths to library_dir; the script's CWD has been + # restored by now resolved = (library_dir / path).resolve() try: flags.append(f"-L{resolved.relative_to(library_root)}") diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 3cc9296923..9c839c5d23 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -574,9 +574,7 @@ def _resolve_registry_version( def split_flag_entry(entry: Any, owner: str) -> list[str]: """``shlex.split`` with a clean error naming the offending flags entry.""" - # Late import: this module loads with the esp32 platform on every - # validate/compile; shlex (and its linecache pull-in) is only needed - # when actually lexing flags + # Late import: shlex is only needed when actually lexing flags import shlex try: @@ -588,13 +586,8 @@ def split_flag_entry(entry: Any, owner: str) -> list[str]: def lex_build_flags(entries: str | list[str], owner: str) -> list[str]: - """Shell-lex a manifest ``build.flags`` list into joined tokens. - - Each entry is lexed the way PlatformIO's ParseFlags does, and bare - ``-I``/``-L``/``-l``/``-D`` tokens re-glue to their argument across the - whole stream. Used by the espidf and arduino backends; zephyr still - classifies raw entries. - """ + """Shell-lex ``build.flags`` entries the way PlatformIO's ParseFlags + does; bare -I/-L/-l/-D tokens re-glue to their argument.""" # Join per entry, as SCons's ParseFlags lexes each string independently: # a dangling -I ending one entry must warn, not absorb the next entry's # first token. @@ -998,11 +991,8 @@ def convert_libraries( key = worklist.popleft() node = nodes[key] - # A node is queued once per referring edge; skip the (uncached) - # registry lookup + download + dependency walk unless its - # requirement set grew since the last resolve. Requirements only - # ever grow, so this still converges the fixpoint and terminates - # dependency cycles. + # Re-resolve only when the requirement set grew; requirements + # only ever grow, so the fixpoint converges and cycles terminate requirements = frozenset(node.requirements) if resolved_requirements.get(key) == requirements: continue @@ -1031,11 +1021,8 @@ def convert_libraries( has_json = library_json_path.is_file() has_properties = library_properties_path.is_file() if not has_json and not has_properties and not node.is_local: - # The shared cache can hold a broken copy (e.g. a clone or an - # extraction interrupted by a killed process). Force one - # re-download so a bad cache entry self-heals instead of failing - # every build until the user runs a full clean. A local source is - # read in place, so there is nothing to re-download. + # An interrupted clone/extraction self-heals with one forced + # re-download; a local source has nothing to re-download _LOGGER.warning( "Library %s at %s is missing library.json and library.properties; " "re-downloading", @@ -1050,10 +1037,8 @@ def convert_libraries( elif has_properties: component.data = parse_library_properties(library_properties_path) else: - # For a local library a missing manifest is user input, so raise - # EsphomeError (clean CLI message) like the missing-directory case; - # for registry/git a missing manifest means a corrupt cache, which - # is not user error, so keep RuntimeError. + # Local sources are user input (EsphomeError); a registry/git + # miss means a corrupt cache (RuntimeError) error_cls = EsphomeError if node.is_local else RuntimeError raise error_cls( f"Invalid PIO library {key}: missing library.json and " @@ -1071,10 +1056,8 @@ def convert_libraries( try: check_library_data(component.data, backend.platform, backend.framework) except InvalidLibrary as e: - # Fail fast if a top-level library the build explicitly requested - # is incompatible; the routine cross-platform skip stays at - # debug, any other cause warns (a silent drop resurfaces as - # undefined symbols at link) + # An explicitly requested library fails fast; the routine + # cross-platform skip stays at debug, other causes warn if key in top_level_keys: raise RuntimeError( f"Requested library {key} is not compatible with "