Merge branch 'esp8266-native-library-backend' into esp8266-native-build-spec

esphome/arduino/library.py

@@ -24,7 +24,7 @@ from esphome.platformio.extra_script import apply_extra_script
 from esphome.platformio.library import (
     DEFAULT_BUILD_INCLUDE_DIR,
     DEFAULT_BUILD_SRC_FILTER,
-    HEADER_FILE_EXTENSIONS,
+    LIBRARY_HEADER_SUFFIXES,
     SRC_FILE_EXTENSIONS,
     ConvertedLibrary,
     InvalidLibrary,
@@ -219,17 +219,18 @@ def _collect_lib_sources(
             len(dropped),
             ", ".join(sorted(dropped)),
         )
-    if (
+    if not lib.sources and not any(
-        not lib.sources
+        Path(f).suffix.lower() in LIBRARY_HEADER_SUFFIXES for f in matched
-        and ("srcFilter" in build or "srcDir" in build)
-        and not any(Path(f).suffix.lower() in HEADER_FILE_EXTENSIONS for f in matched)
     ):
-        # A declared filter matching nothing (or only inert files) is a
+        # Matched headers mean a header-only library; anything else with no
-        # manifest/tree problem; matched headers mean a header-only library
+        # sources yields an empty archive that fails far away at link
-        _LOGGER.warning(
+        if "srcFilter" in build or "srcDir" in build:
-            "Library %s declares srcFilter/srcDir but no source files matched",
+            _LOGGER.warning(
-            name,
+                "Library %s declares srcFilter/srcDir but no source files matched",
-        )
+                name,
+            )
+        else:
+            _LOGGER.warning("Library %s has no sources or headers", name)
 
 
 def _library_info(name: str, read_path: Path, data: dict) -> ArduinoLibrary:
@@ -284,7 +285,7 @@ def _bundled_library(framework_path: Path, name: str) -> ArduinoLibrary:
             )
     lib = _library_info(name, lib_dir, data)
     if not lib.sources and not any(
-        Path(p).suffix.lower() in HEADER_FILE_EXTENSIONS for p in walk_files(lib_dir)
+        Path(p).suffix.lower() in LIBRARY_HEADER_SUFFIXES for p in walk_files(lib_dir)
     ):
         # An empty or half-extracted bundled directory can never link; a
         # warning would scroll away and resurface as undefined symbols
@@ -390,14 +391,23 @@ def resolve_libraries(
                 )
                 continue
             if name in external_short_names:
-                # Deliberate when the external really is this library;
+                if _provided(name):
-                # attributable when the short-name match is accidental
+                    # A bundled copy really is suppressed; an accidental
-                _LOGGER.debug(
+                    # short-name collision would surface as link errors
-                    "Dependency %s of %s assumed satisfied by a requested "
+                    _LOGGER.warning(
-                    "external library",
+                        "Dependency %s of %s is assumed satisfied by a "
-                    name,
+                        "requested external library; the bundled copy is "
-                    component.name,
+                        "not added",
-                )
+                        name,
+                        component.name,
+                    )
+                else:
+                    _LOGGER.debug(
+                        "Dependency %s of %s assumed satisfied by a requested "
+                        "external library",
+                        name,
+                        component.name,
+                    )
                 continue
             if name in bundled_names or is_lib_ignored(name, lib_ignore):
                 continue

esphome/platformio/library.py

@@ -74,7 +74,7 @@ SOURCE_KIND_FOR_SUFFIX: dict[str, str] = {
 SRC_FILE_EXTENSIONS = list(SOURCE_KIND_FOR_SUFFIX)
 # Suffixes that count as headers when probing whether a library has any
 # usable files at all (compare against Path.suffix.lower())
-HEADER_FILE_EXTENSIONS = frozenset(
+LIBRARY_HEADER_SUFFIXES = frozenset(
     {".h", ".hpp", ".hh", ".hxx", ".inc", ".ipp", ".tcc"}
 )
 
@@ -1124,7 +1124,7 @@ def convert_libraries(
             for dependency in normalize_dependencies(
                 component.data.get("dependencies"), component.name
             ):
-                if "name" not in dependency or "version" not in dependency:
+                if "version" not in dependency:
                     # Version-less deps cannot resolve from the registry; the
                     # post-emit reconciliation owns the drop warning. An
                     # is_lib_ignored name is deliberately excluded, not a drop.
@@ -1158,30 +1158,32 @@ def convert_libraries(
                 if is_lib_ignored(dep_name, lib_ignore):
                     _LOGGER.debug("Skip ignored dependency %s", dep_name)
                     continue
-                if (
+                # The version field may actually be a URL (git/archive
+                # dependency), which names one specific source; it must not
+                # be substituted with a same-named bundled library below.
+                dep_version = dependency["version"]
+                dep_url = _url_or_none(dep_version)
+                if dep_url is not None:
+                    dep_version = None
+                elif (
                     backend.provides is not None
                     and not dependency.get("owner")
                     and backend.provides(dep_name)
                 ):
                     # The backend adds it from its own tree; resolving it here
                     # would fetch a same-named registry package instead
-                    if (pin := dependency.get("version")) and pin != "*":
+                    if dep_version and dep_version != "*":
                         # The version pin is discarded for the bundled copy;
                         # make the substitution visible
                         _LOGGER.warning(
                             "Dependency %s pins version %s; using the library "
                             "bundled with the framework instead",
                             dep_name,
-                            pin,
+                            dep_version,
                         )
                     else:
                         _LOGGER.debug("Skip backend-provided dependency %s", dep_name)
                     continue
-                # The version field may actually be a URL (git/archive dependency).
-                dep_version = dependency["version"]
-                dep_url = _url_or_none(dep_version)
-                if dep_url is not None:
-                    dep_version = None
                 dep_key = add_spec(dep_name, dep_version, dep_url)
                 node.edges.add(dep_key)
                 worklist.append(dep_key)

tests/unit_tests/test_arduino_library.py

@@ -188,14 +188,16 @@ def test_library_info_declared_filter_matches_nothing_warns(
     assert "declares srcFilter/srcDir but no source files matched" in caplog.text
 
 
-def test_library_info_header_only_does_not_warn(
+def test_library_info_empty_tree_warns(
     tmp_path: Path, caplog: pytest.LogCaptureFixture
 ) -> None:
+    """No sources and no headers is an empty archive waiting to fail at
+    link; warn by name even without a declared filter."""
     read_path = tmp_path / "lib"
     (read_path / "src").mkdir(parents=True)
     lib = component._library_info("x", read_path, {})
     assert not lib.sources
-    assert "no source files matched" not in caplog.text
+    assert "has no sources or headers" in caplog.text
 
 
 def test_library_info_no_src_dir(tmp_path: Path) -> None:
@@ -458,6 +460,26 @@ def test_nonplatform_rejection_warns_once_through_real_converter(
     assert caplog.text.count("manifest is corrupt") == 1
 
 
+def test_short_name_collision_with_bundled_name_warns(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """Suppressing a genuinely bundled name on a short-name match warns;
+    an accidental collision would otherwise surface at link."""
+    framework = _make_framework(tmp_path)
+    _add_library("Someone/Wire", "1.0.0")
+    converted = _converted(
+        "someone__Wire",
+        tmp_path / "conv",
+        {"build": {}, "dependencies": [{"name": "Wire"}]},
+    )
+    (tmp_path / "conv" / "src").mkdir(parents=True)
+    with _emitting_converter(converted):
+        libs = _resolve(framework)
+    assert "Wire" not in [lib.name for lib in libs]
+    assert "assumed satisfied by a requested external library" in caplog.text
+    assert any(r.levelname == "WARNING" for r in caplog.records)
+
+
 def test_missing_libraries_dir_is_a_broken_install(tmp_path: Path) -> None:
     """A framework tree without libraries/ must fail by name, not silently
     reroute every bundled name to the registry."""

tests/unit_tests/test_main.py

@@ -11,24 +11,28 @@ from pathlib import Path
 import re
 import sys
 import time
+from types import SimpleNamespace
 from typing import Any, Self
 from unittest.mock import AsyncMock, MagicMock, Mock, patch
 
 import pytest
 from pytest import CaptureFixture
+import serial
 from zeroconf import ServiceStateChange
 
-from esphome import __main__ as main
+from esphome import __main__ as main, yaml_util
 from esphome.__main__ import (
     Purpose,
     _get_configured_xtal_freq,
     _make_crystal_freq_callback,
     _redact_with_legacy_fallback,
     _resolve_network_devices,
+    _should_subscribe_states,
     _split_network_devices,
     _unresolved_default_error,
     _validate_bootloader_binary,
     _validate_partition_table_binary,
+    _wrap_to_code,
     check_permissions,
     choose_upload_log_host,
     command_analyze_memory,
@@ -67,19 +71,21 @@ from esphome.__main__ import (
 )
 from esphome.address_cache import AddressCache
 from esphome.bundle import BUNDLE_EXTENSION, BundleFile, BundleResult
-from esphome.components import esp32, esp8266
+from esphome.components import esp32, esp8266, mqtt
 from esphome.components.esp32 import (
     KEY_ESP32,
     KEY_VARIANT,
     VARIANT_ESP32,
     get_esp32_variant,
 )
+from esphome.config import Config
 from esphome.const import (
     CONF_API,
     CONF_AUTH,
     CONF_BAUD_RATE,
     CONF_BROKER,
     CONF_DISABLED,
+    CONF_DISCOVER_IP,
     CONF_ESPHOME,
     CONF_LEVEL,
     CONF_LOG,
@@ -565,8 +571,6 @@ def test_command_config__no_defaults_dumps_user_snapshot(
 ) -> None:
     """``--no-defaults`` dumps ``config.user_config`` instead of the
     validated config, so schema defaults don't leak into the output."""
-    from esphome.config import Config
-
     setup_core(tmp_path=tmp_path, config={"esphome": {"name": "test"}})
     args = MockArgs()
     args.show_secrets = True
@@ -619,8 +623,6 @@ def test_command_config__no_defaults_skips_strip_default_ids(
 ) -> None:
     """When ``--no-defaults`` is set, ``strip_default_ids`` isn't run --
     the user snapshot is already free of schema-injected IDs."""
-    from esphome.config import Config
-
     setup_core(tmp_path=tmp_path, config={"esphome": {"name": "test"}})
     args = MockArgs()
     args.show_secrets = True
@@ -3438,9 +3440,6 @@ def test_get_port_type() -> None:
 
 def test_mqtt_reexports_discover_ip() -> None:
     """The old import path must keep working for external code."""
-    from esphome.components import mqtt
-    from esphome.const import CONF_DISCOVER_IP
-
     assert mqtt.CONF_DISCOVER_IP is CONF_DISCOVER_IP
 
 
@@ -5907,8 +5906,6 @@ class MockSerial:
             chunk = self.chunks[self.chunk_index]
             if chunk is MOCK_SERIAL_END:
                 # Sentinel means we're done - simulate port closed
-                import serial
-
                 raise serial.SerialException("Port closed")
             # Respect the requested size and keep any remaining bytes
             if size <= 0:
@@ -5922,8 +5919,6 @@ class MockSerial:
                 # Entire chunk consumed; advance to the next one
                 self.chunk_index += 1
             return data  # type: ignore[return-value]
-        import serial
-
         raise serial.SerialException("Port closed")
 
 
@@ -6782,8 +6777,6 @@ def test_parse_args_argcomplete_only_runs_when_completing() -> None:
 
 def test_should_subscribe_states_default() -> None:
     """Test that states are shown by default when nothing is set."""
-    from esphome.__main__ import _should_subscribe_states
-
     args = parse_args(["esphome", "logs", "device.yaml"])
     with patch.dict(os.environ, {}, clear=False):
         os.environ.pop("ESPHOME_LOG_STATES", None)
@@ -6792,8 +6785,6 @@ def test_should_subscribe_states_default() -> None:
 
 def test_should_subscribe_states_env_suppresses() -> None:
     """Test that ESPHOME_LOG_STATES=false suppresses states by default."""
-    from esphome.__main__ import _should_subscribe_states
-
     args = parse_args(["esphome", "logs", "device.yaml"])
     with patch.dict(os.environ, {"ESPHOME_LOG_STATES": "false"}):
         assert _should_subscribe_states(args) is False
@@ -6801,8 +6792,6 @@ def test_should_subscribe_states_env_suppresses() -> None:
 
 def test_should_subscribe_states_env_enables() -> None:
     """Test that ESPHOME_LOG_STATES=true enables states by default."""
-    from esphome.__main__ import _should_subscribe_states
-
     args = parse_args(["esphome", "logs", "device.yaml"])
     with patch.dict(os.environ, {"ESPHOME_LOG_STATES": "true"}):
         assert _should_subscribe_states(args) is True
@@ -6810,8 +6799,6 @@ def test_should_subscribe_states_env_enables() -> None:
 
 def test_should_subscribe_states_flag_overrides_env() -> None:
     """Test that --states overrides ESPHOME_LOG_STATES=false."""
-    from esphome.__main__ import _should_subscribe_states
-
     args = parse_args(["esphome", "logs", "--states", "device.yaml"])
     with patch.dict(os.environ, {"ESPHOME_LOG_STATES": "false"}):
         assert _should_subscribe_states(args) is True
@@ -6819,8 +6806,6 @@ def test_should_subscribe_states_flag_overrides_env() -> None:
 
 def test_should_subscribe_states_no_flag_overrides_env() -> None:
     """Test that --no-states overrides ESPHOME_LOG_STATES=true."""
-    from esphome.__main__ import _should_subscribe_states
-
     args = parse_args(["esphome", "logs", "--no-states", "device.yaml"])
     with patch.dict(os.environ, {"ESPHOME_LOG_STATES": "true"}):
         assert _should_subscribe_states(args) is False
@@ -7250,11 +7235,6 @@ async def test_wrap_to_code_comment_is_insertion_order_independent() -> None:
     """The config comment dumps with sorted keys: voluptuous fills schema
     defaults in set-iteration order, so an unsorted dump would churn
     main.cpp and relink the firmware on every run."""
-    from types import SimpleNamespace
-
-    from esphome import yaml_util
-    from esphome.__main__ import _wrap_to_code
-
     comments: list[str] = []
 
     async def to_code(conf):

tests/unit_tests/test_platformio_library.py

@@ -814,6 +814,34 @@ def test_versionless_dependency_without_provider_warns(
     )
 
 
+def test_url_version_dependency_is_not_substituted_by_provides(
+    tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture
+) -> None:
+    """A URL-valued version names one specific source; the backend-provided
+    skip must not replace it with the bundled copy."""
+    _patch_download_with_manifests(
+        monkeypatch,
+        tmp_path,
+        {
+            "esphome/A": {
+                "name": "A",
+                "dependencies": [
+                    {"name": "Hash", "version": "https://github.com/o/Hash.git"}
+                ],
+            },
+            "o/Hash": {"name": "Hash"},
+        },
+    )
+    emitted: list[str] = []
+    convert_libraries(
+        [Library("esphome/A", "1.0.0", None)],
+        _backend(emit=lambda c: emitted.append(c.name), provides=lambda name: True),
+    )
+    assert "Skip backend-provided" not in caplog.text
+    assert "using the library bundled" not in caplog.text
+    assert any("o/hash" in n.lower() for n in emitted)
+
+
 def test_versionless_owner_qualified_dependency_warns_despite_provides(
     tmp_path, monkeypatch, caplog: pytest.LogCaptureFixture
 ) -> None:
@@ -899,10 +927,7 @@ def test_versionless_dependency_matching_resolved_manifest_name_stays_quiet(
         monkeypatch,
         tmp_path,
         {
-            "esphome/A": {
+            "esphome/A": {"name": "A", "dependencies": [{"name": "B"}]},
-                "name": "A",
-                "dependencies": [{"name": "B"}, {"version": "1.0"}],
-            },
             "esphome/B": {"name": "B"},
         },
     )