From 76655cf95122b3f6926769818255b20049a903d2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 19 Jul 2026 18:32:20 -1000 Subject: [PATCH] [platformio] Accept git URLs passed as the library name (#17697) --- esphome/platformio/library.py | 39 ++++++++--- tests/unit_tests/test_espidf_component.py | 78 +++++++++++++++++++++ tests/unit_tests/test_platformio_library.py | 42 +++++++++++ 3 files changed, 150 insertions(+), 9 deletions(-) diff --git a/esphome/platformio/library.py b/esphome/platformio/library.py index 0ffac65e0dc..72a50b795bc 100644 --- a/esphome/platformio/library.py +++ b/esphome/platformio/library.py @@ -25,7 +25,7 @@ from pathlib import Path import re import tempfile from typing import Any -from urllib.parse import urlparse, urlsplit, urlunsplit +from urllib.parse import urlsplit, urlunsplit from esphome import git from esphome.core import CORE, Library @@ -523,6 +523,17 @@ class _LibNode: edges: set[str] = field(default_factory=set) +def _url_or_none(value: Any) -> str | None: + """Return ``value`` if it parses as a URL (scheme and host), else None.""" + if not value or not isinstance(value, str): + return None + try: + parsed = urlsplit(value) + except ValueError: + return None + return value if parsed.scheme and parsed.netloc else None + + def _node_key( name: str | None, version: str | None, repository: str | None ) -> tuple[str, bool, tuple[str | None, str | None]]: @@ -533,9 +544,23 @@ def _node_key( 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 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. """ + 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: + # 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: - split_result = urlsplit(repository) + split_result = urlsplit(repository.removeprefix("git+")) key = str(split_result.path).strip("/").removesuffix(".git") ref = split_result.fragment.strip() or None url = urlunsplit(split_result._replace(fragment="")) @@ -687,13 +712,9 @@ def convert_libraries( continue # The version field may actually be a URL (git/archive dependency). dep_version = dependency["version"] - dep_url = None - try: - parsed = urlparse(dep_version) - if all([parsed.scheme, parsed.netloc]): - dep_url, dep_version = dep_version, None - except (TypeError, ValueError): - pass + 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_espidf_component.py b/tests/unit_tests/test_espidf_component.py index 89d5ce3cf2f..f9ed44b8d2a 100644 --- a/tests/unit_tests/test_espidf_component.py +++ b/tests/unit_tests/test_espidf_component.py @@ -456,6 +456,84 @@ def test_node_key_git_no_ref(): 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 + ) + assert key == "pstolarz/OneWireNg" + assert is_git is True + 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) == ( + "foo/bar", + True, + ("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) == ( + "foo/bar", + True, + ("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)) + + +def test_node_key_custom_name_equals_url_is_git(): + key, is_git, locator = _node_key( + "OneWireNg=https://github.com/pstolarz/OneWireNg", None, None + ) + assert (key, is_git, locator) == ( + "pstolarz/OneWireNg", + True, + ("https://github.com/pstolarz/OneWireNg", None), + ) + + +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) == ( + "x/y", + True, + ("https://host/x/y.git?ref=main", 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 + # fast instead of degrading to a confusing registry lookup error. + with pytest.raises(RuntimeError, match="Invalid PIO library URL"): + _node_key(name, None, 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")) + + +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) + + 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")) diff --git a/tests/unit_tests/test_platformio_library.py b/tests/unit_tests/test_platformio_library.py index 03360eab37c..6a4c0574699 100644 --- a/tests/unit_tests/test_platformio_library.py +++ b/tests/unit_tests/test_platformio_library.py @@ -212,6 +212,48 @@ def test_convert_libraries_handles_unparsable_dependency_version(tmp_path, monke assert [d.name for d in top[0].dependencies] == ["C"] +@pytest.mark.parametrize( + ("value", "expected"), + [ + (None, None), + ("", None), + ("http://[::1", None), # malformed IPv6 makes urlsplit raise ValueError + ("foo/bar", None), + ("file:///no/host", None), + ("https://github.com/x/y", "https://github.com/x/y"), + ], +) +def test_url_or_none(value: str | None, expected: str | None) -> None: + assert lib._url_or_none(value) == expected + + +def test_convert_libraries_url_in_name_resolves_as_git( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # add_library("https://github.com/x/y", None) puts a git URL in the name + # position; it must resolve as a git source and never hit the registry. + _patch_download_with_manifests( + monkeypatch, tmp_path, {"pstolarz/OneWireNg": {"name": "OneWireNg"}} + ) + + def fail_registry(owner: str, pkgname: str, requirements: set[str]) -> None: + raise AssertionError(f"registry consulted for {owner}/{pkgname}") + + # After the helper so this stub wins over the helper's benign one + monkeypatch.setattr(lib, "_resolve_registry_version", fail_registry) + + top = convert_libraries( + [Library("https://github.com/pstolarz/OneWireNg", None, None)], _backend() + ) + + assert [c.name for c in top] == ["pstolarz/OneWireNg"] + assert top[0].data["name"] == "OneWireNg" + source = top[0].source + assert isinstance(source, GitSource) + assert source.url == "https://github.com/pstolarz/OneWireNg" + assert source.ref is None + + 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).