diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index f4807419b8..ae38bc547a 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -167,7 +167,7 @@ class _BuildConfig: mmu_defines: list[str] = field(default_factory=list) -def _flag_defines() -> dict[str, str]: +def _flag_defines(unflags: set[str]) -> 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 @@ -179,6 +179,10 @@ def _flag_defines() -> dict[str, str]: # 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 @@ -209,10 +213,13 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: # Sorted so the pick is deterministic: the dict is built from a set of # build flags, whose iteration order varies between processes. - vtables = next( - (name for name in sorted(defines) if name.startswith("VTABLES_IN_")), - "VTABLES_IN_FLASH", - ) + vtables_knobs = sorted(name for name in defines if name.startswith("VTABLES_IN_")) + known_vtables = {"VTABLES_IN_FLASH", "VTABLES_IN_DRAM", "VTABLES_IN_IRAM"} + if unknown := [k for k in vtables_knobs if k not in known_vtables]: + _LOGGER.warning("Unknown VTABLES_IN_* define(s): %s", ", ".join(unknown)) + if len(vtables_knobs) > 1: + _LOGGER.warning("Multiple VTABLES_IN_* defines; using %s", vtables_knobs[0]) + 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: @@ -278,10 +285,16 @@ def _defines_flags( def _unflag_tokens() -> set[str]: """``build_unflags`` entries shell-lexed to tokens, as PlatformIO matches.""" + # Joined like _project_flags reads build_flags, so "-D FOO" removes + # -DFOO in both spellings (PlatformIO's ProcessUnFlags parses the same + # way) and no bare half can collaterally drop an unrelated token return { tok for entry in CORE.build_unflags - for tok in split_flag_entry(entry, "esphome build_unflags") + for tok in join_flag_args( + split_flag_entry(entry, "esphome build_unflags"), + "esphome build_unflags", + ) } @@ -306,8 +319,15 @@ def _project_flags( 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)) @@ -368,6 +388,15 @@ def generate_ld_scripts( ) from err if result.returncode != 0: raise EsphomeError(f"Generating the linker script failed:\n{result.stderr}") + if result.stderr.strip(): + # Preprocessor warnings on the success path must reach the user + _LOGGER.warning("Linker-script preprocessor: %s", result.stderr.strip()) + if "SECTIONS" not in result.stdout: + # A degenerate zero-exit run must not be stamped as a good cache + raise EsphomeError( + "Generated linker script is missing its SECTIONS block; " + "run 'esphome clean-all' and retry" + ) content = build_surgery.relocate_ratetable(result.stdout) if CORE.testing_mode: content = build_surgery.apply_testing_memory_patches( diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index c28a782308..5c91e54cc1 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -60,7 +60,7 @@ def _set_flags(*flags: str) -> None: def test_build_config_defaults() -> None: _set_flags() - config = _resolve_build_config(_flag_defines()) + config = _resolve_build_config(_flag_defines(set())) assert config.nonosdk == "NONOSDK22x_190703" assert config.lwip_lib == "lwip2-536-feat" assert not config.exceptions @@ -79,7 +79,7 @@ def test_build_config_esphome_lwip_knob() -> None: as the PlatformIO builder.""" _set_flags("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH") - config = _resolve_build_config(_flag_defines()) + config = _resolve_build_config(_flag_defines(set())) assert config.lwip_lib == "lwip2-1460" assert "TCP_MSS=1460" in config.knob_defines assert "LWIP_FEATURES=0" in config.knob_defines @@ -94,7 +94,7 @@ def test_build_config_knobs() -> None: "-DPIO_FRAMEWORK_ARDUINO_MMU_CACHE16_IRAM48", "-DVTABLES_IN_DRAM", ) - config = _resolve_build_config(_flag_defines()) + config = _resolve_build_config(_flag_defines(set())) assert config.nonosdk == "NONOSDK305" assert config.exceptions assert config.vtables == "VTABLES_IN_DRAM" @@ -105,14 +105,14 @@ def test_build_config_mmu_custom_requires_sizes() -> None: _set_flags("-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM") with pytest.raises(EsphomeError, match="MMU_IRAM_SIZE"): - _resolve_build_config(_flag_defines()) + _resolve_build_config(_flag_defines(set())) _set_flags( "-DPIO_FRAMEWORK_ARDUINO_MMU_CUSTOM", "-DMMU_IRAM_SIZE=0xC000", "-DMMU_ICACHE_SIZE=0x4000", ) - config = _resolve_build_config(_flag_defines()) + config = _resolve_build_config(_flag_defines(set())) # Emitted pre-sorted so build.ninja stays byte-stable across runs assert config.mmu_defines == [ "MMU_ICACHE_SIZE=0x4000", @@ -125,7 +125,7 @@ def test_defines_match_platformio_builder() -> None: _set_flags("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH") assert _defines_flags( - _resolve_build_config(_flag_defines()), + _resolve_build_config(_flag_defines(set())), "dout", "nodemcuv2", ESP8266_BOARD_BUILD["nodemcuv2"]["defines"], @@ -196,7 +196,7 @@ def test_build_config_lwip_variants( """Every lwIP knob maps to the same defines and library as the PIO builder.""" _set_flags(f"-D{knob}") - config = _resolve_build_config(_flag_defines()) + config = _resolve_build_config(_flag_defines(set())) assert config.lwip_lib == lib assert f"TCP_MSS={mss}" in config.knob_defines assert f"LWIP_FEATURES={features}" in config.knob_defines @@ -232,13 +232,13 @@ def test_build_config_lwip_variants( def test_build_config_mmu_variants(knob: str, expected: list[str]) -> None: _set_flags(f"-D{knob}") - assert _resolve_build_config(_flag_defines()).mmu_defines == expected + assert _resolve_build_config(_flag_defines(set())).mmu_defines == expected def test_build_config_waveform_locked_phase() -> None: _set_flags("-DPIO_FRAMEWORK_ARDUINO_WAVEFORM_LOCKED_PHASE", "-DFP_IN_IROM") - config = _resolve_build_config(_flag_defines()) + config = _resolve_build_config(_flag_defines(set())) assert "WAVEFORM_LOCKED_PHASE=1" in config.knob_defines assert config.fp_in_irom @@ -260,7 +260,7 @@ SECTIONS def _run_generate_ld_scripts(paths: InstalledPaths) -> Path: - config = _resolve_build_config(_flag_defines()) + config = _resolve_build_config(_flag_defines(set())) arduino8266.generate_ld_scripts(paths, config, "eagle.flash.4m.ld") return CORE.relative_pioenvs_path(CORE.name, "ld") @@ -333,7 +333,7 @@ def test_build_config_nonosdk_precedence() -> None: "-DPIO_FRAMEWORK_ARDUINO_ESPRESSIF_SDK305", "-DPIO_FRAMEWORK_ARDUINO_ESPRESSIF_SDK221", ) - assert _resolve_build_config(_flag_defines()).nonosdk == "NONOSDK221" + assert _resolve_build_config(_flag_defines(set())).nonosdk == "NONOSDK221" def test_project_flags_trailing_bare_linker_flag_warns( @@ -364,7 +364,7 @@ def test_project_flags_lexed_entry_scatters_non_linker_tokens() -> None: def test_flag_defines_lexes_multi_token_entries() -> None: """A knob inside a multi-token entry is detected like PlatformIO does.""" _set_flags("-DPIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH -DFOO=1 -Os") - defines = _flag_defines() + defines = _flag_defines(set()) assert "PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH" in defines assert defines["FOO"] == "FOO=1" config = _resolve_build_config(defines) @@ -406,7 +406,7 @@ def test_project_flags_requotes_lexed_defines() -> None: def test_flag_defines_joins_spaced_define() -> None: """A spaced "-D KNOB" entry is detected exactly as PlatformIO detects it.""" _set_flags("-D PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH") - defines = _flag_defines() + defines = _flag_defines(set()) assert "PIO_FRAMEWORK_ARDUINO_LWIP2_HIGHER_BANDWIDTH_LOW_FLASH" in defines assert "" not in defines @@ -417,7 +417,7 @@ def test_build_config_custom_mmu_without_knob_warns( """Custom MMU sizes without the CUSTOM knob keep the default layout and warn, as the PlatformIO builder does.""" _set_flags("-DMMU_IRAM_SIZE=0xC000") - config = _resolve_build_config(_flag_defines()) + 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 @@ -425,14 +425,14 @@ def test_build_config_custom_mmu_without_knob_warns( def test_flag_defines_lexes_quoted_single_tokens() -> None: """A quoted single-token define reads the same as on the compile line.""" _set_flags('-DMMU_SEC_HEAP="0x40108000"') - assert _flag_defines()["MMU_SEC_HEAP"] == "MMU_SEC_HEAP=0x40108000" + assert _flag_defines(set())["MMU_SEC_HEAP"] == "MMU_SEC_HEAP=0x40108000" def test_flag_defines_duplicate_defines_resolve_deterministically() -> None: """Duplicate conflicting defines pick the same winner every run (sorted iteration, last writer wins), independent of the set's hash seed.""" _set_flags("-DMMU_IRAM_SIZE=0x8000", "-DMMU_IRAM_SIZE=0xC000") - assert _flag_defines()["MMU_IRAM_SIZE"] == "MMU_IRAM_SIZE=0xC000" + assert _flag_defines(set())["MMU_IRAM_SIZE"] == "MMU_IRAM_SIZE=0xC000" def test_flag_tables_match_platformio_builder() -> None: @@ -447,20 +447,56 @@ def test_flag_tables_match_platformio_builder() -> None: "-fno-inline-functions", "-nostdlib", ] - assert arduino8266._CCFLAGS[:6] == [ + assert arduino8266._CCFLAGS == [ "-Os", "-mlongcalls", "-mtext-section-literals", "-falign-functions=4", "-U__STRICT_ANSI__", "-ffunction-sections", + "-fdata-sections", + "-Wall", + "-Werror=return-type", + "-free", + "-fipa-pta", ] - assert arduino8266._LINKFLAGS[:5] == [ + # The -u block is where the deliberate -u _scanf_float omission lives; + # pinned in full so "restoring" it fails here first + assert arduino8266._LINKFLAGS == [ "-Os", "-nostdlib", "-Wl,--no-check-sections", "-Wl,-static", "-Wl,--gc-sections", + "-Wl,-wrap,system_restart_local", + "-Wl,-wrap,spi_flash_read", + "-u", + "app_entry", + "-u", + "_printf_float", + "-u", + "_DebugExceptionVector", + "-u", + "_DoubleExceptionVector", + "-u", + "_KernelExceptionVector", + "-u", + "_NMIExceptionVector", + "-u", + "_UserExceptionVector", + ] + # Order is load-bearing: upstream's LIBS order resolves symbols correctly + assert arduino8266._SYSTEM_LIBS_PRE_LWIP == ["hal", "phy", "pp", "net80211"] + assert arduino8266._SYSTEM_LIBS_POST_LWIP == [ + "wpa", + "crypto", + "main", + "wps", + "bearssl", + "espnow", + "smartconfig", + "airkiss", + "wpa2", ] @@ -471,3 +507,70 @@ def test_generate_ld_scripts_missing_compiler_is_clean(tmp_path: Path) -> None: _set_flags() with pytest.raises(EsphomeError, match="Could not run"): _run_generate_ld_scripts(paths) + + +def test_unflag_tokens_join_spaced_entries() -> None: + """A spaced build_unflags entry ("-D FOO") removes -DFOO, and no bare half leaks into the + unflag set to collaterally drop unrelated tokens.""" + CORE.build_unflags = {"-D FOO", "-l bar"} + tokens = arduino8266._unflag_tokens() + assert tokens == {"-DFOO", "-lbar"} + CORE.build_flags = {"-DFOO -lbar", "-DBAR"} + compile_flags, _link, _dirs, libs = arduino8266._project_flags(tokens) + assert compile_flags == ["-DBAR"] + assert libs == [] + + +def test_flag_defines_respects_unflags() -> None: + """An unflagged knob must not drive the derived toolchain config.""" + _set_flags("-DVTABLES_IN_DRAM") + defines = _flag_defines({"-DVTABLES_IN_DRAM"}) + assert "VTABLES_IN_DRAM" not in defines + config = _resolve_build_config(defines) + 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_project_flags_empty_lib_flags_warn( + caplog: pytest.LogCaptureFixture, +) -> None: + """A bare -L must not silently add the CWD to the search path.""" + CORE.build_flags = {'-L ""', '-l ""'} + _c, _l, lib_dirs, libs = arduino8266._project_flags(set()) + assert lib_dirs == [] + assert libs == [] + assert "Ignoring empty -L" in caplog.text + assert "Ignoring empty -l" in caplog.text + + +def test_generate_ld_scripts_surfaces_preprocessor_warnings( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """Preprocessor stderr on a zero exit reaches the user; degenerate output is refused.""" + paths = _make_framework(tmp_path) + _set_flags() + result = MagicMock( + returncode=0, stdout=_COMMON_LD_H_OUTPUT, stderr="warning: something" + ) + with patch.object(arduino8266.subprocess, "run", return_value=result): + _run_generate_ld_scripts(paths) + assert "Linker-script preprocessor: warning: something" in caplog.text + + # New flags invalidate the stamp so the degenerate run regenerates + _set_flags("-DVTABLES_IN_DRAM") + result = MagicMock(returncode=0, stdout="", stderr="") + with ( + patch.object(arduino8266.subprocess, "run", return_value=result), + pytest.raises(EsphomeError, match="SECTIONS"), + ): + _run_generate_ld_scripts(paths)