diff --git a/esphome/components/zephyr/library.py b/esphome/components/zephyr/library.py index 0e6551ccf1..b339ae45b0 100644 --- a/esphome/components/zephyr/library.py +++ b/esphome/components/zephyr/library.py @@ -28,6 +28,7 @@ from esphome.platformio.library import ( collect_filtered_files, convert_libraries, ensure_list, + lex_build_flags, split_list_by_condition, ) @@ -80,7 +81,11 @@ def generate_cmakelists_txt(component: ConvertedLibrary) -> str: build_include_dir = build.get("includeDir", DEFAULT_BUILD_INCLUDE_DIR) build_src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) - build_flags = ensure_list(build.get("flags", DEFAULT_BUILD_FLAGS)) + # The shared lexer re-glues spaced entries and drops bare/empty + # arguments, same as the espidf emitter + build_flags = lex_build_flags( + build.get("flags", DEFAULT_BUILD_FLAGS), component.name + ) src_files = collect_filtered_files( read_path / Path(build_src_dir), build_src_filter diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index 4655d1c54d..105413cf44 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -58,8 +58,10 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: """ def escape_entry(p: PathType) -> str: - # In CMakeLists.txt, backslashes need to be escaped - return f'"{str(p)}"'.replace("\\", "\\\\") + # In CMakeLists.txt, backslashes and embedded quotes need escaping + # (a quoted define value reaches here via the shlex round-trip) + escaped = str(p).replace("\\", "\\\\").replace('"', '\\"') + return f'"{escaped}"' def escape_path(p: PathType) -> str: # CMake uses forward slashes for paths on every platform and treats diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py index e60a50d746..e04ccd1f55 100644 --- a/esphome/platformio/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -33,7 +33,7 @@ def apply_extra_script( """Run a library's ``extraScript`` and fold its captured env vars into ``build.flags``; ``board_mcu`` is a callable so it resolves lazily.""" extra_script = component.data.get("build", {}).get("extraScript") - if not extra_script: + if extra_script is None or extra_script == "": return if not isinstance(extra_script, str): # A list/dict value would raise an opaque TypeError on the join below @@ -170,6 +170,15 @@ class _FakeSConsEnv: ) return self._vars.get(key, default) + def __contains__(self, key: object) -> bool: + # Without this, "KEY" in env falls back to the legacy sequence + # protocol: __getitem__(0), (1), ... never raises, so it loops + # forever flooding the log + return key in self._vars + + def __iter__(self): + return iter(self._vars) + def __getitem__(self, key: str) -> str: # Scripts also read env["BOARD_MCU"]; an unmodelled subscript # degrades one branch instead of discarding the whole capture diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 6d26fea534..df1d6aa07b 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -656,15 +656,15 @@ def dependency_is_usable( def _valid_dependency_entry(entry: dict, manifest_name: str) -> bool: - """Whether a normalized entry carries a usable name (non-empty string) - and version (string, if present); invalid entries warn naming the - manifest.""" + """Whether a normalized entry carries a usable name (non-empty string), + version (string, if present), and owner (string, if present); invalid + entries warn naming the manifest.""" name = entry.get("name") - if ( - isinstance(name, str) - and name - and ("version" not in entry or isinstance(entry["version"], str)) - ): + owner = entry.get("owner") + name_ok = isinstance(name, str) and name + version_ok = "version" not in entry or isinstance(entry["version"], str) + owner_ok = owner is None or isinstance(owner, str) + if name_ok and version_ok and owner_ok: return True _LOGGER.warning( "Ignoring unrecognized dependency entry %r of %s", entry, manifest_name @@ -683,7 +683,7 @@ def normalize_dependencies( so callers see a uniform list. ``manifest_name`` names the manifest in the warning for entries that cannot be normalized. """ - if not dependencies: + if dependencies is None: return [] if isinstance(dependencies, str): # A plain string is one or more comma-separated names; iterating it @@ -1004,11 +1004,19 @@ def convert_libraries( f"library.properties in {source_dir}" ) - if not isinstance(component.data, dict) or not isinstance( - component.data.get("build", {}), dict - ): - # A bare json.load imposes no shape; every backend dereferences - # data/build, so validate once here and name the library + # A bare json.load imposes no shape; every backend dereferences + # these fields, so validate once here and name the library + malformed = not isinstance(component.data, dict) + if not malformed: + build = component.data.get("build", {}) + malformed = ( + not isinstance(build, dict) + or not isinstance(component.data.get(ESPHOME_DATA_KEY, {}), dict) + or not isinstance(build.get("srcDir", ""), str) + or not isinstance(build.get("includeDir", ""), str) + or not isinstance(build.get("srcFilter", ""), (str, list)) + ) + if malformed: raise EsphomeError(f"Library {key} has a malformed manifest") warn_properties_depends(component.name, component.data) @@ -1018,10 +1026,12 @@ def convert_libraries( # 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 " - f"{backend.framework}: {e}" - ) from e + reason = ( + f"is not compatible with {backend.framework}" + if isinstance(e, IncompatiblePlatform) + else "has a malformed manifest" + ) + raise RuntimeError(f"Requested library {key} {reason}: {e}") from e if isinstance(e, IncompatiblePlatform): _LOGGER.debug("Skip incompatible dependency %s: %s", key, str(e)) else: diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index 5a273b987c..0caff8174e 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -294,6 +294,19 @@ def test_generate_cmakelists_txt_multi_token_flag(tmp_component): assert ' "-include"\n "cp_custom_alloc.h"\n' in content +def test_generate_cmakelists_txt_escapes_embedded_quotes(tmp_component): + """A define value carrying a literal quote survives into CMake as an + escaped quote, not a prematurely-terminated string.""" + src_dir = tmp_component.path / "src" + src_dir.mkdir() + (src_dir / "main.c").write_text("int main() {}") + # shlex keeps the backslash-escaped quotes as literal characters + tmp_component.data = {"build": {"flags": ['-DMSG=\\"hi\\"']}} + + content = generate_cmakelists_txt(tmp_component) + assert '"-DMSG=\\"hi\\""' in content + + def test_generate_cmakelists_txt_extra_script_link_flags(tmp_component): """Captured extra-script LINKFLAGS come out as target_link_options, not compile options where they would be silently ineffective.""" diff --git a/tests/unit_tests/test_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py index 980ce29ccb..07f0e78500 100644 --- a/tests/unit_tests/test_platformio_extra_script.py +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -461,6 +461,27 @@ def test_prepend_inserts_ahead_of_existing(method: str) -> None: assert env.result.libs == ["algobsec", "bsec", "m"] +def test_env_membership_and_iteration(tmp_path) -> None: + """Membership tests and for-loops must use the mapping protocol; the + legacy sequence fallback through __getitem__ would loop forever.""" + env = _FakeSConsEnv( + board_mcu="esp8266", pio_env="esphome_esp8266", pio_platform="espressif8266" + ) + assert "BOARD_MCU" in env + assert "NOPE" not in env + assert sorted(env) == ["BOARD_MCU", "PIOENV", "PIOPLATFORM"] + + +def test_apply_extra_script_non_string_falsey_raises(tmp_path) -> None: + """A falsey non-string extraScript (false, 0, []) is a malformed + manifest, not an absent script.""" + c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy")) + c.path = tmp_path + c.data = {"build": {"extraScript": False}} + with pytest.raises(EsphomeError, match="must be a string"): + apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") + + def test_env_get_unknown_key_warns_once(caplog) -> None: """A script branching on an unmodelled env var is diagnosable.""" env = _FakeSConsEnv( diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 0f90ad3e84..1f62513b7b 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -614,10 +614,28 @@ def test_normalize_dependencies_forms(caplog) -> None: assert normalize_dependencies({"Foo": ["1.0", "2.0"]}, "libx") == [] assert normalize_dependencies([{"name": "Foo", "version": 1}], "libx") == [] assert caplog.text.count("unrecognized dependency entry") == 7 + # A non-string owner would stringify into a malformed registry name + assert ( + normalize_dependencies( + [{"name": "Foo", "owner": {"bad": 1}, "version": "1.0"}], "libx" + ) + == [] + ) + # A falsey scalar (0, false) is malformed, not an empty list + assert normalize_dependencies(0, "libx") == [] + assert "Ignoring unrecognized dependencies 0 of libx" in caplog.text @pytest.mark.parametrize( - "manifest", [["not", "a", "manifest"], {"name": "A", "build": "src"}] + "manifest", + [ + ["not", "a", "manifest"], + {"name": "A", "build": "src"}, + {"name": "A", "ESPHOME": "yes"}, + {"name": "A", "build": {"srcDir": 123}}, + {"name": "A", "build": {"includeDir": ["inc"]}}, + {"name": "A", "build": {"srcFilter": {"+": "src"}}}, + ], ) def test_convert_libraries_malformed_manifest_raises( tmp_path, monkeypatch, manifest diff --git a/tests/unit_tests/test_zephyr_library.py b/tests/unit_tests/test_zephyr_library.py index b370fe0c47..0d899ec91d 100644 --- a/tests/unit_tests/test_zephyr_library.py +++ b/tests/unit_tests/test_zephyr_library.py @@ -66,6 +66,22 @@ def test_generate_cmakelists_txt_flags_and_includes(tmp_path): assert "-lm" in out +def test_generate_cmakelists_txt_lexes_spaced_flags(tmp_path): + """A spaced -I entry routes to include dirs instead of landing verbatim + in compile options; same shared lexer as the espidf emitter.""" + c = _make_component(tmp_path) + (tmp_path / "src").mkdir() + (tmp_path / "src" / "a.c").write_text("") + (tmp_path / "include").mkdir() + c.data = {"build": {"flags": "-I include -DBAR=1"}} + + out = generate_cmakelists_txt(c) + + assert str((tmp_path / "include").resolve()).replace("\\", "\\\\") in out + assert "-DBAR=1" in out + assert "-I include" not in out + + def test_generate_zephyr_modules_collects_all_dirs_and_writes(tmp_path, monkeypatch): # Two converted libraries: one top-level, one transitive dependency. The # converter calls backend.emit for both; generate_zephyr_modules must return