Merge branch 'esp8266-native-build-surgery' into esp8266-native-toolchain-plumbing

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:

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

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)

esphome/platformio/toolchain.py

@@ -254,6 +254,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):

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:

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