mirror of
https://github.com/esphome/esphome.git
synced 2026-08-22 22:26:21 +00:00
Never substitute a URL-pinned dependency; rename the header-suffix set; warn on real bundled suppression
A dependency whose version is a repository URL names one specific source, so it is now detected before the backend-provided skip and resolves as git instead of being replaced by the bundled copy. The library header-suffix set becomes LIBRARY_HEADER_SUFFIXES so it cannot shadow esphome.const's narrower HEADER_FILE_EXTENSIONS. Suppressing a genuinely bundled name on an external short-name match warns (an accidental collision would surface as link errors), a converted library with no sources and no headers warns by name, and the walk's unreachable nameless-dependency check is gone.
This commit is contained in:
+30
-20
@@ -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
|
||||
|
||||
@@ -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"}
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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"},
|
||||
},
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user