diff --git a/esphome/arduino/library.py b/esphome/arduino/library.py index 869a939f7c..6932196207 100644 --- a/esphome/arduino/library.py +++ b/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 ( - not lib.sources - and ("srcFilter" in build or "srcDir" in build) - and not any(Path(f).suffix.lower() in HEADER_FILE_EXTENSIONS for f in matched) + if not lib.sources and not any( + Path(f).suffix.lower() in LIBRARY_HEADER_SUFFIXES for f in matched ): - # A declared filter matching nothing (or only inert files) is a - # manifest/tree problem; matched headers mean a header-only library - _LOGGER.warning( - "Library %s declares srcFilter/srcDir but no source files matched", - name, - ) + # Matched headers mean a header-only library; anything else with no + # sources yields an empty archive that fails far away at link + if "srcFilter" in build or "srcDir" in build: + _LOGGER.warning( + "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; - # attributable when the short-name match is accidental - _LOGGER.debug( - "Dependency %s of %s assumed satisfied by a requested " - "external library", - name, - component.name, - ) + if _provided(name): + # A bundled copy really is suppressed; an accidental + # short-name collision would surface as link errors + _LOGGER.warning( + "Dependency %s of %s is assumed satisfied by a " + "requested external library; the bundled copy is " + "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 diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 9ec099a63a..097a07b8d2 100644 --- a/esphome/platformio/library.py +++ b/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) diff --git a/tests/unit_tests/test_arduino_library.py b/tests/unit_tests/test_arduino_library.py index 87dbd9b088..17315e9a27 100644 --- a/tests/unit_tests/test_arduino_library.py +++ b/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.""" diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index b848ac1b53..7638447af7 100644 --- a/tests/unit_tests/test_main.py +++ b/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): diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 8662e13639..ee73384383 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/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": { - "name": "A", - "dependencies": [{"name": "B"}, {"version": "1.0"}], - }, + "esphome/A": {"name": "A", "dependencies": [{"name": "B"}]}, "esphome/B": {"name": "B"}, }, )