From 735c64d1386a4240e5adc53ff55edd1b5d180680 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:48:57 -0400 Subject: [PATCH] [core] Support local library directories via file:// on the native toolchain (#18005) --- esphome/components/zephyr/library.py | 18 +- esphome/core/config.py | 6 +- esphome/espidf/component.py | 44 +++- esphome/platformio/library.py | 242 +++++++++++++++++--- tests/unit_tests/core/test_config.py | 9 + tests/unit_tests/test_espidf_component.py | 176 +++++++++++--- tests/unit_tests/test_platformio_library.py | 199 +++++++++++++++- tests/unit_tests/test_zephyr_library.py | 5 +- 8 files changed, 608 insertions(+), 91 deletions(-) diff --git a/esphome/components/zephyr/library.py b/esphome/components/zephyr/library.py index 7654e63700..0e6551ccf1 100644 --- a/esphome/components/zephyr/library.py +++ b/esphome/components/zephyr/library.py @@ -65,10 +65,16 @@ def generate_cmakelists_txt(component: ConvertedLibrary) -> str: """ build = component.data.get("build", {}) + # The library's own files live in source_path (the user's directory for a + # local library, the downloaded dir otherwise); the generated zephyr/ files + # go under component.path. Sources are already emitted as absolute paths, so + # they resolve correctly wherever source_path points. + read_path = component.source_dir + build_src_dir = build.get("srcDir") if not build_src_dir: for d in ["src", "Src", "."]: - if (component.path / Path(d)).is_dir(): + if (read_path / Path(d)).is_dir(): build_src_dir = d break @@ -77,7 +83,7 @@ def generate_cmakelists_txt(component: ConvertedLibrary) -> str: build_flags = ensure_list(build.get("flags", DEFAULT_BUILD_FLAGS)) src_files = collect_filtered_files( - component.path / Path(build_src_dir), build_src_filter + read_path / Path(build_src_dir), build_src_filter ) src_files = sorted( str(Path(p).resolve()) @@ -91,15 +97,19 @@ def generate_cmakelists_txt(component: ConvertedLibrary) -> str: link_directories, build_flags = split_list_by_condition( build_flags, lambda a: a[2:].strip() if a.startswith("-L") else None ) + # The zephyr/CMakeLists lives in a subdir, so a relative -L would resolve + # from there rather than the library root; make link dirs absolute against + # the library's own directory (source_dir), matching src/include handling. + link_directories = [str((read_path / Path(d)).resolve()) for d in link_directories] link_libraries, build_flags = split_list_by_condition( build_flags, lambda a: a[2:].strip() if a.startswith("-l") else None ) include_dirs = [build_include_dir, build_src_dir, *include_dir_flags] include_dirs = [ - str((component.path / Path(d)).resolve()) + str((read_path / Path(d)).resolve()) for d in include_dirs - if (component.path / Path(d)).is_dir() + if (read_path / Path(d)).is_dir() ] lines = [f"zephyr_library_named({component.get_require_name()})"] diff --git a/esphome/core/config.py b/esphome/core/config.py index 6b24a55487..1095a4886e 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -542,8 +542,10 @@ def _add_library_str(lib: str) -> None: if "@" in lib: name, vers = lib.split("@", 1) cg.add_library(name, vers) - elif "://" in lib: - # Repository... + elif "://" in lib or lib.split("=", 1)[-1].startswith("file:"): + # A repository or URL source. Also catch a ``file:`` source spelled with + # fewer than two slashes (e.g. ``file:lib_dev``) so it reaches the + # file:// handling and its clear error, rather than a registry lookup. if "=" in lib: name, repo = lib.split("=", 1) cg.add_library(name, None, repo) diff --git a/esphome/espidf/component.py b/esphome/espidf/component.py index cad5bbf665..b22b39bf6d 100644 --- a/esphome/espidf/component.py +++ b/esphome/espidf/component.py @@ -46,10 +46,11 @@ def _apply_extra_script(component: IDFComponent) -> None: extra_script = component.data.get("build", {}).get("extraScript") if not extra_script: return - # Resolve and confine to the component dir so a malicious library.json - # can't escape (e.g. ``"extraScript": "../../etc/passwd"``). - library_root = component.path.resolve() - script_path = (component.path / extra_script).resolve() + # Resolve and confine to the library's source dir so a malicious + # library.json can't escape (e.g. ``"extraScript": "../../etc/passwd"``). + 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(): return from esphome.components.esp32 import get_esp32_variant @@ -58,9 +59,9 @@ def _apply_extra_script(component: IDFComponent) -> None: idf_target = variant_to_idf_target(get_esp32_variant()) result = run_extra_script( - script_path, library_dir=component.path, idf_target=idf_target + script_path, library_dir=source_path, idf_target=idf_target ) - extra_flags = captured_as_build_flags(result, library_dir=component.path) + extra_flags = captured_as_build_flags(result, library_dir=source_path) if not extra_flags: return flags = component.data.setdefault("build", {}).setdefault("flags", []) @@ -101,11 +102,17 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: # which Windows accepts too, so the generated CMakeLists is portable. return f'"{str(p).replace(os.sep, "/")}"' + # The library's own files live in source_path (the user's directory for a + # local library, the downloaded dir otherwise). When it differs from the + # component dir the CMakeLists must reference sources by absolute path. + read_path = component.source_dir + external = read_path.resolve() != component.path.resolve() + # Extract the values build_src_dir = component.data.get("build", {}).get("srcDir", None) if not build_src_dir: for d in ["src", "Src", "."]: - if (component.path / Path(d)).is_dir(): + if (read_path / Path(d)).is_dir(): build_src_dir = d break @@ -138,7 +145,7 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: # List all sources files build_src_files = collect_filtered_files( - component.path / Path(build_src_dir), build_src_filter + read_path / Path(build_src_dir), build_src_filter ) # Only bake library.json-declared deps here. Project-managed and @@ -150,8 +157,12 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: dependency.get_require_name() for dependency in component.dependencies } - # Only keep sources - build_src_files = [os.path.relpath(p, component.path) for p in build_src_files] + # Only keep sources. Reference them absolutely when they live outside the + # component dir (a local library), relative otherwise. + if external: + build_src_files = [str(Path(p).resolve()) for p in build_src_files] + else: + build_src_files = [os.path.relpath(p, component.path) for p in build_src_files] build_src_files = [ f for f in build_src_files if Path(f).suffix in SRC_FILE_EXTENSIONS ] @@ -166,13 +177,24 @@ def generate_cmakelists_txt(component: IDFComponent) -> str: link_libraries, build_flags = split_list_by_condition( build_flags, lambda a: a[2:].strip() if a.startswith("-l") else None ) + # A local library's relative -L paths are relative to its own directory; + # resolve them against it so they still work from the component cache dir. + # (read_path / d yields d unchanged when d is already absolute.) + if external: + link_directories = [ + str((read_path / Path(d)).resolve()) for d in link_directories + ] # Split include directories from build_flags # Only keep an include directory if it exists build_include_dirs = [build_include_dir, build_src_dir] + include_dir_flags build_include_dirs = [ - d for d in build_include_dirs if (component.path / Path(d)).is_dir() + d for d in build_include_dirs if (read_path / Path(d)).is_dir() ] + if external: + build_include_dirs = [ + str((read_path / Path(d)).resolve()) for d in build_include_dirs + ] # Split build_flags list into private and public lists private_build_flags, public_build_flags = split_list_by_condition( diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 1a523ce0ab..ee0a758a31 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -21,14 +21,15 @@ import itertools import json import logging import os -from pathlib import Path +from pathlib import Path, PurePosixPath import re import tempfile from typing import Any from urllib.parse import urlsplit, urlunsplit +from urllib.request import url2pathname from esphome import git -from esphome.core import CORE, Library +from esphome.core import CORE, EsphomeError, Library from esphome.framework_helpers import archive_extract_all, download_from_mirrors, rmdir _LOGGER = logging.getLogger(__name__) @@ -73,6 +74,14 @@ class Source: ) -> Path: raise NotImplementedError + def source_root(self, build_path: Path) -> Path: + """Directory holding the library's own files (manifest + sources). + + Defaults to the downloaded build directory; a source that references its + files in place (:class:`LocalSource`) overrides this to point elsewhere. + """ + return build_path + class URLSource(Source): def __init__(self, url: str): @@ -143,6 +152,53 @@ class GitSource(Source): return f"{self.url}#{self.ref}" if self.ref else self.url +class LocalSource(Source): + """A library that already exists as a directory on the local filesystem. + + Referenced with a ``file://`` URL (PlatformIO's spelling for a local library + folder). Nothing is copied: the backend generates its build files into an + otherwise empty cache directory and references the library's own sources in + place by absolute path (via :meth:`source_root`). So the user's source tree + stays untouched and edits are picked up on the next build without syncing. + """ + + def __init__(self, path: str): + self.local_path = path + + def download( + self, dir_suffix: str, force: bool = False, salt: str = "", namespace: str = "" + ) -> Path: + src = Path(self.local_path) + if not src.is_dir(): + # EsphomeError (not InvalidLibrary) so the CLI prints a clean message + # instead of a traceback -- pointing a file:// at a missing folder is + # the most common first mistake with a local library. + raise EsphomeError( + f"Local library directory does not exist: {self.local_path}" + ) + base_dir = Path(CORE.data_dir) / DOMAIN + if namespace: + base_dir = base_dir / namespace + h = hashlib.new("sha256") + h.update(str(src.resolve()).encode()) + if salt: + h.update(salt.encode()) + # Only the generated build files live here; the library's own sources + # are referenced in place from source_root(). + path = base_dir / h.hexdigest()[:8] / dir_suffix + path.mkdir(parents=True, exist_ok=True) + return path + + def source_root(self, build_path: Path) -> Path: + return Path(self.local_path) + + def __str__(self): + path = Path(self.local_path) + # as_uri() needs an absolute path; _node_key rejects relative file:// + # URLs, but guard anyway so a diagnostic can't itself raise. + return path.as_uri() if path.is_absolute() else f"file://{self.local_path}" + + class InvalidLibrary(Exception): pass @@ -162,6 +218,9 @@ class ConvertedLibrary: self.data = {} self.dependencies: list[ConvertedLibrary] = [] self._path: Path | None = None + # Where the library's own files live (manifest + sources). Set by + # download(); equals path for registry/git, the user's dir for local. + self.source_path: Path | None = None def __str__(self): return f"{self.name}@{self.version}={self.source}" @@ -176,6 +235,16 @@ class ConvertedLibrary: def path(self, value: Path) -> None: self._path = value + @property + def source_dir(self) -> Path: + """Directory the library's own files (manifest + sources) are read from. + + The build dir for a registry/git source; the user's directory for a + local library. Backends read sources from here and emit their build + files into ``path``. + """ + return self.source_path or self.path + def get_sanitized_name(self): return re.sub(r"[^a-zA-Z0-9_.\-/]", "_", self.name) @@ -193,6 +262,7 @@ class ConvertedLibrary: self.path = self.source.download( self.get_sanitized_name(), force=force, salt=salt, namespace=namespace ) + self.source_path = self.source.source_root(self.path) @dataclass @@ -515,11 +585,14 @@ class _LibNode: key: str is_git: bool + is_local: bool = False + is_registry: bool = False owner: str | None = None pkgname: str | None = None requirements: set[str] = field(default_factory=set) url: str | None = None ref: str | None = None + local_path: str | None = None edges: set[str] = field(default_factory=set) @@ -536,40 +609,83 @@ def _url_or_none(value: Any) -> str | None: def _node_key( name: str | None, version: str | None, repository: str | None -) -> tuple[str, bool, tuple[str | None, str | None]]: - """Return ``(key, is_git, locator)`` for a library or dependency spec. +) -> tuple[str, str, tuple[str | None, str | None]]: + """Return ``(key, kind, locator)`` for a library or dependency spec. - The key is derived from the *input* spec (the registry name as written, or - the git URL path), not the resolved canonical name. So a package referenced - inconsistently -- bare ``name`` vs ``owner/name``, or git vs registry -- maps - to distinct keys and isn't deduplicated; ``convert_libraries`` warns about - that after resolution rather than merging the nodes. + ``kind`` is one of: - PlatformIO's Library Manager also accepted a git URL in the *name* - position (``add_library("https://github.com/x/y", None)``), including the - ``git+`` VCS prefix and the ``CustomName=URL`` form; recognize those here - so such specs resolve as git sources instead of failing a registry lookup. + - ``"registry"`` -- ``locator`` is ``(owner, pkgname)``. + - ``"git"`` -- ``locator`` is ``(url, ref)``. + - ``"local"`` -- a ``file://`` directory; ``locator`` is ``(path, None)``. + + The key is derived from the *input* spec (the registry name as written, the + git URL path, or the custom name / directory name for a local folder), not + the resolved canonical name. So a package referenced inconsistently -- bare + ``name`` vs ``owner/name``, or git vs registry -- maps to distinct keys and + isn't deduplicated; ``convert_libraries`` warns about that after resolution + rather than merging the nodes. + + PlatformIO's Library Manager also accepted a URL in the *name* position + (``add_library("https://github.com/x/y", None)``), including the ``git+`` + VCS prefix and the ``CustomName=URL`` form; recognize those here so such + specs resolve as git (or local) sources instead of failing a registry + lookup. A plain ``file://`` URL is PlatformIO's spelling for a local library + folder, so it resolves as a local directory; ``git+file://`` stays a git + source. """ if not repository and name and "://" in name: - # Try the whole name first so a bare URL whose query contains ``=`` - # stays intact; fall back to the ``CustomName=URL`` form, where the - # key derives from the URL path and the custom name is irrelevant. - repository = _url_or_none(name) or _url_or_none(name.split("=", 1)[-1]) - if repository is None: + # Split a ``CustomName=URL`` name, but only when the whole string isn't + # itself a valid URL (a bare URL whose query contains ``=`` must stay + # intact). + custom_name, candidate = None, name + if "=" in name and _url_or_none(name) is None: + custom_name, candidate = name.split("=", 1) + try: + scheme = urlsplit(candidate).scheme + except ValueError: + scheme = "" + if scheme == "file" or _url_or_none(candidate): + name, repository = custom_name, candidate + else: # Anything with ``://`` was meant to be a URL; failing it fast # beats a confusing registry "package not found" error. raise RuntimeError(f"Invalid PIO library URL: {name}") if repository: + is_git_prefixed = repository.startswith("git+") split_result = urlsplit(repository.removeprefix("git+")) + if split_result.scheme == "file" and not is_git_prefixed: + # A plain file:// URL points at a local library directory. A local + # file URL is written file:///absolute/path (empty host) or, less + # commonly, file://localhost/path. Anything else -- a real host, or + # a relative path whose first segment parses as the host -- is + # rejected rather than silently resolved to the wrong directory. + if split_result.netloc not in ("", "localhost"): + raise RuntimeError( + f"Unsupported host in file:// library URL '{repository}'; " + "use an absolute path, e.g. file:///path/to/lib" + ) + # Validate the URL path itself (always POSIX-style, leading slash), + # not the OS path: on Windows a "/foo" path is not is_absolute() + # without a drive, which would wrongly reject a valid file:/// URL. + # Reject a relative path (``file:lib_dev``) or a bare root + # (``file:///``, which has no final segment). + url_path = split_result.path + if not url_path.startswith("/") or not PurePosixPath(url_path).name: + raise RuntimeError( + f"file:// library URL '{repository}' must be an absolute " + "directory path, e.g. file:///path/to/lib" + ) + path = url2pathname(url_path) + return (name or PurePosixPath(url_path).name), "local", (path, None) key = str(split_result.path).strip("/").removesuffix(".git") ref = split_result.fragment.strip() or None url = urlunsplit(split_result._replace(fragment="")) - return key, True, (url, ref) + return key, "git", (url, ref) if name and "/" in name: owner, pkgname = name.split("/", 1) else: owner, pkgname = None, name - return name, False, (owner, pkgname) + return name, "registry", (owner, pkgname) def convert_libraries( @@ -618,13 +734,45 @@ def convert_libraries( return name.split("/")[-1].lower() in lib_ignore def add_spec(name: str | None, version: str | None, repository: str | None) -> str: - key, is_git, locator = _node_key(name, version, repository) - node = nodes.get(key) or _LibNode(key=key, is_git=is_git) + key, kind, locator = _node_key(name, version, repository) + node = nodes.get(key) or _LibNode(key=key, is_git=kind == "git") nodes[key] = node - if is_git: + # The same key requested from two different kinds of source (or two + # different local paths) is a config mistake: one silently wins. Warn so + # it isn't a surprise. (git-vs-registry is reported separately below.) + if kind == "git": + if node.is_local: + _LOGGER.warning( + "Library %s is requested as both a local directory and a git " + "source; using the git source.", + key, + ) node.is_git = True node.url, node.ref = locator + elif kind == "local": + new_path = locator[0] + if node.is_git: + # git wins (checked first when building the source); leave the + # node as a git source. + _LOGGER.warning( + "Library %s is requested as both a local directory and a git " + "source; using the git source.", + key, + ) + else: + if node.is_local and node.local_path != new_path: + _LOGGER.warning( + "Library %s is requested from two local directories (%s " + "and %s); using %s.", + key, + node.local_path, + new_path, + new_path, + ) + node.is_local = True + node.local_path = new_path else: + node.is_registry = True node.owner, node.pkgname = locator if version: node.requirements.add(version) @@ -658,6 +806,8 @@ def convert_libraries( if node.is_git: component = ConvertedLibrary(key, "*", GitSource(node.url, node.ref)) + elif node.is_local: + component = ConvertedLibrary(key, "*", LocalSource(node.local_path)) else: owner, name, version, url = _resolve_registry_version( node.owner, node.pkgname, node.requirements @@ -667,20 +817,22 @@ def convert_libraries( ) component.download(salt=salt, namespace=backend.cache_key) - library_json_path = component.path / "library.json" - library_properties_path = component.path / "library.properties" + source_dir = component.source_dir + library_json_path = source_dir / "library.json" + library_properties_path = source_dir / "library.properties" has_json = library_json_path.is_file() has_properties = library_properties_path.is_file() - if not has_json and not has_properties: + if not has_json and not has_properties and not node.is_local: # The shared cache can hold a broken copy (e.g. a clone or an # extraction interrupted by a killed process). Force one # re-download so a bad cache entry self-heals instead of failing - # every build until the user runs a full clean. + # every build until the user runs a full clean. A local source is + # read in place, so there is nothing to re-download. _LOGGER.warning( "Library %s at %s is missing library.json and library.properties; " "re-downloading", key, - component.path, + source_dir, ) component.download(force=True, salt=salt, namespace=backend.cache_key) has_json = library_json_path.is_file() @@ -690,9 +842,14 @@ def convert_libraries( elif has_properties: component.data = _parse_library_properties(library_properties_path) else: - raise RuntimeError( + # For a local library a missing manifest is user input, so raise + # EsphomeError (clean CLI message) like the missing-directory case; + # for registry/git a missing manifest means a corrupt cache, which + # is not user error, so keep RuntimeError. + error_cls = EsphomeError if node.is_local else RuntimeError + raise error_cls( f"Invalid PIO library {key}: missing library.json and " - f"library.properties in {component.path}" + f"library.properties in {source_dir}" ) try: @@ -735,17 +892,26 @@ def convert_libraries( node.edges.add(dep_key) worklist.append(dep_key) - # A git source wins over any registry version requested for the same - # component. That's intentional, but warn so a dropped registry pin isn't a - # silent surprise. + # A git or local source wins over the same component requested from the + # registry. That's intentional, but warn so the dropped registry spec isn't + # a silent surprise -- including when it carried no version pin (a bare + # cg.add_library("Foo"), which is how most components add libraries). for node in nodes.values(): - if node.is_git and node.requirements: + if (node.is_git or node.is_local) and (node.is_registry or node.requirements): + source = "git" if node.is_git else "local" + registry = ( + f"registry version(s) {sorted(node.requirements)}" + if node.requirements + else "a registry package" + ) _LOGGER.warning( - "Library %s is requested both from a git source (%s) and as " - "registry version(s) %s; using the git source.", + "Library %s is requested both from a %s source (%s) and as %s; " + "using the %s source.", node.key, - node.url, - sorted(node.requirements), + source, + node.url if node.is_git else node.local_path, + registry, + source, ) # Two graph nodes that resolve to the same component name (e.g. a package diff --git a/tests/unit_tests/core/test_config.py b/tests/unit_tests/core/test_config.py index 0362c40bce..e09edd7f26 100644 --- a/tests/unit_tests/core/test_config.py +++ b/tests/unit_tests/core/test_config.py @@ -1242,6 +1242,15 @@ def test_make_app_name_cpp_special_chars_escaped() -> None: None, "https://github.com/esphome/noise-c.git", ), + # A local file:// source is routed to the repository, not a registry name + # -- including the fewer-than-two-slashes spelling. + ( + "TeslaBLE=file:///config/esphome/lib_dev", + "TeslaBLE", + None, + "file:///config/esphome/lib_dev", + ), + ("MyLib=file:lib_dev", "MyLib", None, "file:lib_dev"), ], ) def test_add_library_str( diff --git a/tests/unit_tests/test_espidf_component.py b/tests/unit_tests/test_espidf_component.py index 879d98c0a7..f9e048f6f4 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -155,6 +155,62 @@ def test_generate_cmakelists_txt_basic(tmp_component): assert "main.c" in content +def test_generate_cmakelists_txt_external_source_uses_absolute_paths( + tmp_component, tmp_path +): + # A local library's sources live outside the component dir (source_path), + # so SRCS and INCLUDE_DIRS must be emitted as absolute paths into it. + source = tmp_path / "user_lib" + (source / "src").mkdir(parents=True) + (source / "include").mkdir() + (source / "src" / "thing.cpp").write_text("int t;") + tmp_component.source_path = source + tmp_component.data = {} + + content = generate_cmakelists_txt(tmp_component) + + abs_src = str((source / "src" / "thing.cpp").resolve()).replace("\\", "/") + abs_inc = str((source / "include").resolve()).replace("\\", "/") + assert abs_src in content + assert abs_inc in content + # Nothing was copied into the component dir. + assert not (tmp_component.path / "src").exists() + + +def test_generate_cmakelists_txt_external_source_absolutises_link_dirs( + tmp_component, tmp_path +): + # A local library's relative -L path must be made absolute against its own + # directory so it resolves from the component cache dir. + source = tmp_path / "user_lib" + (source / "src").mkdir(parents=True) + (source / "src" / "thing.cpp").write_text("int t;") + (source / "libs").mkdir() + tmp_component.source_path = source + tmp_component.data = {"build": {"flags": ["-Llibs"]}} + + content = generate_cmakelists_txt(tmp_component) + + abs_lib = str((source / "libs").resolve()).replace("\\", "/") + assert "target_link_directories" in content + assert abs_lib in content + + +def test_generate_cmakelists_txt_external_source_root_srcdir(tmp_component, tmp_path): + # An external source with files at its root (no src/ or include/ dir): + # the src-dir search falls through to "." and the missing include dirs are + # filtered out. + source = tmp_path / "flat_lib" + source.mkdir() + (source / "thing.cpp").write_text("int t;") + tmp_component.source_path = source + tmp_component.data = {} + + content = generate_cmakelists_txt(tmp_component) + + assert str((source / "thing.cpp").resolve()).replace("\\", "/") in content + + def test_generate_cmakelists_txt_with_flags(tmp_component, tmp_path): src_dir = tmp_component.path / "src" src_dir.mkdir() @@ -462,70 +518,66 @@ empty= def test_node_key_git_with_ref(): - key, is_git, locator = _node_key( + key, kind, locator = _node_key( "name", None, "https://github.com/foo/bar.git#v1.2.3" ) assert key == "foo/bar" - assert is_git is True + assert kind == "git" assert locator == ("https://github.com/foo/bar.git", "v1.2.3") def test_node_key_git_branch_ref(): - key, is_git, locator = _node_key( + key, kind, locator = _node_key( "name", None, "https://github.com/foo/bar.git#some-branch" ) - assert (key, is_git, locator[1]) == ("foo/bar", True, "some-branch") + assert (key, kind, locator[1]) == ("foo/bar", "git", "some-branch") def test_node_key_git_no_ref(): - _key, is_git, locator = _node_key("name", None, "https://github.com/foo/bar.git") - assert is_git is True + _key, kind, locator = _node_key("name", None, "https://github.com/foo/bar.git") + assert kind == "git" assert locator == ("https://github.com/foo/bar.git", None) def test_node_key_url_in_name_is_git(): # add_library("https://github.com/x/y", None): PlatformIO accepted a bare # git URL as the library name, so the converter must too. - key, is_git, locator = _node_key( - "https://github.com/pstolarz/OneWireNg", None, None - ) + key, kind, locator = _node_key("https://github.com/pstolarz/OneWireNg", None, None) assert key == "pstolarz/OneWireNg" - assert is_git is True + assert kind == "git" assert locator == ("https://github.com/pstolarz/OneWireNg", None) def test_node_key_url_in_name_with_ref(): - key, is_git, locator = _node_key( - "https://github.com/foo/bar.git#v1.2.3", None, None - ) - assert (key, is_git, locator) == ( + key, kind, locator = _node_key("https://github.com/foo/bar.git#v1.2.3", None, None) + assert (key, kind, locator) == ( "foo/bar", - True, + "git", ("https://github.com/foo/bar.git", "v1.2.3"), ) def test_node_key_url_in_name_git_plus_prefix(): - key, is_git, locator = _node_key("git+https://github.com/foo/bar", None, None) - assert (key, is_git, locator) == ( + key, kind, locator = _node_key("git+https://github.com/foo/bar", None, None) + assert (key, kind, locator) == ( "foo/bar", - True, + "git", ("https://github.com/foo/bar", None), ) def test_node_key_git_plus_prefix_in_repository(): - _key, is_git, locator = _node_key("name", None, "git+https://github.com/foo/bar") - assert (is_git, locator) == (True, ("https://github.com/foo/bar", None)) + _key, kind, locator = _node_key("name", None, "git+https://github.com/foo/bar") + assert (kind, locator) == ("git", ("https://github.com/foo/bar", None)) def test_node_key_custom_name_equals_url_is_git(): - key, is_git, locator = _node_key( + key, kind, locator = _node_key( "OneWireNg=https://github.com/pstolarz/OneWireNg", None, None ) - assert (key, is_git, locator) == ( + assert (key, kind, locator) == ( "pstolarz/OneWireNg", - True, + "git", ("https://github.com/pstolarz/OneWireNg", None), ) @@ -533,14 +585,70 @@ def test_node_key_custom_name_equals_url_is_git(): def test_node_key_url_in_name_with_query_containing_equals(): # A bare URL whose query string contains ``=`` must not be split by the # CustomName=URL handling. - key, is_git, locator = _node_key("https://host/x/y.git?ref=main", None, None) - assert (key, is_git, locator) == ( + key, kind, locator = _node_key("https://host/x/y.git?ref=main", None, None) + assert (key, kind, locator) == ( "x/y", - True, + "git", ("https://host/x/y.git?ref=main", None), ) +def test_node_key_file_url_in_repository_is_local(): + # A plain file:// entry (PlatformIO's spelling for a local library folder) + # resolves as a local directory, keeping the custom name as the key. The + # path is the OS-native form of the URL (backslashes on Windows). + key, kind, (path, ref) = _node_key( + "TeslaBLE", None, "file:///config/esphome/lib_dev" + ) + assert (key, kind, ref) == ("TeslaBLE", "local", None) + assert Path(path) == Path("/config/esphome/lib_dev") + + +def test_node_key_bare_file_url_is_local_named_for_dir(): + # Without a custom name the directory's own name becomes the key. + key, kind, (path, ref) = _node_key(None, None, "file:///opt/mylib") + assert (key, kind, ref) == ("mylib", "local", None) + assert Path(path) == Path("/opt/mylib") + + +def test_node_key_custom_name_equals_file_url_is_local(): + key, kind, (path, ref) = _node_key("Foo=file:///opt/mylib", None, None) + assert (key, kind, ref) == ("Foo", "local", None) + assert Path(path) == Path("/opt/mylib") + + +def test_node_key_file_url_localhost_host_is_local(): + # A localhost host is ignored; only the path identifies the directory. + key, kind, (path, ref) = _node_key(None, None, "file://localhost/opt/mylib") + assert (key, kind, ref) == ("mylib", "local", None) + assert Path(path) == Path("/opt/mylib") + + +@pytest.mark.parametrize( + "url", ["file://server/share/lib", "file://lib_dev", "file://../mylib"] +) +def test_node_key_file_url_with_host_rejected(url: str) -> None: + # A real host, or a relative path whose first segment parses as the host, + # is rejected rather than silently resolved to the wrong directory. + with pytest.raises(RuntimeError, match="Unsupported host in file://"): + _node_key(None, None, url) + + +@pytest.mark.parametrize("url", ["file:lib_dev", "file:./lib", "file:///"]) +def test_node_key_file_url_must_be_absolute(url: str) -> None: + # A relative path (no host, e.g. file:lib_dev) or a bare root (file:///) + # is rejected rather than resolved against the cwd or yielding an empty name. + with pytest.raises(RuntimeError, match="must be an absolute"): + _node_key(None, None, url) + + +def test_node_key_git_plus_file_url_stays_git(): + # git+file:// is an explicit local git repo, not a plain directory. + _key, kind, locator = _node_key("X", None, "git+file:///srv/foo.git") + assert kind == "git" + assert locator == ("file:///srv/foo.git", None) + + @pytest.mark.parametrize("name", ["http://[::1", "CustomName=http://[::1"]) def test_node_key_malformed_url_in_name_raises(name: str) -> None: # A name that was clearly meant to be a URL but does not parse must fail @@ -550,25 +658,25 @@ def test_node_key_malformed_url_in_name_raises(name: str) -> None: def test_node_key_name_with_equals_but_no_url_is_registry(): - key, is_git, locator = _node_key("FOO=BAR", "1.0", None) - assert (key, is_git, locator) == ("FOO=BAR", False, (None, "FOO=BAR")) + key, kind, locator = _node_key("FOO=BAR", "1.0", None) + assert (key, kind, locator) == ("FOO=BAR", "registry", (None, "FOO=BAR")) def test_node_key_version_url_still_ignored_when_name_plain(): # A version that is a URL is handled by the dependency walk, not here; # a plain name must stay a registry spec regardless of version shape. - key, is_git, _locator = _node_key("bar", "https://github.com/foo/bar", None) - assert (key, is_git) == ("bar", False) + key, kind, _locator = _node_key("bar", "https://github.com/foo/bar", None) + assert (key, kind) == ("bar", "registry") def test_node_key_registry_owner_name(): - key, is_git, locator = _node_key("foo/bar", "^1.0.0", None) - assert (key, is_git, locator) == ("foo/bar", False, ("foo", "bar")) + key, kind, locator = _node_key("foo/bar", "^1.0.0", None) + assert (key, kind, locator) == ("foo/bar", "registry", ("foo", "bar")) def test_node_key_registry_bare_name(): - key, is_git, locator = _node_key("bar", "1.0", None) - assert (key, is_git, locator) == ("bar", False, (None, "bar")) + key, kind, locator = _node_key("bar", "1.0", None) + assert (key, kind, locator) == ("bar", "registry", (None, "bar")) def test_normalize_dependencies_none(): diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index c0a0c678db..0eede78656 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -10,13 +10,14 @@ from pathlib import Path import pytest -from esphome.core import Library +from esphome.core import EsphomeError, Library import esphome.platformio.library as lib from esphome.platformio.library import ( ConvertedLibrary, GitSource, InvalidLibrary, LibraryBackend, + LocalSource, Source, URLSource, _resolve_registry_version, @@ -87,6 +88,68 @@ def test_gitsource_str_includes_ref_when_present(): assert str(GitSource("http://git/repo.git", None)) == "http://git/repo.git" +def test_source_root_defaults_to_build_dir() -> None: + # Registry/git sources are read from where they were downloaded. + build = Path("/some/build/dir") + assert URLSource("http://x/y.tar.gz").source_root(build) == build + assert GitSource("http://x/y.git", None).source_root(build) == build + + +def test_converted_library_source_dir_defaults_to_path() -> None: + c = ConvertedLibrary("x", "1.0", source=None) + c.path = Path("/build") + assert c.source_dir == Path("/build") # no source_path set -> build dir + c.source_path = Path("/user/lib") + assert c.source_dir == Path("/user/lib") + + +def test_convert_libraries_local_missing_manifest_is_esphome_error( + setup_core: Path, +) -> None: + # A local directory that has no library.json/library.properties is user + # input, so it must surface as a clean EsphomeError (named at the user's dir). + src = setup_core / "not_a_lib" + src.mkdir() # exists, but no manifest + # match= is a regex; a Windows path has backslashes, so match a literal + # fragment and check the directory is named separately. + with pytest.raises(EsphomeError, match="missing library.json") as excinfo: + convert_libraries([Library("Foo", None, src.as_uri())], _backend()) + assert str(src) in str(excinfo.value) + + +def test_localsource_download_missing_dir_raises(tmp_path: Path) -> None: + # EsphomeError so the CLI prints it cleanly instead of a traceback. + with pytest.raises(EsphomeError, match="does not exist"): + LocalSource(str(tmp_path / "nope")).download("mylib") + + +def test_localsource_str() -> None: + assert str(LocalSource("/tmp/lib")) == "file:///tmp/lib" + # A relative path can't form a file:// URI; fall back rather than raise. + assert str(LocalSource("rel/lib")) == "file://rel/lib" + + +def test_localsource_download_returns_empty_build_dir(setup_core: Path) -> None: + # Nothing is copied: download() returns an empty build dir (for generated + # files), and source_root() points back at the user's directory. + src = setup_core / "lib_dev" + (src / "src").mkdir(parents=True) + (src / "library.json").write_text("{}") + (src / "src" / "a.cpp").write_text("int a;") + + source = LocalSource(str(src)) + out = source.download("mylib", salt="s", namespace="ns") + + assert out.is_dir() + assert list(out.iterdir()) == [] # no sources copied in + assert out != src + assert source.source_root(out) == src + + # salt/namespace change the cache path. + plain = LocalSource(str(src)).download("mylib") + assert plain != out + + def test_urlsource_download_extracts_then_reuses_marker(setup_core, monkeypatch): monkeypatch.setattr(lib, "rmdir", lambda path, msg="": None) dl_calls: list[list[str]] = [] @@ -317,6 +380,140 @@ def test_convert_libraries_url_in_name_resolves_as_git( assert source.ref is None +def test_convert_libraries_file_url_resolves_as_local( + setup_core: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # A "Name=file://" library points at an on-disk folder: it resolves as a + # local source read in place (no copy), and the registry is never consulted. + src = setup_core / "lib_dev" + (src / "src").mkdir(parents=True) + (src / "library.json").write_text(json.dumps({"name": "TeslaBLE"})) + (src / "src" / "tesla.cpp").write_text("int foo() { return 1; }") + + def fail_registry(owner: str, pkgname: str, requirements: set[str]) -> None: + raise AssertionError(f"registry consulted for {owner}/{pkgname}") + + monkeypatch.setattr(lib, "_resolve_registry_version", fail_registry) + + # as_uri() produces a valid file:// URL on every platform (file:///tmp/... on + # POSIX, file:///C:/... on Windows). + top = convert_libraries([Library("TeslaBLE", None, src.as_uri())], _backend()) + + assert [c.name for c in top] == ["TeslaBLE"] + assert top[0].data["name"] == "TeslaBLE" + assert isinstance(top[0].source, LocalSource) + # Sources are read in place from the user's dir; the build dir stays separate + # and holds no copied sources. + assert top[0].source_path == src + assert top[0].path != src + assert not (top[0].path / "src").exists() + + +def test_convert_libraries_local_overrides_registry_version( + setup_core: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + # The same library requested both from the registry (with a version) and as + # a local directory resolves to the local source, with a warning that the + # registry version was dropped. + src = setup_core / "lib_dev" + (src / "src").mkdir(parents=True) + (src / "library.json").write_text(json.dumps({"name": "TeslaBLE"})) + + def fail_registry(owner: str, pkgname: str, requirements: set[str]) -> None: + raise AssertionError(f"registry consulted for {owner}/{pkgname}") + + monkeypatch.setattr(lib, "_resolve_registry_version", fail_registry) + + with caplog.at_level(logging.WARNING, logger="esphome.platformio.library"): + top = convert_libraries( + [ + Library("TeslaBLE", "1.0.0", None), + Library("TeslaBLE", None, src.as_uri()), + ], + _backend(), + ) + + assert isinstance(top[0].source, LocalSource) + assert "local source" in caplog.text + + +def test_convert_libraries_versionless_registry_and_local_warns( + setup_core: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + # A bare cg.add_library("Foo") (versionless registry, the common case) that + # collides with a local directory of the same key must still warn -- the + # registry spec is dropped and the local folder silently takes over. + src = setup_core / "foo" + src.mkdir() + (src / "library.json").write_text(json.dumps({"name": "Foo"})) + + def fail_registry(owner: str, pkgname: str, requirements: set[str]) -> None: + raise AssertionError(f"registry consulted for {owner}/{pkgname}") + + monkeypatch.setattr(lib, "_resolve_registry_version", fail_registry) + + with caplog.at_level(logging.WARNING, logger="esphome.platformio.library"): + top = convert_libraries( + [Library("Foo", None, None), Library("Foo", None, src.as_uri())], + _backend(), + ) + + assert isinstance(top[0].source, LocalSource) + assert "a registry package" in caplog.text + + +def test_convert_libraries_two_local_dirs_warns( + setup_core: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + # The same key pointed at two local directories warns and uses the last one. + dir_a = setup_core / "a" + dir_b = setup_core / "b" + for d in (dir_a, dir_b): + d.mkdir() + (d / "library.json").write_text(json.dumps({"name": "Foo"})) + + with caplog.at_level(logging.WARNING, logger="esphome.platformio.library"): + top = convert_libraries( + [ + Library("Foo", None, dir_a.as_uri()), + Library("Foo", None, dir_b.as_uri()), + ], + _backend(), + ) + + assert isinstance(top[0].source, LocalSource) + assert top[0].source_path == dir_b # the last one wins + assert "two local directories" in caplog.text + + +@pytest.mark.parametrize("local_first", [True, False]) +def test_convert_libraries_git_and_local_same_key_warns( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, + local_first: bool, +) -> None: + # A key requested as both a git source and a local directory warns and uses + # git, whichever order they appear in. The git URL basename matches the local + # custom name so both map to the key "Foo". + _patch_download_with_manifests(monkeypatch, tmp_path, {"Foo": {"name": "Foo"}}) + git = Library("X", None, "https://host/Foo") + local = Library("Foo", None, "file:///abs/foo") + libs = [local, git] if local_first else [git, local] + + with caplog.at_level(logging.WARNING, logger="esphome.platformio.library"): + top = convert_libraries(libs, _backend()) + + assert isinstance(top[0].source, GitSource) + assert "using the git source" in caplog.text + + def test_convert_libraries_skips_incompatible_dependency(tmp_path, monkeypatch): # A dependency that declares an incompatible platform is skipped (the # top-level library still builds). diff --git a/tests/unit_tests/test_zephyr_library.py b/tests/unit_tests/test_zephyr_library.py index 0ba3577fa7..b370fe0c47 100644 --- a/tests/unit_tests/test_zephyr_library.py +++ b/tests/unit_tests/test_zephyr_library.py @@ -59,7 +59,10 @@ def test_generate_cmakelists_txt_flags_and_includes(tmp_path): assert "-DFOO" in out assert "-Wall" in out assert "zephyr_link_libraries(" in out - assert "-Llibdir" in out + # -L paths are absolutised against the library dir (the CMakeLists lives in a + # zephyr/ subdir, so a relative path would resolve from the wrong place). + abs_libdir = str((tmp_path / "libdir").resolve()).replace("\\", "\\\\") + assert f"-L{abs_libdir}" in out assert "-lm" in out