diff --git a/esphome/arduino8266/component.py b/esphome/arduino8266/component.py index 59d8746769..09cd4a5ea4 100644 --- a/esphome/arduino8266/component.py +++ b/esphome/arduino8266/component.py @@ -18,7 +18,7 @@ import logging from pathlib import Path import shlex -from esphome.core import CORE, Library +from esphome.core import CORE, EsphomeError, Library from esphome.espidf.extra_script import apply_extra_script from esphome.platformio.library import ( DEFAULT_BUILD_INCLUDE_DIR, @@ -100,6 +100,12 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: for d in [include_dir, src_dir, *include_flags]: if (path := (read_path / d)).is_dir(): lib.include_dirs.append(path.resolve()) + elif d in include_flags: + # The includeDir/srcDir defaults are probes; an explicit -I that + # does not resolve is a manifest or packaging error worth naming + _LOGGER.warning( + "Library %s declares include dir %s which does not exist", name, d + ) lib.sources = sorted( path.resolve() @@ -134,12 +140,11 @@ def resolve_libraries(framework_path: Path) -> list[ArduinoLibrary]: elif (framework_path / "libraries" / library.name).is_dir(): bundled.append(_bundled_library(framework_path, library.name)) else: - # Likely a typo or a library the build genuinely needs; a debug - # log here would surface only as a wall of include errors later. - _LOGGER.warning( - "Library %s is not bundled with the Arduino framework and has " - "no version or repository to download it from; skipping", - library.name, + # PlatformIO fails on an unresolvable lib_deps entry too; building + # without it would only surface as unrelated include/link errors. + raise EsphomeError( + f"Library {library.name} is not bundled with the Arduino " + "framework and has no version or repository to download it from" ) converted: list[ArduinoLibrary] = [] diff --git a/esphome/arduino8266/framework.py b/esphome/arduino8266/framework.py index 3a9dab4b87..32689ccc1a 100644 --- a/esphome/arduino8266/framework.py +++ b/esphome/arduino8266/framework.py @@ -56,13 +56,6 @@ ESPHOME_ARDUINO8266_FRAMEWORK_MIRRORS = str_to_lst_of_str( ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS = str_to_lst_of_str( os.environ.get("ESPHOME_ARDUINO8266_TOOLCHAIN_MIRRORS", "") ) -# Default ninja source; downloads from it verify against _NINJA_SHA256. -_NINJA_URL = ( - "https://github.com/ninja-build/ninja/releases/download/v{VERSION}/{ARCHIVE}" -) -ESPHOME_ARDUINO8266_NINJA_MIRRORS = str_to_lst_of_str( - os.environ.get("ESPHOME_ARDUINO8266_NINJA_MIRRORS", "") -) def get_arduino8266_tools_path() -> Path: @@ -143,6 +136,9 @@ def _registry_download( continue for file in ver.get("files", []): systems = file.get("system") or "*" + # A bare string would make ``in`` a substring test + if isinstance(systems, str) and systems != "*": + systems = [systems] if systems == "*" or system in systems: return ( file["download_url"], @@ -240,20 +236,27 @@ def get_build_env(toolchain_path: Path) -> dict[str, str]: def ccache_path() -> str | None: """The ccache binary to prefix compiles with, or None when disabled. - Same opt-out convention as the PlatformIO path: on by default when the - binary is on PATH, ``ESPHOME_CCACHE_ENABLE=0`` disables it. + Same convention as the PlatformIO path: on by default when the binary is + on PATH, ``ESPHOME_CCACHE_ENABLE=0`` disables it, and an explicit ``=1`` + warns when no binary is found and skips the runnability probe. """ from esphome.platformio.toolchain import _ccache_runs, _strip_win_long_path_prefix - if "ESPHOME_CCACHE_ENABLE" in os.environ and not get_bool_env( - "ESPHOME_CCACHE_ENABLE" - ): + explicit = "ESPHOME_CCACHE_ENABLE" in os.environ + if explicit and not get_bool_env("ESPHOME_CCACHE_ENABLE"): return None ccache = shutil.which("ccache") if ccache is None: + if explicit: + _LOGGER.warning( + "ESPHOME_CCACHE_ENABLE is set but no ccache binary is on PATH; " + "compiling without ccache" + ) return None ccache = _strip_win_long_path_prefix(ccache) - return ccache if _ccache_runs(ccache) else None + if not explicit and not _ccache_runs(ccache): + return None + return ccache def ccache_env() -> dict[str, str]: diff --git a/esphome/arduino8266/toolchain.py b/esphome/arduino8266/toolchain.py index 2896a3aba3..dbd24277ab 100644 --- a/esphome/arduino8266/toolchain.py +++ b/esphome/arduino8266/toolchain.py @@ -144,8 +144,13 @@ def _print_size_summary(build_dir: Path, toolchain_path: Path) -> None: try: sections[parts[0]] = int(parts[1]) except ValueError: - # An omitted section would silently skew the reported totals - _LOGGER.warning("Unparsable size output for section %s", parts[0]) + # A confident total built on a dropped section would feed a + # wrong number to CI's memory-impact metric; skip the summary. + _LOGGER.warning( + "Unparsable size output for section %s; skipping the size summary", + parts[0], + ) + return ram = sum(sections.get(s, 0) for s in _RAM_SECTIONS) flash = sum(sections.get(s, 0) for s in _FLASH_SECTIONS) print(f"RAM: {format_bar(ram, _MAX_RAM_SIZE)}") diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index b6ed132cf5..8c0b5495e7 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -183,6 +183,7 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig: for name, define in _NONOSDK_VERSIONS: if f"PIO_FRAMEWORK_ARDUINO_ESPRESSIF_{name}" in defines: nonosdk = define + break tcp_mss, features, ipv6, lwip_lib = _LWIP_DEFAULT for knob, variant in _LWIP_VARIANTS: @@ -291,18 +292,30 @@ def _defines_flags( ] -def _project_flags() -> tuple[list[str], list[str]]: - """Split the ESPHome build flags into (compile, linker) lists. +def _project_flags() -> tuple[list[str], list[str], list[Path], list[str]]: + """Split the ESPHome build flags into compile, linker, -L, and -l lists. Unlike ``framework_helpers.get_project_compile_flags`` this keeps every non-linker flag, matching how PlatformIO passes ``build_flags`` to the - compiler verbatim, and applies ``build_unflags``. + compiler verbatim, and applies ``build_unflags``. ``-L``/``-l`` are + classified out so they reach the link line as they do under PlatformIO. """ unflags = set(CORE.build_unflags) flags = [f for f in sorted(CORE.build_flags) if f not in unflags] - compile_flags = [f for f in flags if not f.startswith("-Wl,")] - link_flags = [f for f in flags if f.startswith("-Wl,")] - return compile_flags, link_flags + compile_flags: list[str] = [] + link_flags: list[str] = [] + lib_dirs: list[Path] = [] + libs: list[str] = [] + for flag in flags: + if flag.startswith("-Wl,"): + link_flags.append(flag) + elif flag.startswith("-L") and len(flag) > 2: + lib_dirs.append(Path(flag[2:])) + elif flag.startswith("-l") and len(flag) > 2: + libs.append(flag[2:]) + else: + compile_flags.append(flag) + return compile_flags, link_flags, lib_dirs, libs def _collect_sources(root: Path, exclude: set[str] = frozenset()) -> list[Path]: @@ -444,7 +457,12 @@ def write_project(paths: dict[str, Path]) -> bool: for lib in libraries: include_dirs += lib.include_dirs - project_compile_flags, project_link_flags = _project_flags() + ( + project_compile_flags, + project_link_flags, + project_lib_dirs, + project_libs, + ) = _project_flags() defines = _defines_flags( config, esp8266_data[KEY_FLASH_MODE], board, board_build["defines"] ) @@ -470,12 +488,14 @@ def write_project(paths: dict[str, Path]) -> bool: link_flags += ["-T", flash_ld] lib_dirs = [Path("ld"), sdk / "lib", sdk / "ld", sdk / "lib" / config.nonosdk] + lib_dirs += project_lib_dirs for lib in libraries: lib_dirs += lib.link_dirs system_libs = ( _SYSTEM_LIBS_PRE_LWIP + [config.lwip_lib] + _SYSTEM_LIBS_POST_LWIP + + project_libs + [lib_name for lib in libraries for lib_name in lib.link_libs] + ["stdc++-exc" if config.exceptions else "stdc++", "m", "c", "gcc"] ) diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index de77624756..1e743ae031 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -222,6 +222,8 @@ def test_write_project_link_line_and_exclusions(tmp_path: Path) -> None: "-Wl,--wrap=millis", "-Wl,--wrap=printf", "-Wno-nonnull-compare", + "-L/opt/blobs", + "-luser_blob", ) content = _write_ninja(paths) @@ -247,11 +249,19 @@ def test_write_project_link_line_and_exclusions(tmp_path: Path) -> None: assert "-T eagle.flash.4m.ld" in content # scanf float disabled: the forced-link flag must not appear assert "_scanf_float" not in content + # -L/-l from esphome build_flags reach the link line, not the compiles + assert '-L"/opt/blobs"' in content + assert "-luser_blob" in content + assert "cflags" not in [ + line.split(" = ")[0].strip() + for line in content.splitlines() + if "user_blob" in line + ] # System libraries with the selected lwIP variant, in the builder's order assert ( "-lhal -lphy -lpp -lnet80211 -llwip2-1460 -lwpa -lcrypto -lmain -lwps " - "-lbearssl -lespnow -lsmartconfig -lairkiss -lwpa2 -lstdc++ -lm -lc -lgcc" - in content + "-lbearssl -lespnow -lsmartconfig -lairkiss -lwpa2 -luser_blob " + "-lstdc++ -lm -lc -lgcc" in content ) # Core exclusions: native OTA backend and waveform stubs assert "Updater.cpp" not in content @@ -511,3 +521,12 @@ def test_write_project_missing_framework_dir_raises(tmp_path: Path) -> None: _set_flags() with pytest.raises(EsphomeError, match="incomplete.*lwip2"): _write_ninja(paths) + + +def test_build_config_nonosdk_precedence() -> None: + """With two SDK knobs set, the first table entry wins (documented order).""" + _set_flags( + "-DPIO_FRAMEWORK_ARDUINO_ESPRESSIF_SDK305", + "-DPIO_FRAMEWORK_ARDUINO_ESPRESSIF_SDK221", + ) + assert _resolve_build_config(_flag_defines()).nonosdk == "NONOSDK221" diff --git a/tests/unit_tests/test_arduino8266_component.py b/tests/unit_tests/test_arduino8266_component.py index e18c435fc8..f654ee57da 100644 --- a/tests/unit_tests/test_arduino8266_component.py +++ b/tests/unit_tests/test_arduino8266_component.py @@ -93,15 +93,21 @@ def test_library_info_no_src_dir(tmp_path: Path) -> None: assert lib.include_dirs == [read_path.resolve()] -def test_resolve_libraries_bundled_and_unknown( - tmp_path: Path, caplog: pytest.LogCaptureFixture -) -> None: +def test_resolve_libraries_bundled(tmp_path: Path) -> None: framework = _make_framework(tmp_path) _add_library("ESP8266WiFi", None) - _add_library("Typoo", None) # unknown bare name: skipped with a warning libs = component.resolve_libraries(framework) assert [lib.name for lib in libs] == ["ESP8266WiFi"] - assert "Typoo" in caplog.text + + +def test_resolve_libraries_unknown_bare_name_raises(tmp_path: Path) -> None: + """An unresolvable library fails the build by name, as PlatformIO does.""" + from esphome.core import EsphomeError + + framework = _make_framework(tmp_path) + _add_library("Typoo", None) + with pytest.raises(EsphomeError, match="Typoo"): + component.resolve_libraries(framework) def _converted(name: str, source_dir: Path, data: dict) -> ConvertedLibrary: @@ -205,3 +211,13 @@ def test_library_info_trailing_bare_flag_warns( assert lib.flags == ["-DA=1"] assert lib.link_libs == [] assert "Ignoring trailing '-l'" in caplog.text + + +def test_library_info_missing_explicit_include_warns( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + lib = component._library_info("x", read_path, {"build": {"flags": ["-Inope"]}}) + assert lib.include_dirs == [(read_path / "src").resolve()] + assert "include dir nope which does not exist" in caplog.text diff --git a/tests/unit_tests/test_arduino8266_framework.py b/tests/unit_tests/test_arduino8266_framework.py index bb1254c1f2..758ee3bf90 100644 --- a/tests/unit_tests/test_arduino8266_framework.py +++ b/tests/unit_tests/test_arduino8266_framework.py @@ -87,6 +87,21 @@ def test_registry_download_matches_system() -> None: ) +def test_registry_download_bare_string_system() -> None: + """A bare-string system tag is an exact match, not a substring test.""" + resp = _registry_response( + [ + {"system": "linux_x86", "download_url": "http://x/x86"}, + {"system": "linux_x86_64", "download_url": "http://x/x86_64"}, + ] + ) + with ( + patch("requests.get", return_value=resp), + patch.object(framework, "_pio_system", return_value="linux_x86_64"), + ): + assert framework._registry_download("pkg", "1.0.0")[0] == "http://x/x86_64" + + def test_registry_download_wildcard_system() -> None: resp = _registry_response([{"system": "*", "download_url": "http://x/any"}]) with patch("requests.get", return_value=resp): @@ -243,6 +258,25 @@ def test_ccache_path_ok(monkeypatch: pytest.MonkeyPatch) -> None: assert framework.ccache_path() == "/usr/bin/ccache" +def test_ccache_path_explicit_missing_binary_warns( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + monkeypatch.setenv("ESPHOME_CCACHE_ENABLE", "1") + with patch("shutil.which", return_value=None): + assert framework.ccache_path() is None + assert "no ccache binary is on PATH" in caplog.text + + +def test_ccache_path_explicit_skips_probe(monkeypatch: pytest.MonkeyPatch) -> None: + """An explicit opt-in trusts the binary without the runnability probe.""" + monkeypatch.setenv("ESPHOME_CCACHE_ENABLE", "1") + with ( + patch("shutil.which", return_value="/usr/bin/ccache"), + patch("esphome.platformio.toolchain._ccache_runs", side_effect=AssertionError), + ): + assert framework.ccache_path() == "/usr/bin/ccache" + + def test_ccache_env(tmp_path: Path) -> None: with patch.object(framework, "ccache_path", return_value=None): assert framework.ccache_env() == {} diff --git a/tests/unit_tests/test_arduino8266_toolchain.py b/tests/unit_tests/test_arduino8266_toolchain.py index ad2211fdf4..84ff8f7436 100644 --- a/tests/unit_tests/test_arduino8266_toolchain.py +++ b/tests/unit_tests/test_arduino8266_toolchain.py @@ -27,7 +27,6 @@ section size addr .text1 27489 1074790896 .rodata 2588 1073645504 .bss 26504 1073648096 -.comment abc 0 Total 401861 """ @@ -222,3 +221,20 @@ def test_run_compile_skips_compdb_when_ninja_unchanged(tmp_path: Path) -> None: # Present compile DB + unchanged build.ninja: skipped (build_dir / "compile_commands.json").write_text("[]") run(regenerate_expected=False) + + +def test_print_size_summary_unparsable_section_skips_summary( + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + caplog: pytest.LogCaptureFixture, +) -> None: + """A section that fails to parse must not produce a confident wrong total.""" + bad = _SIZE_OUTPUT + ".broken abc 0\n" + with patch.object( + toolchain.subprocess, + "run", + return_value=MagicMock(returncode=0, stdout=bad), + ): + toolchain._print_size_summary(tmp_path, tmp_path / "toolchain") + assert capsys.readouterr().out == "" + assert "Unparsable size output" in caplog.text