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 49753cfc7f..22e122dd28 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -1140,8 +1140,8 @@ def convert_libraries( ): if "version" not in dependency: # Cannot resolve from the registry; common for bundled - # names (Wire, SPI) -- add_library() is the fix if real - _LOGGER.info( + # names (Wire, SPI) -- unactionable noise above debug + _LOGGER.debug( "Skip version-less dependency %r of %s", dependency.get("name"), component.name, 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 78c4723054..140f8bb169 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -852,20 +852,15 @@ 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( @@ -881,20 +876,11 @@ def test_convert_libraries_warns_for_nonplatform_invalid_dependency_component( "name": "A", "dependencies": [{"name": "C", "owner": "esphome", "version": "1.0"}], }, - "esphome/C": {"name": "C"}, + "esphome/C": {"name": "C", "frameworks": [None]}, }, ) - 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 "Malformed frameworks" in caplog.text assert "Skipping dependency" in caplog.text