Address review: route versioned bare libraries to the registry, deterministic MMU order, scoped toolchain branches, diagnostics

esphome/__main__.py

@@ -813,7 +813,7 @@ def write_cpp_file() -> int:
         from esphome.build_gen import espidf
 
         espidf.write_project()
-    elif CORE.using_toolchain_arduino:
+    elif CORE.is_esp8266 and CORE.using_toolchain_arduino:
         # The ninja project is generated at compile time by
         # esphome.arduino8266.toolchain (it needs the downloaded framework).
         pass
@@ -968,7 +968,7 @@ def upload_using_esptool(
         flash_images = [
             FlashImage(path=toolchain.get_factory_firmware_path(), offset="0x0")
         ]
-    elif CORE.using_toolchain_arduino:
+    elif CORE.is_esp8266 and CORE.using_toolchain_arduino:
         # The native backend writes PlatformIO-compatible output paths, so the
         # shared property already points at the right file.
         flash_images = [FlashImage(path=CORE.firmware_bin, offset="0x0")]

esphome/arduino8266/component.py

@@ -78,7 +78,13 @@ def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary:
     include_flags: list[str] = []
     for tok in it:
         if tok in ("-I", "-L", "-l"):
-            tok += next(it, "")
+            arg = next(it, None)
+            if arg is None:
+                _LOGGER.warning(
+                    "Ignoring trailing '%s' in library %s build flags", tok, name
+                )
+                break
+            tok += arg
         if tok.startswith("-I"):
             include_flags.append(tok[2:])
         elif tok.startswith("-L"):
@@ -116,7 +122,14 @@ def resolve_libraries(framework_path: Path) -> list[ArduinoLibrary]:
     bundled: list[ArduinoLibrary] = []
     external: list[Library] = []
     for library in CORE.platformio_libraries.values():
-        if library.repository or not library.name or "/" in library.name:
+        # A version pin means a registry package ("pngle@1.1.0"), never a
+        # framework-bundled library.
+        if (
+            library.repository
+            or library.version
+            or not library.name
+            or "/" in library.name
+        ):
             external.append(library)
         elif (framework_path / "libraries" / library.name).is_dir():
             bundled.append(_bundled_library(framework_path, library.name))

esphome/arduino8266/toolchain.py

@@ -92,7 +92,8 @@ def _parse_app_size(build_dir: Path) -> int | None:
 
     try:
         ld_text = get_flash_ld_path(build_dir).read_text(encoding="utf-8")
-    except OSError:
+    except OSError as err:
+        _LOGGER.debug("Cannot read linker script for the Flash summary: %s", err)
         return None
     return segment_length(ld_text, "irom0_0_seg")
 
@@ -114,6 +115,7 @@ def _print_size_summary(build_dir: Path, toolchain_path: Path) -> None:
         close_fds=False,
     )
     if result.returncode != 0:
+        _LOGGER.warning("Could not summarize firmware size: %s", result.stderr)
         return
     sections: dict[str, int] = {}
     for line in result.stdout.splitlines():

esphome/build_gen/arduino8266.py

@@ -199,8 +199,10 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig:
     if "PIO_FRAMEWORK_ARDUINO_WAVEFORM_LOCKED_PHASE" in defines:
         knob_defines.append("WAVEFORM_LOCKED_PHASE=1")
 
+    # 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 defines if name.startswith("VTABLES_IN_")),
+        (name for name in sorted(defines) if name.startswith("VTABLES_IN_")),
         "VTABLES_IN_FLASH",
     )
 
@@ -233,7 +235,9 @@ def _resolve_build_config(defines: dict[str, str]) -> _BuildConfig:
                 "PIO_FRAMEWORK_ARDUINO_MMU_CUSTOM requires MMU_IRAM_SIZE and "
                 "MMU_ICACHE_SIZE build flags"
             )
-        mmu = [body for name, body in defines.items() if name.startswith("MMU_")]
+        # 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:
         mmu = ["MMU_IRAM_SIZE=0x8000", "MMU_ICACHE_SIZE=0x8000"]
 
@@ -419,6 +423,13 @@ def write_project(paths: dict[str, Path]) -> bool:
 
     libraries = resolve_libraries(framework)
 
+    # A missing framework-owned directory is a broken install; failing here
+    # names the path instead of producing a wall of include errors.
+    for required in (sdk / "include", core_dir, sdk / "lwip2" / "include", variant_dir):
+        if not required.is_dir():
+            raise EsphomeError(
+                f"Arduino framework install is incomplete: missing {required}"
+            )
     include_dirs = [
         src_dir,
         sdk / "include",

esphome/espidf/extra_script.py

@@ -62,7 +62,19 @@ def apply_extra_script(
     source_path = component.source_dir
     library_root = source_path.resolve()
     script_path = (source_path / extra_script).resolve()
-    if not script_path.is_relative_to(library_root) or not script_path.is_file():
+    if not script_path.is_relative_to(library_root):
+        _LOGGER.warning(
+            "Ignoring extraScript %s of library %s: it escapes the library directory",
+            extra_script,
+            component.name,
+        )
+        return
+    if not script_path.is_file():
+        _LOGGER.debug(
+            "extraScript %s of library %s not found; skipping",
+            extra_script,
+            component.name,
+        )
         return
     if callable(idf_target):
         idf_target = idf_target()

script/determine-jobs.py

@@ -634,13 +634,16 @@ ESP8266_NATIVE_TEST_COMPONENTS = frozenset(
     }
 )
 
-# Infrastructure whose changes always trigger the native ESP8266 compile test.
-ESP8266_NATIVE_TRIGGER_PATH_PREFIXES = ("esphome/arduino8266/",)
+# Infrastructure whose changes always trigger the native ESP8266 compile
+# test. esphome/espidf/ is included because the backend shares its idedata,
+# extra-script, and size-summary helpers.
+ESP8266_NATIVE_TRIGGER_PATH_PREFIXES = ("esphome/arduino8266/", "esphome/espidf/")
 ESP8266_NATIVE_TRIGGER_FILES = frozenset(
     {
         "esphome/build_gen/arduino8266.py",
         "esphome/components/esp8266/build_surgery.py",
         "esphome/components/esp8266/boards.py",
+        "esphome/platformio/library.py",
         "script/test_build_components.py",
         ".github/workflows/ci.yml",
     }

tests/script/test_determine_jobs.py

@@ -3080,6 +3080,9 @@ def test_esp8266_native_components_full_list_on_infra_change() -> None:
         ["esphome/arduino8266/framework.py"],
         ["esphome/build_gen/arduino8266.py"],
         ["esphome/components/esp8266/build_surgery.py"],
+        # Shared modules the native build depends on
+        ["esphome/espidf/idedata.py"],
+        ["esphome/platformio/library.py"],
     ):
         with (
             patch.object(determine_jobs, "changed_files", return_value=changed),
@@ -3108,7 +3111,8 @@ def test_esp8266_native_components_full_list_on_infra_change() -> None:
             ["wifi", "network"],
             [],
         ),
-        # ESP-IDF infra is not an esp8266-native trigger.
+        # The espidf build generator is not an esp8266-native trigger
+        # (only the shared esphome/espidf/ package is).
         (["esphome/build_gen/espidf.py"], [], []),
         (["README.md"], [], []),
     ],

tests/unit_tests/build_gen/test_arduino8266.py

@@ -126,7 +126,8 @@ def test_build_config_mmu_custom_requires_sizes() -> None:
         "-DMMU_ICACHE_SIZE=0x4000",
     )
     config = _resolve_build_config(_flag_defines())
-    assert sorted(config.mmu_defines) == [
+    # Emitted pre-sorted so build.ninja stays byte-stable across runs
+    assert config.mmu_defines == [
         "MMU_ICACHE_SIZE=0x4000",
         "MMU_IRAM_SIZE=0xC000",
     ]
@@ -487,3 +488,14 @@ def test_write_project_testing_mode(tmp_path: Path) -> None:
     content = _write_ninja(paths)
     assert "-T testing_eagle.flash.4m.ld" in content
     assert "ld/testing_eagle.flash.4m.ld" in content
+
+
+def test_write_project_missing_framework_dir_raises(tmp_path: Path) -> None:
+    """An incomplete framework install fails naming the missing path."""
+    import shutil
+
+    paths = _make_framework(tmp_path)
+    shutil.rmtree(paths["framework_path"] / "tools" / "sdk" / "lwip2")
+    _set_flags()
+    with pytest.raises(EsphomeError, match="incomplete.*lwip2"):
+        _write_ninja(paths)

tests/unit_tests/test_arduino8266_component.py

@@ -178,3 +178,27 @@ def test_resolve_libraries_bundled_dep_already_present(tmp_path: Path) -> None:
 
     # Wire appears once (from the explicit registration), not twice
     assert [lib.name for lib in libs] == ["Wire", "some__External"]
+
+
+def test_resolve_libraries_versioned_bare_name_is_external(tmp_path: Path) -> None:
+    """A bare name with a version pin ("pngle@1.1.0") is a registry package,
+    not a bundled library, and must reach the converter."""
+    framework = _make_framework(tmp_path)
+    _add_library("pngle", "1.1.0")
+
+    with patch.object(component, "convert_libraries", return_value=[]) as mock_convert:
+        component.resolve_libraries(framework)
+
+    (libraries, _backend), _ = mock_convert.call_args
+    assert [lib.name for lib in libraries] == ["pngle"]
+
+
+def test_library_info_trailing_bare_flag_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": ["-DA=1 -l"]}})
+    assert lib.flags == ["-DA=1"]
+    assert lib.link_libs == []
+    assert "Ignoring trailing '-l'" in caplog.text

tests/unit_tests/test_arduino8266_framework.py

@@ -251,12 +251,12 @@ def test_ccache_path_no_binary(monkeypatch: pytest.MonkeyPatch) -> None:
         assert framework.ccache_path() is None
 
 
-def test_ccache_path_probe_failure() -> None:
+def test_ccache_path_probe_failure(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.delenv("ESPHOME_CCACHE_ENABLE", raising=False)
     with (
         patch("shutil.which", return_value="/usr/bin/ccache"),
         patch("subprocess.run", side_effect=subprocess.SubprocessError),
     ):
-        os.environ.pop("ESPHOME_CCACHE_ENABLE", None)
         assert framework.ccache_path() is None
 
 

tests/unit_tests/test_arduino8266_toolchain.py

@@ -166,13 +166,18 @@ def test_print_size_summary_no_app_size(
 
 
 def test_print_size_summary_size_tool_failure(
-    tmp_path: Path, capsys: pytest.CaptureFixture[str]
+    tmp_path: Path,
+    capsys: pytest.CaptureFixture[str],
+    caplog: pytest.LogCaptureFixture,
 ) -> None:
     with patch.object(
-        toolchain.subprocess, "run", return_value=MagicMock(returncode=1, stdout="")
+        toolchain.subprocess,
+        "run",
+        return_value=MagicMock(returncode=1, stdout="", stderr="bad elf"),
     ):
         toolchain._print_size_summary(tmp_path, tmp_path / "toolchain")
     assert capsys.readouterr().out == ""
+    assert "Could not summarize firmware size" in caplog.text
 
 
 def test_get_idedata_delegates(tmp_path: Path) -> None:

tests/unit_tests/test_espidf_component.py

@@ -1267,3 +1267,17 @@ def test_apply_extra_script_pio_platform(tmp_path) -> None:
     c.data = {"build": {"extraScript": "extra.py"}}
     apply_extra_script(c, "esp8266", pio_platform="espressif8266")
     assert c.data["build"]["flags"] == ["-lespressif8266"]
+
+
+def test_apply_extra_script_missing_script_logged(tmp_path, caplog) -> None:
+    """A declared but absent extraScript is skipped with a diagnostic."""
+    import logging
+
+    c = IDFComponent("owner/name", "1.0", source=URLSource("http://dummy"))
+    c.path = tmp_path
+    c.data = {"build": {"extraScript": "nope.py"}}
+    with caplog.at_level(logging.DEBUG, logger="esphome.espidf.extra_script"):
+        from esphome.espidf.extra_script import apply_extra_script
+
+        apply_extra_script(c, "esp8266")
+    assert "not found" in caplog.text

tests/unit_tests/test_main.py

@@ -7186,3 +7186,24 @@ def test_write_cpp_file_platformio_toolchain_writes_project(tmp_path: Path) -> N
 
     mock_write_cpp.assert_called_once()
     mock_pio_project.assert_called_once()
+
+
+def test_write_cpp_file_arduino_toolchain_other_platform_falls_through(
+    tmp_path: Path,
+) -> None:
+    """The 'arduino' toolchain is ESP8266-only; other platforms keep the
+    PlatformIO project generation."""
+    setup_core(platform=PLATFORM_ESP32, tmp_path=tmp_path, name="test")
+    CORE.toolchain = Toolchain.ARDUINO
+
+    with (
+        patch("esphome.writer.write_cpp"),
+        patch("esphome.build_gen.platformio.write_project") as mock_pio_project,
+        patch.object(
+            type(CORE), "cpp_main_section", new_callable=PropertyMock
+        ) as mock_section,
+    ):
+        mock_section.return_value = ""
+        assert main.write_cpp_file() == 0
+
+    mock_pio_project.assert_called_once()