diff --git a/esphome/build_helpers/ccache.py b/esphome/build_helpers/ccache.py index 11d40ac3c6..6c9223e4cc 100644 --- a/esphome/build_helpers/ccache.py +++ b/esphome/build_helpers/ccache.py @@ -32,6 +32,8 @@ def _ccache_runs(ccache: str) -> bool: stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, timeout=15, + # Repo-wide convention (posix_spawn fast path); pinned by + # tests/script/test_helpers.py close_fds=False, ) except (OSError, subprocess.SubprocessError): diff --git a/esphome/build_helpers/idedata.py b/esphome/build_helpers/idedata.py index 4bd8f41e10..e0b1a8efe4 100644 --- a/esphome/build_helpers/idedata.py +++ b/esphome/build_helpers/idedata.py @@ -18,6 +18,7 @@ from pathlib import Path import shlex import subprocess +from esphome.core import EsphomeError from esphome.helpers import write_file _LOGGER = logging.getLogger(__name__) @@ -278,14 +279,6 @@ def load_or_build_idedata( data = idedata_from_build(compile_commands, launcher) data["prog_path"] = str(elf_path) - if _is_launcher(data["cxx_path"]): - # Known-unusable: consumers must not run a launcher as the compiler, - # and a cached copy would outlive the timestamp check - _LOGGER.warning( - "compile_commands names the launcher %s as the compiler; no usable idedata", - data["cxx_path"], - ) - return None cache.parent.mkdir(parents=True, exist_ok=True) # Atomic so a crash mid-write cannot leave a truncated cache write_file(cache, json.dumps(data, indent=2) + "\n") @@ -315,6 +308,14 @@ def idedata_from_build(compile_commands: Path, launcher: str | None = None) -> d return parsed[key] cxx_path, defines, _, cxx_flags = _parse(_pick_entry(entries)) + if _is_launcher(cxx_path): + # Checked before the toolchain probe (which would fail opaquely on + # a launcher) so the unusable compile DB is named, and never + # cached or conflated with "nothing built yet" + raise EsphomeError( + f"compile_commands.json names the launcher {cxx_path} as the " + "compiler; the compile database is unusable" + ) build_includes: dict[str, None] = {} for entry in entries: diff --git a/esphome/platformio/extra_script.py b/esphome/platformio/extra_script.py index 67f143d2e7..1e0a233309 100644 --- a/esphome/platformio/extra_script.py +++ b/esphome/platformio/extra_script.py @@ -143,6 +143,12 @@ class _FakeSConsEnv: def Append(self, **kwargs) -> None: # noqa: N802 (SCons API name) for key, value in kwargs.items(): if key not in _CAPTURED_KEYS: + # Diagnosable from the build log when a script configures + # something this shim does not translate + _LOGGER.warning( + "PIO extra-script env.Append(%s=...) is not captured; ignoring", + key, + ) continue items = list(value) if isinstance(value, (list, tuple)) else [value] bucket = getattr(self.result, key.lower()) @@ -152,7 +158,7 @@ class _FakeSConsEnv: def __getattr__(self, name: str): def _noop(*args, **kwargs): - return None + _LOGGER.debug("PIO extra-script env.%s(...) is a no-op here", name) return _noop @@ -173,9 +179,10 @@ def run_extra_script( process CWD so relative-path lookups (``join``, ``realpath``, ``open``) resolve against the library tree. - On any exception inside the script we warn and return whatever the - script captured before failing — extra-scripts are best-effort, and an - unsupported script shouldn't block the build. + 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. """ env = _FakeSConsEnv( board_mcu=board_mcu, @@ -200,15 +207,16 @@ def run_extra_script( }, ) except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught - # Keep what the script captured before failing: dropping flags it - # already appended would fail later at link time, far from the cause + # Discard any partial capture: folding half a script's flags into the + # build could produce wrong-output firmware that links cleanly. The + # warning plus the resulting loud link error point back here. _LOGGER.warning( - "PIO extra-script %s (in %s) raised %s; keeping the partial capture", + "PIO extra-script %s (in %s) raised %s; ignoring its output", script_path, library_dir.name, e, ) - return env.result + return ExtraScriptResult() finally: os.chdir(old_cwd) return env.result diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index aa8a89ba88..60c1ddda60 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -560,7 +560,7 @@ def _resolve_registry_version( return owner, name, best["name"], pkgfile["download_url"] -def split_flag_entry(entry: str, owner: str) -> list[str]: +def split_flag_entry(entry: Any, owner: str) -> list[str]: """``shlex.split`` with a clean error naming the offending flags entry.""" try: return shlex.split(entry) diff --git a/tests/unit_tests/build_helpers/test_idedata.py b/tests/unit_tests/build_helpers/test_idedata.py index c34475724e..7297912955 100644 --- a/tests/unit_tests/build_helpers/test_idedata.py +++ b/tests/unit_tests/build_helpers/test_idedata.py @@ -11,6 +11,7 @@ from unittest.mock import MagicMock, patch import pytest from esphome.build_helpers import idedata +from esphome.core import EsphomeError # An absolute, forward-slash (shlex-safe) path prefix valid on the host OS, so # tests exercise the same is-absolute / normalize behavior as a real compile DB @@ -427,11 +428,9 @@ def test_load_or_build_idedata_corrupted_cache_is_logged( assert "Discarding unreadable idedata cache" in caplog.text -def test_load_or_build_idedata_never_caches_a_launcher( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: - """Idedata whose compiler path is a known launcher is served for this - run but not persisted, so the next build re-parses.""" +def test_load_or_build_idedata_never_caches_a_launcher(tmp_path: Path) -> None: + """A compile DB naming a launcher as the compiler is rejected by name, + before the toolchain probe could fail opaquely, and never cached.""" compile_commands = tmp_path / "compile_commands.json" compile_commands.write_text( json.dumps( @@ -445,13 +444,10 @@ def test_load_or_build_idedata_never_caches_a_launcher( ) ) cache = tmp_path / "c.json" - with patch.object(idedata, "get_toolchain_includes", return_value=[]): - data = idedata.load_or_build_idedata( - compile_commands, tmp_path / "f.elf", cache - ) - assert data is None + # No probe patch needed: the launcher is rejected before the probe runs + with pytest.raises(EsphomeError, match="compile database is unusable"): + idedata.load_or_build_idedata(compile_commands, tmp_path / "f.elf", cache) assert not cache.exists() - assert "no usable idedata" in caplog.text def test_load_or_build_idedata_cache_hit_skips_rebuild(tmp_path: Path) -> None: diff --git a/tests/unit_tests/test_platformio_extra_script.py b/tests/unit_tests/test_platformio_extra_script.py index a5f5382500..e1217a4006 100644 --- a/tests/unit_tests/test_platformio_extra_script.py +++ b/tests/unit_tests/test_platformio_extra_script.py @@ -224,7 +224,7 @@ def test_apply_extra_script_swallows_script_errors(tmp_path, caplog) -> None: c.data = {"build": {"extraScript": "extra.py"}} apply_extra_script(c, board_mcu=lambda: "esp8266", pio_platform="espressif8266") assert "flags" not in c.data["build"] - assert "keeping the partial capture" in caplog.text + assert "ignoring its output" in caplog.text def test_apply_extra_script_pio_platform(tmp_path) -> None: @@ -252,8 +252,9 @@ def test_apply_extra_script_missing_script_logged(tmp_path, caplog) -> None: assert "not found" in caplog.text -def test_run_extra_script_keeps_partial_capture(tmp_path, caplog) -> None: - """Flags appended before a script fails are kept, not dropped.""" +def test_run_extra_script_failure_discards_partial_capture(tmp_path, caplog) -> None: + """A crashed script yields an empty result: half-applied flags could + build wrong-output firmware that links cleanly.""" from esphome.platformio.extra_script import run_extra_script script = tmp_path / "extra.py" @@ -261,8 +262,8 @@ def test_run_extra_script_keeps_partial_capture(tmp_path, caplog) -> None: result = run_extra_script( script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" ) - assert result.libs == ["algobsec"] - assert "keeping the partial capture" in caplog.text + assert result.libs == [] + assert "ignoring its output" in caplog.text def test_run_extra_script_syntax_error_is_best_effort(tmp_path, caplog) -> None: @@ -276,4 +277,4 @@ def test_run_extra_script_syntax_error_is_best_effort(tmp_path, caplog) -> None: script, library_dir=tmp_path, board_mcu="esp32", pio_platform="espressif32" ) assert result.libs == [] - assert "keeping the partial capture" in caplog.text + assert "ignoring its output" in caplog.text