diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index dee58b5fed..f699d79704 100644 --- a/esphome/arduino/library.py +++ b/esphome/arduino/library.py @@ -32,6 +32,7 @@ from esphome.platformio.library import ( DEFAULT_BUILD_SRC_FILTER, SRC_FILE_EXTENSIONS, ConvertedLibrary, + IncompatiblePlatform, InvalidLibrary, LibraryBackend, check_library_data, @@ -73,16 +74,19 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: build = data.get("build", {}) # PIO's source-dir resolution: manifest srcDir, else src/Src, else the root - src_dir = build.get("srcDir") or next( - (d for d in ("src", "Src") if (read_path / d).is_dir()), "." - ) - if "srcDir" in build and not (read_path / src_dir).is_dir(): - # Unlike the default probes, an explicitly declared srcDir that does - # not resolve is unambiguously a manifest/tree error; a silently - # empty source set would surface as link errors far from the cause - raise EsphomeError( - f"Library {name} declares srcDir {src_dir} which does not exist" - ) + if "srcDir" in build: + # An explicitly declared srcDir (falsy included) that does not + # resolve is unambiguously a manifest/tree error; a silently empty + # source set would surface as link errors far from the cause + src_dir = build["srcDir"] + if not ( + isinstance(src_dir, str) and src_dir and (read_path / src_dir).is_dir() + ): + raise EsphomeError( + f"Library {name} declares srcDir {src_dir!r} which does not exist" + ) + else: + src_dir = next((d for d in ("src", "Src") if (read_path / d).is_dir()), ".") src_filter = ensure_list(build.get("srcFilter", DEFAULT_BUILD_SRC_FILTER)) # PlatformIO shell-lexes each build.flags entry @@ -92,7 +96,20 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary: # deliberate extra (Arduino IDE's property, which PIO ignores) so # properties-only libraries can opt out of archiving too if "libArchive" in build: - lib_archive = bool(build["libArchive"]) + raw_archive = build["libArchive"] + if isinstance(raw_archive, bool): + lib_archive = raw_archive + elif str(raw_archive).strip().lower() in ("true", "false"): + lib_archive = str(raw_archive).strip().lower() == "true" + else: + # bool("false") is True; an unparsable value must not silently + # archive a library whose author disabled archiving + _LOGGER.warning( + "Library %s has an unrecognized libArchive value %r; assuming true", + name, + raw_archive, + ) + lib_archive = True elif "dot_a_linkage" in data: lib_archive = str(data["dot_a_linkage"]).lower() == "true" else: @@ -192,9 +209,10 @@ def resolve_libraries( and "/" not in library.name and (framework_path / "libraries" / library.name).is_dir() ): - # A bundled library's own manifest dependencies are deliberately - # not walked (PlatformIO's lib_ldf_mode=off does not either); - # core add_library() calls list what they need explicitly. + # A bundled library's own manifest dependencies are not walked. + # PlatformIO would walk them even under lib_ldf_mode=off, but no + # library bundled with the ESP8266 core declares any, so the walk + # is a no-op there; core add_library() calls list what they need. bundled.append(_bundled_library(framework_path, library.name)) else: external.append(library) @@ -217,11 +235,13 @@ def resolve_libraries( continue if name in bundled_names or is_lib_ignored(name, lib_ignore): continue - if "version" in dep: - # The converter resolves versioned deps from the registry. - # Note: the dict shorthand {"Wire": "*"} normalizes to - # version="*" and takes this path even for a bundled name; - # use the list form for bundled dependencies. + bundled_dir = framework_path / "libraries" / name + if "version" in dep and (dep.get("owner") or not bundled_dir.is_dir()): + # The converter resolves versioned deps from the registry. An + # owner-less versioned name that exists in the framework tree + # ({"Wire": "*"} normalizes to version="*") falls through to + # the bundled path below, matching PlatformIO's + # process_dependencies preference for bundled builders. continue if dep.get("owner"): # Owner but no version: the converter skips it too, so this @@ -233,7 +253,7 @@ def resolve_libraries( component.name, ) continue - if not (framework_path / "libraries" / name).is_dir(): + if not bundled_dir.is_dir(): # The shared converter skips version-less deps too, so this # is the only place the drop can be made visible before the # missing sources surface as link errors. @@ -251,7 +271,7 @@ def resolve_libraries( # manifest is routine (every ESPAsyncWebServer build hits # it), so the platform filter stays at debug; any other # cause means a dropped dependency and must be visible - if "platform" in str(err).lower(): + if isinstance(err, IncompatiblePlatform): _LOGGER.debug("Skipping bundled dependency %s: %s", name, err) else: _LOGGER.warning( @@ -287,12 +307,15 @@ def resolve_libraries( ) if len(resolved) < len(external): # A requested library the converter dropped would otherwise - # surface only as link errors far from the cause + # surface only as link errors far from the cause; name the + # requests that went missing, not just the survivors + resolved_names = {c.name for c in resolved} + dropped = [str(lib) for lib in external if lib.name not in resolved_names] _LOGGER.warning( - "%d of %d requested libraries were not resolved (resolved: %s)", + "%d of %d requested libraries were not resolved (missing: %s)", len(external) - len(resolved), len(external), - ", ".join(sorted(c.name for c in resolved)) or "none", + ", ".join(sorted(dropped)) or "unknown", ) return bundled + converted diff --git a/esphome/build_gen/arduino8266.py b/esphome/build_gen/arduino8266.py index 2716ed3bc7..2b3750eb54 100644 --- a/esphome/build_gen/arduino8266.py +++ b/esphome/build_gen/arduino8266.py @@ -256,10 +256,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) @@ -278,13 +283,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) @@ -361,7 +367,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] = [] @@ -435,11 +444,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 @@ -472,10 +490,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/esphome/platformio/library.py b/esphome/platformio/library.py index 7349c46144..34c7a272cc 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -209,6 +209,14 @@ class InvalidLibrary(Exception): pass +class IncompatiblePlatform(InvalidLibrary): + """The manifest's platform filter rejected the target platform. + + A distinct type so callers can treat the routine cross-platform skip + differently from other manifest problems without matching message text. + """ + + class ConvertedLibrary: """A resolved PlatformIO library plus its parsed manifest and on-disk path. @@ -438,7 +446,7 @@ def check_library_data(data: dict, platform: str | None, framework: str): valid_platforms = platform is None or "*" in platforms or platform in platforms if not valid_platforms: - raise InvalidLibrary(f"Unsupported library platforms: {platforms}") + raise IncompatiblePlatform(f"Unsupported library platforms: {platforms}") frameworks = data.get("frameworks", "*") if isinstance(frameworks, str): diff --git a/tests/unit_tests/build_gen/test_arduino8266.py b/tests/unit_tests/build_gen/test_arduino8266.py index 1f5d02858c..64bfc5e3bd 100644 --- a/tests/unit_tests/build_gen/test_arduino8266.py +++ b/tests/unit_tests/build_gen/test_arduino8266.py @@ -168,7 +168,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) @@ -404,9 +404,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] @@ -431,6 +436,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) @@ -452,7 +471,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() @@ -718,15 +737,14 @@ def test_write_project_missing_src_dir_raises(tmp_path: Path) -> None: arduino8266.write_project(paths) -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: @@ -881,15 +899,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( diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index f0bff7b729..59a1f9fa45 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/tests/unit_tests/test_arduino_library.py @@ -291,7 +291,7 @@ def test_library_info_missing_declared_src_dir_raises(tmp_path: Path) -> None: """An explicitly declared srcDir that does not exist is a manifest error.""" read_path = tmp_path / "lib" read_path.mkdir() - with pytest.raises(EsphomeError, match="srcDir nosrc which does not exist"): + with pytest.raises(EsphomeError, match="srcDir 'nosrc' which does not exist"): component._library_info("x", read_path, {"build": {"srcDir": "nosrc"}}) @@ -425,6 +425,8 @@ def test_resolve_libraries_warns_when_converter_drops_a_request( cache_key="arduino8266", ) assert "1 of 1 requested libraries were not resolved" in caplog.text + # The actionable fact is which request went missing, not the survivors + assert "missing: pngle" in caplog.text def test_bundled_dependency_nonplatform_rejection_warns( @@ -458,3 +460,95 @@ def test_bundled_dependency_nonplatform_rejection_warns( ) assert "Skipping bundled dependency Wire" in caplog.text assert "manifest is corrupt" in caplog.text + + +@pytest.mark.parametrize("declared", ["", None]) +def test_library_info_falsy_declared_src_dir_raises( + tmp_path: Path, declared: str | None +) -> None: + """A declared-but-falsy srcDir must not silently fall back to the probe.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + with pytest.raises(EsphomeError, match="does not exist"): + component._library_info("x", read_path, {"build": {"srcDir": declared}}) + + +@pytest.mark.parametrize( + ("value", "expected", "warns"), + [ + (False, False, False), + ("false", False, False), + ("False", False, False), + ("true", True, False), + ("archive-me", True, True), + ], +) +def test_library_info_lib_archive_parse( + tmp_path: Path, + value: object, + expected: bool, + warns: bool, + caplog: pytest.LogCaptureFixture, +) -> None: + """bool("false") is True; the string forms must parse, not coerce.""" + read_path = tmp_path / "lib" + (read_path / "src").mkdir(parents=True) + lib = component._library_info("x", read_path, {"build": {"libArchive": value}}) + assert lib.lib_archive is expected + assert ("unrecognized libArchive" in caplog.text) is warns + + +def test_bundled_dependency_dict_shorthand_prefers_bundled(tmp_path: Path) -> None: + """The {"Wire": "*"} dict shorthand (version="*", no owner) must resolve + to the bundled library, matching PIO's process_dependencies, instead of + being routed to the registry.""" + framework = _make_framework(tmp_path) + _add_library("ESP32Async/ESPAsyncWebServer", "3.9.6") + lib_dir = tmp_path / "converted" / "webserver" + (lib_dir / "src").mkdir(parents=True) + converted = _converted( + "esp32async__ESPAsyncWebServer", + lib_dir, + {"build": {}, "dependencies": {"Wire": "*"}}, + ) + with _emitting_converter(converted): + libs = component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) + assert "Wire" in [lib.name for lib in libs] + + +def test_bundled_dependency_platform_rejection_is_debug( + tmp_path: Path, caplog: pytest.LogCaptureFixture +) -> None: + """The typed IncompatiblePlatform (the routine cross-platform skip) + stays at debug regardless of message wording.""" + from esphome.platformio.library import IncompatiblePlatform + + framework = _make_framework(tmp_path) + _add_library("ESP32Async/ESPAsyncWebServer", "3.9.6") + lib_dir = tmp_path / "converted" / "webserver" + (lib_dir / "src").mkdir(parents=True) + converted = _converted( + "esp32async__ESPAsyncWebServer", + lib_dir, + {"build": {}, "dependencies": [{"name": "Wire"}]}, + ) + with ( + _emitting_converter(converted), + patch.object( + component, + "check_library_data", + side_effect=IncompatiblePlatform("nothing about the p-word here"), + ), + ): + component.resolve_libraries( + framework, + pio_platform="espressif8266", + board_mcu="esp8266", + cache_key="arduino8266", + ) + assert "Skipping bundled dependency Wire" not in caplog.text