diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index c483d91cc6..77a6364c15 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -105,28 +105,28 @@ _LWIP_DEFAULT = (536, 1, 0, "lwip2-536-feat") _MMU_VARIANTS = ( ( "PIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48", - ["MMU_IRAM_SIZE=0xC000", "MMU_ICACHE_SIZE=0x4000"], + ("MMU_IRAM_SIZE=0xC000", "MMU_ICACHE_SIZE=0x4000"), ), ( "PIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48_SECHEAP_SHARED", - ["MMU_IRAM_SIZE=0xC000", "MMU_ICACHE_SIZE=0x4000", "MMU_IRAM_HEAP"], + ("MMU_IRAM_SIZE=0xC000", "MMU_ICACHE_SIZE=0x4000", "MMU_IRAM_HEAP"), ), ( "PIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM32_SECHEAP_NOTSHARED", - [ + ( "MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x4000", "MMU_SEC_HEAP_SIZE=0x4000", "MMU_SEC_HEAP=0x40108000", - ], + ), ), ( "PIO_FRAMEWORK_ARDUINO_MMU_EXTERNAL_128K", - ["MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000", "MMU_EXTERNAL_HEAP=128"], + ("MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000", "MMU_EXTERNAL_HEAP=128"), ), ( "PIO_FRAMEWORK_ARDUINO_MMU_EXTERNAL_1024K", - ["MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000", "MMU_EXTERNAL_HEAP=256"], + ("MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000", "MMU_EXTERNAL_HEAP=256"), ), ) _MMU_DEFAULT = ("MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000") @@ -208,25 +208,34 @@ class _BuildConfig: mmu_defines: list[str] = field(default_factory=list) -def _flag_defines(unflags: set[str]) -> dict[str, str]: +def _lexed_build_flags() -> list[str]: + """Shell-lex every ``CORE.build_flags`` entry the way PlatformIO's + ``ParseFlags`` does, so a knob in ``"-DKNOB -DOTHER"``, a spaced + ``"-D KNOB"``, and quoted bodies all read identically everywhere. + + Sorted so duplicate defines resolve the same way every run (the winner + feeds the linker-script preprocessor line, which is also the cache + stamp). Lex once per build and pass the result to ``_flag_defines`` and + ``_project_flags`` so a malformed entry warns once, not per consumer. + """ + return [ + tok + for flag in sorted(CORE.build_flags) + for tok in join_flag_args(split_flag_entry(flag, "esphome"), "esphome") + ] + + +def _flag_defines(unflags: set[str], tokens: list[str] | None = None) -> dict[str, str]: """Map define name -> full ``NAME[=VALUE]`` for every -D build flag.""" defines: dict[str, str] = {} - # Sorted so duplicate defines resolve the same way every run: the - # winner feeds the linker-script preprocessor line, which is also the - # cache stamp - for flag in sorted(CORE.build_flags): - # Shell-lex every entry the way PlatformIO's ParseFlags does, so a - # knob in "-DKNOB -DOTHER", a spaced "-D KNOB", and quoted bodies all - # read identically to _project_flags (and the compile line). - tokens = join_flag_args(split_flag_entry(flag, "esphome"), "esphome") - for tok in tokens: - # An unflagged knob must not drive lwIP/SDK/MMU selection while - # being absent from the compile line - if tok in unflags: - continue - if tok.startswith("-D") and len(tok) > 2: - body = tok[2:] - defines[body.split("=", 1)[0]] = body + for tok in _lexed_build_flags() if tokens is None else tokens: + # An unflagged knob must not drive lwIP/SDK/MMU selection while + # being absent from the compile line + if tok in unflags: + continue + if tok.startswith("-D") and len(tok) > 2: + body = tok[2:] + defines[body.split("=", 1)[0]] = body return defines @@ -243,6 +252,16 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: tcp_mss, features, ipv6, lwip_lib = variant break + # The lwIP triple selects a prebuilt library; a raw override would win + # the compile line (user tokens come last here) while the link still + # pulls the library built for the knob's values + if owned := sorted( + n for n in ("TCP_MSS", "LWIP_FEATURES", "LWIP_IPV6") if n in defines + ): + raise EsphomeError( + f"{', '.join(owned)} are set by the PIO_FRAMEWORK_ARDUINO_LWIP2_* " + "knobs; drop the raw build flags" + ) knob_defines = [ f"{nonosdk}=1", f"TCP_MSS={tcp_mss}", @@ -267,32 +286,37 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: ) 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) - if mmu is None: - if "PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" in defines: - if "MMU_IRAM_SIZE" not in defines or "MMU_ICACHE_SIZE" not in defines: - raise EsphomeError( - "PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM requires MMU_IRAM_SIZE and " - "MMU_ICACHE_SIZE build flags" - ) - # Sorted so build.ninja and the linker-script stamp stay - # byte-stable across runs (the flag set has no deterministic - # iteration order). - mmu = sorted( - body for name, body in defines.items() if name.startswith("MMU_") + mmu_knob = next((knob for knob, _variant in _MMU_VARIANTS if knob in defines), None) + if mmu_knob is not None: + if raw := sorted(n for n in defines if n.startswith("MMU_")): + # Same compile-line/linker-script split as the no-knob case below + raise EsphomeError( + f"{', '.join(raw)} conflict with {mmu_knob}; drop the raw MMU_* " + "build flags or use PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" ) - else: - if "MMU_IRAM_SIZE" in defines or "MMU_ICACHE_SIZE" in defines: - # 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) + mmu = list(dict(_MMU_VARIANTS)[mmu_knob]) + elif "PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM" in defines: + if "MMU_IRAM_SIZE" not in defines or "MMU_ICACHE_SIZE" not in defines: + raise EsphomeError( + "PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM requires MMU_IRAM_SIZE and " + "MMU_ICACHE_SIZE build flags" + ) + # Sorted so build.ninja and the linker-script stamp stay + # byte-stable across runs (the flag set has no deterministic + # iteration order). + mmu = sorted(body for name, body in defines.items() if name.startswith("MMU_")) + else: + if "MMU_IRAM_SIZE" in defines or "MMU_ICACHE_SIZE" in defines: + # 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) return _BuildConfig( nonosdk=nonosdk, @@ -360,7 +384,7 @@ def _unflag_tokens() -> set[str]: def _project_flags( - unflags: set[str], + unflags: set[str], tokens: list[str] | None = None ) -> tuple[list[str], list[str], list[Path], list[str]]: """Split the ESPHome build flags into compile, linker, -L, and -l lists. @@ -376,25 +400,31 @@ def _project_flags( link_flags: list[str] = [] lib_dirs: list[Path] = [] libs: list[str] = [] - for flag in sorted(CORE.build_flags): - for tok in join_flag_args(split_flag_entry(flag, "esphome"), "esphome"): - if tok in unflags: + for tok in _lexed_build_flags() if tokens is None else tokens: + if tok in unflags: + continue + if tok.startswith("-Wl,"): + link_flags.append(_shell_token(tok)) + elif tok.startswith("-L"): + if len(tok) == 2: + # Path("") is the CWD; never add it silently + _LOGGER.warning("Ignoring empty -L in build_flags") continue - if tok.startswith("-Wl,"): - link_flags.append(_shell_token(tok)) - elif tok.startswith("-L"): - if len(tok) == 2: - # Path("") is the CWD; never add it silently - _LOGGER.warning("Ignoring empty -L in build_flags") - continue - lib_dirs.append(Path(tok[2:])) - elif tok.startswith("-l"): - if len(tok) == 2: - _LOGGER.warning("Ignoring empty -l in build_flags") - continue - libs.append(tok[2:]) - else: - compile_flags.append(_shell_token(tok)) + lib_dirs.append(Path(tok[2:])) + elif tok.startswith("-l"): + if len(tok) == 2: + _LOGGER.warning("Ignoring empty -l in build_flags") + continue + libs.append(tok[2:]) + else: + if tok == "-u" or tok.startswith(("-T", "-Xlinker")): + # Inert on the compile line; the user expects it to link + _LOGGER.warning( + "Linker flag %s in build_flags is not routed to the link " + "line; use the -Wl, form", + tok, + ) + compile_flags.append(_shell_token(tok)) return compile_flags, link_flags, lib_dirs, libs @@ -424,11 +454,8 @@ def generate_ld_scripts( cmd += [f"-D{d}" for d in config.mmu_defines] if config.fp_in_irom: cmd.append("-DFP_IN_IROM") - cmd += [ - str(framework / "tools" / "sdk" / "ld" / "eagle.app.v6.common.ld.h"), - "-o", - "-", - ] + header = framework / "tools" / "sdk" / "ld" / "eagle.app.v6.common.ld.h" + cmd += [str(header), "-o", "-"] # The inputs are the command line (defines + framework version, which is # baked into the paths) plus testing mode; skip the preprocessor spawn on @@ -437,25 +464,37 @@ def generate_ld_scripts( stamp = ld_dir / ".local.eagle.app.v6.common.ld.stamp" # The surgery constants are inputs too: an edit to build_surgery.py must # invalidate existing build dirs, not wait for an esphome clean. + # The header's size and mtime cover an in-place framework edit or + # re-extraction at the same versioned path, which the command line + # alone would not notice + try: + header_stat = header.stat() + header_sig = f"{header_stat.st_size}:{header_stat.st_mtime_ns}" + except OSError: + header_sig = "missing" # the preprocessor spawn below names it stamp_content = ( " ".join(cmd) + f" testing={CORE.testing_mode}" + + f" header={header_sig}" # One fingerprint instead of enumerating surgery internals here, so # any behavioral edit in build_surgery self-invalidates the cache + f" {build_surgery.surgery_fingerprint()}" ) def _cached_ld_is_valid() -> bool: - if not ( - output.is_file() - and stamp.is_file() - and stamp.read_text(encoding="utf-8") == stamp_content - ): + # A damaged cache (unreadable, non-UTF-8, truncated, externally + # edited) must regenerate, not abort the build or be reused on + # existence alone (the SECTIONS check below only guards generation) + try: + if not ( + output.is_file() + and stamp.is_file() + and stamp.read_text(encoding="utf-8") == stamp_content + ): + return False + return "SECTIONS" in output.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): 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: @@ -478,7 +517,12 @@ def generate_ld_scripts( "Generated linker script is missing its SECTIONS block; " "run 'esphome clean-all' and retry" ) - content = build_surgery.relocate_ratetable(result.stdout) + try: + content = build_surgery.relocate_ratetable(result.stdout) + except RuntimeError as err: + # The anchor moved in a new core release: a named error, not a + # traceback, and never a silently unrelocated rate table + raise EsphomeError(str(err)) from err if CORE.testing_mode: content = build_surgery.apply_testing_memory_patches( content, ("iram1_0_seg",) diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index 4e9de9d468..da803bd7bb 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -981,3 +981,96 @@ def test_generate_ld_scripts_surfaces_preprocessor_warnings( pytest.raises(EsphomeError, match="SECTIONS"), ): _run_generate_ld_scripts(paths) + + +def test_build_config_mmu_knob_with_raw_mmu_flag_raises() -> None: + """A variant knob plus a raw MMU_* define would split the compile line + from the linker script; refuse like the no-knob case.""" + _set_flags("-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48", "-DMMU_IRAM_SIZE=0x4000") + with pytest.raises(EsphomeError, match="MMU_IRAM_SIZE conflict with .*CACHE16"): + _resolve_build_config(_flag_defines(set())) + + +def test_build_config_raw_lwip_define_raises() -> None: + """TCP_MSS/LWIP_* belong to the lwIP knobs: a raw value would win the + compile line while the prebuilt library stays the knob's.""" + _set_flags("-DTCP_MSS=1024") + with pytest.raises(EsphomeError, match="TCP_MSS are set by the .*LWIP2"): + _resolve_build_config(_flag_defines(set())) + + +def test_build_config_mmu_defines_do_not_alias_the_table() -> None: + """The resolved list must be a copy; mutating it must not corrupt the + module table for later builds in the same process.""" + _set_flags("-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48") + config = _resolve_build_config(_flag_defines(set())) + config.mmu_defines.append("MMU_BOGUS") + again = _resolve_build_config(_flag_defines(set())) + assert "MMU_BOGUS" not in again.mmu_defines + assert all(isinstance(v, tuple) for _k, v in arduino8266._MMU_VARIANTS) + + +def test_lexed_build_flags_shared_between_consumers( + caplog: pytest.LogCaptureFixture, +) -> None: + """Lexing once and passing the tokens to both consumers yields the same + result as each lexing itself, with a malformed entry warned once.""" + _set_flags("-DFOO=1 -l", "-Wl,--wrap=x") + tokens = arduino8266._lexed_build_flags() + assert caplog.text.count("Ignoring trailing '-l'") == 1 + assert _flag_defines(set(), tokens) == _flag_defines(set()) + assert arduino8266._project_flags(set(), tokens) == arduino8266._project_flags( + set() + ) + + +def test_project_flags_warns_on_plain_linker_forms( + caplog: pytest.LogCaptureFixture, +) -> None: + """A plain-form linker flag lands on the compile line where it is inert; + the user must be told to use the -Wl, form.""" + _set_flags("-Tcustom.ld", "-Xlinker", "-u", "-Os") + compile_flags, _l, _d, _libs = arduino8266._project_flags(set()) + assert "-Os" in compile_flags + for tok in ("-Tcustom.ld", "-Xlinker", "-u"): + assert f"Linker flag {tok} in build_flags is not routed" in caplog.text + + +def test_generate_ld_scripts_header_change_invalidates_stamp( + tmp_path: Path, +) -> None: + """An in-place framework edit at the same path regenerates the script.""" + paths = _make_framework(tmp_path) + header = paths.framework / "tools" / "sdk" / "ld" / "eagle.app.v6.common.ld.h" + header.write_text("v1") + result = MagicMock(returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="") + with patch.object(arduino8266.subprocess, "run", return_value=result): + _run_generate_ld_scripts(paths) + header.write_text("v2 (longer)") + with patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run: + _run_generate_ld_scripts(paths) + mock_run.assert_called_once() + + +def test_generate_ld_scripts_unreadable_stamp_regenerates(tmp_path: Path) -> None: + """A non-UTF-8 stamp is a damaged cache: regenerate, never abort.""" + 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) + (ld_dir / ".local.eagle.app.v6.common.ld.stamp").write_bytes(b"\xff\xfe") + with patch.object(arduino8266.subprocess, "run", return_value=result) as mock_run: + _run_generate_ld_scripts(paths) + mock_run.assert_called_once() + + +def test_generate_ld_scripts_surgery_failure_is_named(tmp_path: Path) -> None: + """A moved rate-table anchor surfaces as a build error, not a traceback + or a silently unrelocated table.""" + paths = _make_framework(tmp_path) + result = MagicMock(returncode=0, stdout="SECTIONS { no anchor here }", stderr="") + with ( + patch.object(arduino8266.subprocess, "run", return_value=result), + pytest.raises(EsphomeError, match="anchor not found"), + ): + _run_generate_ld_scripts(paths)