diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index ae38bc547a..6f302833a1 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -215,10 +215,15 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: # build flags, whose iteration order varies between processes. vtables_knobs = sorted(name for name in defines if name.startswith("VTABLES_IN_")) known_vtables = {"VTABLES_IN_FLASH", "VTABLES_IN_DRAM", "VTABLES_IN_IRAM"} + # A typo would otherwise win the sorted pick and end in the SDK header's + # #error, and a conflicting pair would resolve arbitrarily; both are + # config errors, not build-time surprises if unknown := [k for k in vtables_knobs if k not in known_vtables]: - _LOGGER.warning("Unknown VTABLES_IN_* define(s): %s", ", ".join(unknown)) + raise EsphomeError(f"Unknown VTABLES_IN_* define(s): {', '.join(unknown)}") if len(vtables_knobs) > 1: - _LOGGER.warning("Multiple VTABLES_IN_* defines; using %s", vtables_knobs[0]) + raise EsphomeError( + f"Conflicting VTABLES_IN_* defines: {', '.join(vtables_knobs)}" + ) vtables = vtables_knobs[0] if vtables_knobs else "VTABLES_IN_FLASH" mmu = next((variant for knob, variant in _MMU_VARIANTS if knob in defines), None) @@ -237,13 +242,14 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: ) else: if "MMU_IRAM_SIZE" in defines or "MMU_ICACHE_SIZE" in defines: - # Same diagnostic the PlatformIO builder prints: without the - # knob the linker script keeps the default layout while the - # compile line carries the custom sizes - _LOGGER.warning( - "Detected custom MMU flags; use " - "-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM to disable the " - "default configuration" + # PlatformIO only warns here and appends its defaults last so + # they win the compile line; in this generator the user's + # tokens would come last instead, compiling against a memory + # layout the linker script does not implement. Refuse rather + # than reproduce the upstream footgun with worse odds. + raise EsphomeError( + "Custom MMU_IRAM_SIZE/MMU_ICACHE_SIZE build flags require " + "-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" ) mmu = list(_MMU_DEFAULT) @@ -306,7 +312,10 @@ def _project_flags( Every entry is shell-lexed the way PlatformIO's ``ParseFlags`` does, so a linker flag anywhere in an entry reaches the link line and ``build_unflags`` matches individual tokens (``-Os`` inside ``-Os -g3``). - Lexed tokens are re-quoted at emission via ``_shell_token``. + Only the flag forms ESPHome emits are classified (``-Wl,``/``-L``/``-l`` + and compile flags); rarities like plain-form ``-T``/``-u``/``-Xlinker`` + route to the compile line, unlike full ParseFlags. Lexed tokens are + re-quoted at emission via ``_shell_token``. """ compile_flags: list[str] = [] link_flags: list[str] = [] @@ -372,11 +381,20 @@ def generate_ld_scripts( # any behavioral edit in build_surgery self-invalidates the cache + f" {build_surgery.surgery_fingerprint()}" ) - if not ( - output.is_file() - and stamp.is_file() - and stamp.read_text(encoding="utf-8") == stamp_content - ): + + def _cached_ld_is_valid() -> bool: + if not ( + output.is_file() + and stamp.is_file() + and stamp.read_text(encoding="utf-8") == stamp_content + ): + return False + # A truncated or externally edited script must force regeneration, + # not be reused on existence alone (the SECTIONS check below only + # guards the generation path) + return "SECTIONS" in output.read_text(encoding="utf-8") + + if not _cached_ld_is_valid(): try: result = subprocess.run( cmd, capture_output=True, text=True, check=False, close_fds=False @@ -409,10 +427,17 @@ def generate_ld_scripts( # A patched copy of the flash ld in the build dir; resolved through # the same -L path as the SDK original it shadows. flash_ld = framework / "tools" / "sdk" / "ld" / flash_ld_name + try: + flash_ld_text = flash_ld.read_text(encoding="utf-8") + except OSError as err: + # Same half-extracted-cache hazard as the gcc spawn above + raise EsphomeError( + f"Could not read {flash_ld}: {err}; run 'esphome clean-all' and retry" + ) from err write_file_if_changed( ld_dir / f"testing_{flash_ld_name}", build_surgery.apply_testing_memory_patches( - flash_ld.read_text(encoding="utf-8"), + flash_ld_text, ("dram0_0_seg", "irom0_0_seg"), ), ) diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index e7ea3b6e77..8303ec0322 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -153,7 +153,7 @@ def test_defines_match_platformio_builder() -> None: ] -def _make_framework(tmp_path: Path) -> dict[str, Path]: +def _make_framework(tmp_path: Path) -> InstalledPaths: framework = tmp_path / "framework" core = framework / "cores" / "esp8266" core.mkdir(parents=True) @@ -270,9 +270,14 @@ def test_generate_ld_scripts(tmp_path: Path) -> None: paths = _make_framework(tmp_path) _set_flags("-DFP_IN_IROM") - result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT) - with patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run: + result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="") + with ( + patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run, + patch.object(arduino8266._LOGGER, "warning") as mock_warn, + ): ld_dir = _run_generate_ld_scripts(paths) + # A clean preprocessor run must be quiet + mock_warn.assert_not_called() content = (ld_dir / "local.eagle.app.v6.common.ld").read_text() assert RATETABLE_RULE in content cmd = mock_run.call_args[0][0] @@ -297,6 +302,20 @@ def test_generate_ld_scripts(tmp_path: Path) -> None: mock_run.assert_called_once() +def test_generate_ld_scripts_corrupt_cache_regenerates(tmp_path: Path) -> None: + """A truncated cached linker script regenerates even with a fresh stamp.""" + paths = _make_framework(tmp_path) + result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="") + with patch.object(arduino8266.subprocess, "run", return_value=result): + ld_dir = _run_generate_ld_scripts(paths) + output = ld_dir / "local.eagle.app.v6.common.ld" + output.write_text("truncated garbage") + with patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run: + _run_generate_ld_scripts(paths) + mock_run.assert_called_once() + assert RATETABLE_RULE in output.read_text() + + def test_generate_ld_scripts_failure(tmp_path: Path) -> None: paths = _make_framework(tmp_path) @@ -320,7 +339,7 @@ def test_generate_ld_scripts_testing_mode(tmp_path: Path) -> None: "}\n" ) CORE.testing_mode = True - result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT) + result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="") with patch.object(arduino8266.subprocess, "run", return_value=result): ld_dir = _run_generate_ld_scripts(paths) patched = (ld_dir / "testing_eagle.flash.4m.ld").read_text() @@ -415,15 +434,14 @@ def test_flag_defines_joins_spaced_define() -> None: assert "" not in defines -def test_build_config_custom_mmu_without_knob_warns( - caplog: pytest.LogCaptureFixture, -) -> None: - """Custom MMU sizes without the CUSTOM knob keep the default layout and - warn, as the PlatformIO builder does.""" +def test_build_config_custom_mmu_without_knob_raises() -> None: + """Custom MMU sizes without the CUSTOM knob would compile against a + layout the linker script does not implement; refuse instead of warning + (PlatformIO warns, but its defaults win the compile line; ours would + not).""" _set_flags("-DMMU_IRAM_SIZE=0xC000") - config = _resolve_build_config(_flag_defines(set())) - assert config.mmu_defines == ["MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000"] - assert "Detected custom MMU flags" in caplog.text + with pytest.raises(EsphomeError, match="PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM"): + _resolve_build_config(_flag_defines(set())) def test_flag_defines_lexes_quoted_single_tokens() -> None: @@ -534,15 +552,18 @@ def test_flag_defines_respects_unflags() -> None: assert config.vtables == "VTABLES_IN_FLASH" -def test_vtables_unknown_and_conflicting_warn( - caplog: pytest.LogCaptureFixture, -) -> None: - _set_flags("-DVTABLES_IN_BANANA", "-DVTABLES_IN_DRAM") - config = _resolve_build_config(_flag_defines(set())) - assert "Unknown VTABLES_IN_*" in caplog.text - assert "Multiple VTABLES_IN_*" in caplog.text - # Deterministic pick, as before - assert config.vtables == "VTABLES_IN_BANANA" +def test_vtables_unknown_raises() -> None: + """A typo'd knob would win the sorted pick and die in the SDK header's + #error; fail by name at generation instead.""" + _set_flags("-DVTABLES_IN_BANANA") + with pytest.raises(EsphomeError, match="Unknown VTABLES_IN_.*BANANA"): + _resolve_build_config(_flag_defines(set())) + + +def test_vtables_conflicting_raises() -> None: + _set_flags("-DVTABLES_IN_DRAM", "-DVTABLES_IN_IRAM") + with pytest.raises(EsphomeError, match="Conflicting VTABLES_IN_"): + _resolve_build_config(_flag_defines(set())) def test_project_flags_empty_lib_flags_warn(