diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py index f29cf32951..bdef357671 100644 --- a/esphome/platformio/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -127,9 +127,14 @@ class _FakeSConsEnv: return self._vars.get(key, default) def __getitem__(self, key: str) -> str: - # Scripts also read env["BOARD_MCU"]; without this the broad - # handler would discard every flag the script captured - return self._vars[key] + # Scripts also read env["BOARD_MCU"]; an unmodelled subscript + # degrades one branch instead of discarding the whole capture + if key not in self._vars and key not in self._warned_gets: + self._warned_gets.add(key) + _LOGGER.warning( + "PIO extra-script env[%r] is not modelled; returning ''", key + ) + return self._vars.get(key, "") def Append(self, **kwargs) -> None: # noqa: N802 (SCons API name) for key, value in kwargs.items(): @@ -155,14 +160,18 @@ class _FakeSConsEnv: # ----- Everything else is a no-op so unsupported scripts don't crash ----- def __getattr__(self, name: str): + if name.startswith("__") and name.endswith("__"): + # Protocol probes (copy, pickle, iteration) are not script calls + raise AttributeError(name) + if name not in self._warned_methods: + # Warn on access, not call: hasattr()/truthiness branches would + # otherwise silently take the wrong path; a script whose whole + # effect is env.Replace() stays diagnosable either way + self._warned_methods.add(name) + _LOGGER.warning("PIO extra-script env.%s is not supported; ignoring", name) + def _noop(*args, **kwargs): - # Once per method: a script whose whole effect is env.Replace() - # must be diagnosable from a normal build log - if name not in self._warned_methods: - self._warned_methods.add(name) - _LOGGER.warning( - "PIO extra-script env.%s(...) is not supported; ignoring", name - ) + return None return _noop @@ -278,10 +287,14 @@ def captured_as_build_flags( for define in result.cppdefines: # SCons also accepts dict/list CPPDEFINES; formatting those blind # would hand the compiler garbage like -D{'FOO': '1'} - if isinstance(define, (tuple, list)) and len(define) == 2: - flags.append(f"-D{define[0]}={define[1]}") + if ( + isinstance(define, (tuple, list)) + and len(define) == 2 + and all(isinstance(part, (str, int)) for part in define) + ): + flags.append(shlex.quote(f"-D{define[0]}={define[1]}")) elif isinstance(define, str): - flags.append(f"-D{define}") + flags.append(shlex.quote(f"-D{define}")) else: _LOGGER.warning("Ignoring unsupported CPPDEFINES entry %r", define) # Each captured entry is one argv token in SCons; quote so the diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 3d2d2965f7..9635236d8e 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -475,6 +475,9 @@ def check_library_data(data: dict, platform: str | None, framework: str | None): if isinstance(platforms, str): platforms = [a.strip() for a in platforms.split(",")] platforms = ensure_list(platforms) + if not all(isinstance(pf, str) for pf in platforms): + # A real (non-platform) manifest problem; callers warn, not skip + raise InvalidLibrary(f"Malformed platforms value: {platforms!r}") # Check if library supports the target platform valid_platforms = platform is None or "*" in platforms or platform in platforms @@ -486,6 +489,8 @@ def check_library_data(data: dict, platform: str | None, framework: str | None): if isinstance(frameworks, str): frameworks = [a.strip() for a in frameworks.split(",")] frameworks = ensure_list(frameworks) + if not all(isinstance(fw, str) for fw in frameworks): + raise InvalidLibrary(f"Malformed frameworks value: {frameworks!r}") # Check if library declares the active framework. PIO library manifests # often list only "arduino" even when the library actually compiles fine diff --git a/tests/unit_tests/test_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py index a4eeb5bf41..e2484408eb 100644 --- a/tests/unit_tests/test_platformio_extra_script.py +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -270,7 +270,7 @@ def test_apply_extra_script_ignores_uncaptured_env_calls(tmp_path, caplog) -> No apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") assert c.data["build"]["flags"] == ["-lsingle"] assert "env.Append(UNCAPTURED=...) is not captured" in caplog.text - assert "env.Replace(...) is not supported" in caplog.text + assert "env.Replace is not supported" in caplog.text def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None: @@ -378,7 +378,7 @@ def test_unsupported_env_method_warns_once(caplog) -> None: ) env.Replace(CC="clang") env.Replace(CC="gcc") - assert caplog.text.count("env.Replace(...) is not supported") == 1 + assert caplog.text.count("env.Replace is not supported") == 1 def test_run_extra_script_sys_exit_is_best_effort(tmp_path, caplog) -> None: @@ -458,10 +458,14 @@ def test_env_get_unknown_key_warns_once(caplog) -> None: def test_spaced_linkflag_survives_relexing(tmp_path) -> None: """A captured argv token with a space stays one token after lexing.""" result = ExtraScriptResult( - linkflags=["-Wl,-T my linker.ld"], cppflags=["-include my hdr.h"] + linkflags=["-Wl,-T my linker.ld"], + cppflags=["-include my hdr.h"], + cppdefines=[("MSG", '"hello world"'), "PLAIN"], ) flags = captured_as_build_flags(result, library_dir=tmp_path) assert lex_build_flags(flags, "test") == [ + '-DMSG="hello world"', + "-DPLAIN", "-Wl,-T my linker.ld", "-include my hdr.h", ] diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 14189883a1..a4aa79b584 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -603,36 +603,6 @@ def test_convert_libraries_skips_incompatible_dependency(tmp_path, monkeypatch): assert top[0].dependencies == [] -def test_convert_libraries_warns_for_nonplatform_invalid_dependency_component( - tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture -) -> None: - """A dependency component dropped for any cause other than the platform - filter warns; only the routine cross-platform skip stays at debug.""" - _patch_download_with_manifests( - monkeypatch, - tmp_path, - { - "esphome/A": { - "name": "A", - "dependencies": [{"name": "C", "owner": "esphome", "version": "1.0"}], - }, - "esphome/C": {"name": "C"}, - }, - ) - real = lib.check_library_data - - def flaky(data, platform, framework): - # Fail only on C's resolved manifest, not on A's dependency entry - if data.get("name") == "C" and "version" not in data: - raise InvalidLibrary("manifest is corrupt") - return real(data, platform, framework) - - monkeypatch.setattr(lib, "check_library_data", flaky) - convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) - assert "manifest is corrupt" in caplog.text - assert "Skipping dependency" in caplog.text - - def test_split_flag_entry_unbalanced_quote_is_clean() -> None: """A malformed flags entry raises EsphomeError, not a raw ValueError.""" @@ -886,20 +856,36 @@ def test_walk_warns_for_nonplatform_invalid_library( _patch_download_with_manifests( monkeypatch, tmp_path, - {"esphome/A": {"name": "A", "dependencies": [{"name": "B", "version": "1.0"}]}}, + { + "esphome/A": { + "name": "A", + "dependencies": [{"name": "B", "version": "1.0", "platforms": [123]}], + } + }, ) - 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 + assert "Skipping dependency B of esphome/A: Malformed platforms" in caplog.text + + +def test_convert_libraries_warns_for_nonplatform_invalid_dependency_component( + tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture +) -> None: + """A dependency component dropped for any cause other than the platform + filter warns; only the routine cross-platform skip stays at debug.""" + _patch_download_with_manifests( + monkeypatch, + tmp_path, + { + "esphome/A": { + "name": "A", + "dependencies": [{"name": "C", "owner": "esphome", "version": "1.0"}], + }, + "esphome/C": {"name": "C", "frameworks": [None]}, + }, + ) + convert_libraries([Library("esphome/A", "1.0.0", None)], _backend()) + assert "Malformed frameworks" in caplog.text + assert "Skipping dependency" in caplog.text def test_split_flag_entry_non_string_is_clean() -> None: