From db25f0b025df952fed4db838fc18cdcea54c37f6 Mon Sep 17 00:00:00 2001 From: Diego Ferigo Date: Thu, 6 Aug 2026 18:10:54 +0200 Subject: [PATCH] Support source archives in additional recipes Previously an entry of rosdistro_additional_recipes.yaml was always assumed to be a git repository: its package.xml was read from the raw file endpoint of github.com or gitlab.com, and generate_source() emitted a git/rev source. Any other URL was rejected with "Cannot handle unknown repository hoster". A ROS package distributed as a plain source archive, for instance a vendor driver hosted on an Artifactory server, could therefore not be layered on a distro at all. Accept an entry whose url points at a source archive (.zip, .tar.gz and the other common suffixes) and carries a sha256 checksum instead of a rev or a tag. Its package.xml is read out of the downloaded archive, after dropping a single common top-level directory so that additional_folder means the same thing for this lookup and for the generated build script, and the source block becomes url/sha256. A missing checksum is a hard error, since a URL source without one would silently be fetched unverified. Archives can live behind authentication, so the downloads now go through requests, which is already a dependency. It resolves the ambient credentials, proxy and CA settings on its own, and it drops the Authorization header when a redirect crosses to another host, which is what an Artifactory server does when it hands out a presigned storage URL. Vinca therefore keeps no credential handling of its own beyond the existing GITHUB_TOKEN and GITLAB_TOKEN support. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- vinca/distro.py | 165 +++++++++++++++++++- vinca/main.py | 23 +-- vinca/test_archive_sources.py | 282 ++++++++++++++++++++++++++++++++++ 3 files changed, 455 insertions(+), 15 deletions(-) create mode 100644 vinca/test_archive_sources.py diff --git a/vinca/distro.py b/vinca/distro.py index 7dc0091..9ce23fb 100644 --- a/vinca/distro.py +++ b/vinca/distro.py @@ -1,11 +1,98 @@ +import io import os +import posixpath +import tarfile import urllib.parse -import urllib.request +import zipfile +import requests from rosdistro import get_cached_distribution, get_index, get_index_url from rosdistro.dependency_walker import DependencyWalker from rosdistro.manifest_provider import get_release_tag +# Source archive suffixes an additional recipe may point at instead of a git repository. +# Such a package is fetched by URL (checksummed with sha256) rather than cloned. +# Zstandard is left out on purpose: tarfile only reads it from Python 3.14 on, while +# vinca still supports older interpreters, so accepting it here would promise a format +# that fails to open on most of them. +ARCHIVE_SUFFIXES = ( + ".zip", + ".tar", + ".tar.gz", + ".tgz", + ".tar.bz2", + ".tbz2", + ".tar.xz", + ".txz", +) + + +def is_archive_url(url): + """Return True when url points at a source archive rather than a git repository.""" + if not url: + return False + path = urllib.parse.urlparse(url).path.lower() + return path.endswith(ARCHIVE_SUFFIXES) + + +def _normalize_member(name): + """Return an archive member name without its './' prefix and trailing slash.""" + name = name.strip("/") + while name.startswith("./"): + name = name[2:] + return "" if name == "." else name + + +def _strip_common_root(entries): + """Return the single top-level directory shared by entries, or an empty string. + + Entries are (name, is_dir) pairs. Build tools unpack an archive whose contents all + live under one directory by dropping that directory, so member lookups do the same. + """ + names = [(_normalize_member(name), is_dir) for name, is_dir in entries] + roots = {name.split("/", 1)[0] for name, _ in names if name} + if len(roots) != 1: + return "" + root = roots.pop() + # A lone top-level file, rather than a directory, is not a root to strip. + if any(name == root and not is_dir for name, is_dir in names): + return "" + return root + + +def _resolve_member(*, entries, member): + """Return the name entries use for member, resolved against the stripped root. + + Names are compared normalized, so an archive listing its members as './pkg/...' + resolves like one listing them as 'pkg/...'. + """ + wanted = posixpath.join(_strip_common_root(entries), member) + for name, is_dir in entries: + if not is_dir and _normalize_member(name) == wanted: + return name + raise KeyError(member) + + +def _read_archive_member(*, payload, url, member): + """Return the text of member inside the archive payload, stripping its common root.""" + member = _normalize_member(member) + if zipfile.is_zipfile(io.BytesIO(payload)): + with zipfile.ZipFile(io.BytesIO(payload)) as archive: + entries = [(info.filename, info.is_dir()) for info in archive.infolist()] + return archive.read(_resolve_member(entries=entries, member=member)).decode( + "utf-8" + ) + try: + archive = tarfile.open(fileobj=io.BytesIO(payload), mode="r:*") + except tarfile.TarError: + raise RuntimeError(f"Unsupported archive format: {url}") + with archive: + entries = [(m.name, m.isdir()) for m in archive.getmembers()] + extracted = archive.extractfile(_resolve_member(entries=entries, member=member)) + if extracted is None: + raise KeyError(member) + return extracted.read().decode("utf-8") + class Distro(object): def __init__( @@ -41,6 +128,7 @@ def __init__( # simple caches to avoid repeatedly fetching/processing the same data self._additional_xml_cache = {} + self._last_archive = None self._depends_cache = {} os.environ["ROS_VERSION"] = "1" if self.check_ros1() else "2" @@ -125,6 +213,14 @@ def get_released_repo(self, pkg_name): # we also support a 'rev' field, so depending on what is available # we return either the tag or the rev, and the third argument is either 'rev' or 'tag' url = self.snapshot[pkg_name].get("url", None) + # An archive URL is not a git repository: the reference is its sha256 checksum. + if is_archive_url(url): + sha256 = self.snapshot[pkg_name].get("sha256", None) + if not sha256: + raise RuntimeError( + f"The archive source of '{pkg_name}' has no sha256 checksum: {url}" + ) + return url, sha256, "sha256" if "tag" in self.snapshot[pkg_name].keys(): tag_or_rev = self.snapshot[pkg_name].get("tag", None) ref_type = "tag" @@ -202,6 +298,9 @@ def get_package_names(self): def get_package_xml_for_additional_package(self, pkg_info): raw_url_base = pkg_info.get("url") + # An archive has no raw-file endpoint, so read package.xml out of the archive itself. + if is_archive_url(raw_url_base): + return self._package_xml_from_archive_or_cached(pkg_info) if "github.com" in raw_url_base: raw_url = self._construct_raw_url_github(pkg_info) return self._download_raw_pkg_xml_or_cached(url=raw_url) @@ -211,6 +310,12 @@ def get_package_xml_for_additional_package(self, pkg_info): raise RuntimeError(f"Cannot handle unknown repository hoster: {raw_url_base}") def _get_auth_headers(self, url): + """Token header for the hosters vinca knows, empty otherwise. + + Any other credential (a private artifact server such as Artifactory, a proxy, + a custom CA) is resolved by requests from the standard environment, so vinca + does not need to know about it. + """ host = urllib.parse.urlparse(url).netloc for env_var, domains in ( ("GITHUB_TOKEN", ("github.com", "githubusercontent.com")), @@ -221,17 +326,65 @@ def _get_auth_headers(self, url): return {"Authorization": f"token {token}"} return {} + def _get(self, url): + """Fetch url, letting requests apply the ambient credentials and proxy settings. + + requests also drops the Authorization header when a redirect leaves the original + host, which matters because an Artifactory server usually redirects a download + to a presigned URL on a separate storage host. + """ + response = requests.get(url, headers=self._get_auth_headers(url), timeout=60) + response.raise_for_status() + return response + def _download_raw_pkg_xml_or_cached(self, url): if url in self._additional_xml_cache: return self._additional_xml_cache[url] - req = urllib.request.Request(url, headers=self._get_auth_headers(url)) try: - with urllib.request.urlopen(req) as resp: - xml_content = resp.read().decode("utf-8") - self._additional_xml_cache[url] = xml_content - return xml_content + xml_content = self._get(url).text except Exception as e: raise RuntimeError(f"Failed to fetch package.xml from {url}: {e}") + self._additional_xml_cache[url] = xml_content + return xml_content + + def _package_xml_from_archive_or_cached(self, pkg_info): + """Read package.xml out of a source archive referenced by an additional recipe. + + The member is looked up under the archive's additional_folder, after dropping a + single common top-level directory. That mirrors how the build tool unpacks the + archive, so one additional_folder value describes both the recipe and this lookup. + """ + url = pkg_info.get("url") + xml_name = pkg_info.get("package_xml_name", "package.xml") + member = posixpath.join(pkg_info.get("additional_folder", ""), xml_name) + cache_key = f"{url}#{member}" + if cache_key in self._additional_xml_cache: + return self._additional_xml_cache[cache_key] + + payload = self._download_archive_or_cached(url) + try: + xml_content = _read_archive_member(payload=payload, url=url, member=member) + except KeyError: + raise RuntimeError(f"Could not find '{member}' inside the archive {url}") + except Exception as e: + raise RuntimeError(f"Failed to read '{member}' from the archive {url}: {e}") + self._additional_xml_cache[cache_key] = xml_content + return xml_content + + def _download_archive_or_cached(self, url): + """Download the archive at url, keeping only the most recent payload in memory. + + Several packages can share one archive, so reusing the last download avoids + fetching it again without holding every archive of the run in memory. + """ + if self._last_archive and self._last_archive[0] == url: + return self._last_archive[1] + try: + payload = self._get(url).content + except Exception as e: + raise RuntimeError(f"Failed to download the archive {url}: {e}") + self._last_archive = (url, payload) + return payload # Based on https://github.com/ros-infrastructure/rosdistro/blob/fad8d9f647631945847cb18bc1d1f43008d7a282/src/rosdistro/manifest_provider/github.py#L51C1-L69C29 # But with the option to specify the name of the package.xml file in case the repo uses a non-standard name diff --git a/vinca/main.py b/vinca/main.py index 8357850..f247281 100644 --- a/vinca/main.py +++ b/vinca/main.py @@ -707,6 +707,17 @@ def generate_outputs_version(distro, vinca_conf): return outputs +def _source_reference(*, url, ref, ref_type): + """Return the source keys locating url at ref, for a git repository or an archive. + + An archive is identified by a sha256 ref_type and is fetched by URL, so it carries + a checksum instead of a git revision. + """ + if ref_type == "sha256": + return {"url": url, "sha256": ref} + return {"git": url, ref_type: ref} + + def generate_source(distro, vinca_conf): source = {} for pkg_shortname in vinca_conf["_selected_pkgs"]: @@ -717,9 +728,7 @@ def generate_source(distro, vinca_conf): if is_dummy_metapackage(pkg_shortname, vinca_conf): continue url, ref, ref_type = distro.get_released_repo(pkg_shortname) - entry = {} - entry["git"] = url - entry[ref_type] = ref + entry = _source_reference(url=url, ref=ref, ref_type=ref_type) pkg_names = resolve_pkgname(pkg_shortname, vinca_conf, distro) pkg_version = distro.get_version(pkg_shortname) print("Checking ", pkg_shortname, pkg_version) @@ -772,9 +781,7 @@ def generate_source_version(distro, vinca_conf): url, ref, ref_type = distro.get_released_repo(pkg_shortname) - entry = {} - entry["git"] = url - entry[ref_type] = ref + entry = _source_reference(url=url, ref=ref, ref_type=ref_type) pkg_names = resolve_pkgname(pkg_shortname, vinca_conf, distro) version = distro.get_version(pkg_shortname) if vinca_conf.get("trigger_new_versions"): @@ -813,9 +820,7 @@ def generate_fat_source(distro, vinca_conf): continue url, ref, ref_type = distro.get_released_repo(pkg_shortname) - entry = {} - entry["git"] = url - entry[ref_type] = ref + entry = _source_reference(url=url, ref=ref, ref_type=ref_type) pkg_names = resolve_pkgname(pkg_shortname, vinca_conf, distro) if not pkg_names: continue diff --git a/vinca/test_archive_sources.py b/vinca/test_archive_sources.py new file mode 100644 index 0000000..645e39a --- /dev/null +++ b/vinca/test_archive_sources.py @@ -0,0 +1,282 @@ +import io +import tarfile +import zipfile + +import pytest +import requests + +from vinca.distro import ( + Distro, + _read_archive_member, + _strip_common_root, + is_archive_url, +) +from vinca.main import _source_reference + +PACKAGE_XML = "demo_pkg" + + +def _zip_bytes(files): + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w") as archive: + for name, content in files.items(): + if name.endswith("/"): + archive.writestr(zipfile.ZipInfo(name), b"") + else: + archive.writestr(name, content) + return buffer.getvalue() + + +def _tar_bytes(files): + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as archive: + for name, content in files.items(): + if name.endswith("/"): + info = tarfile.TarInfo(name.rstrip("/")) + info.type = tarfile.DIRTYPE + archive.addfile(info) + continue + payload = content.encode() + info = tarfile.TarInfo(name) + info.size = len(payload) + archive.addfile(info, io.BytesIO(payload)) + return buffer.getvalue() + + +@pytest.mark.parametrize( + "url, expected", + [ + ("https://example.com/pkg-1.0.zip", True), + ("https://example.com/pkg-1.0.tar.gz", True), + ("https://example.com/pkg-1.0.tar.bz2", True), + ("https://example.com/pkg.zip?token=abc", True), + ("https://github.com/org/repo.git", False), + ("https://gitlab.com/org/repo", False), + (None, False), + ], +) +def test_is_archive_url(url, expected): + assert is_archive_url(url) is expected + + +def test_strip_common_root(): + assert _strip_common_root([("root/", True), ("root/a.txt", False)]) == "root" + # A tar lists its directories without a trailing slash. + assert _strip_common_root([("root", True), ("root/a.txt", False)]) == "root" + # A tar created from '.' prefixes every member. + assert ( + _strip_common_root([(".", True), ("./root", True), ("./root/a.txt", False)]) + == "root" + ) + # Several top-level entries mean there is nothing to strip. + assert _strip_common_root([("a/x", False), ("b/x", False)]) == "" + # A top-level file is not a root directory. + assert _strip_common_root([("root", False), ("root/x", False)]) == "" + + +@pytest.mark.parametrize("pack", [_zip_bytes, _tar_bytes]) +def test_read_archive_member_strips_the_common_root(pack): + payload = pack( + { + "demo-1.0/": "", + "demo-1.0/src/demo_pkg/package.xml": PACKAGE_XML, + } + ) + content = _read_archive_member( + payload=payload, + url="https://example.com/demo-1.0.zip", + member="src/demo_pkg/package.xml", + ) + assert content == PACKAGE_XML + + +@pytest.mark.parametrize("pack", [_zip_bytes, _tar_bytes]) +def test_read_archive_member_without_a_common_root(pack): + payload = pack({"package.xml": PACKAGE_XML, "CMakeLists.txt": ""}) + content = _read_archive_member( + payload=payload, url="https://example.com/demo-1.0.zip", member="package.xml" + ) + assert content == PACKAGE_XML + + +def test_read_archive_member_of_a_dot_prefixed_tar(): + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as archive: + for name in (".", "./demo-1.0", "./demo-1.0/src"): + info = tarfile.TarInfo(name) + info.type = tarfile.DIRTYPE + archive.addfile(info) + payload = PACKAGE_XML.encode() + info = tarfile.TarInfo("./demo-1.0/src/package.xml") + info.size = len(payload) + archive.addfile(info, io.BytesIO(payload)) + content = _read_archive_member( + payload=buffer.getvalue(), + url="https://example.com/demo-1.0.tar.gz", + member="src/package.xml", + ) + assert content == PACKAGE_XML + + +def test_read_archive_member_missing_member(): + payload = _zip_bytes({"demo-1.0/": "", "demo-1.0/README.md": ""}) + with pytest.raises(KeyError): + _read_archive_member( + payload=payload, + url="https://example.com/demo-1.0.zip", + member="package.xml", + ) + + +def test_read_archive_member_rejects_a_directory_member(): + payload = _zip_bytes({"demo-1.0/": "", "demo-1.0/package.xml/": ""}) + with pytest.raises(KeyError): + _read_archive_member( + payload=payload, + url="https://example.com/demo-1.0.zip", + member="package.xml", + ) + + +def test_get_released_repo_returns_the_checksum_of_an_archive(): + distro = Distro.__new__(Distro) + distro.snapshot = { + "demo_pkg": { + "url": "https://example.com/demo-1.0.zip", + "sha256": "abc123", + "version": "1.0", + } + } + assert distro.get_released_repo("demo_pkg") == ( + "https://example.com/demo-1.0.zip", + "abc123", + "sha256", + ) + + +def test_get_released_repo_rejects_an_archive_without_a_checksum(): + distro = Distro.__new__(Distro) + distro.snapshot = {"demo_pkg": {"url": "https://example.com/demo-1.0.zip"}} + with pytest.raises(RuntimeError, match="no sha256 checksum"): + distro.get_released_repo("demo_pkg") + + +def test_get_released_repo_still_returns_a_git_reference(): + distro = Distro.__new__(Distro) + distro.snapshot = { + "demo_pkg": {"url": "https://github.com/org/repo.git", "rev": "deadbeef"} + } + assert distro.get_released_repo("demo_pkg") == ( + "https://github.com/org/repo.git", + "deadbeef", + "rev", + ) + + +def test_package_xml_is_read_from_a_downloaded_archive(monkeypatch): + distro = Distro.__new__(Distro) + distro._additional_xml_cache = {} + distro._last_archive = None + payload = _zip_bytes( + {"demo-1.0/": "", "demo-1.0/src/demo_pkg/package.xml": PACKAGE_XML} + ) + downloads = [] + + def fake_download(url): + downloads.append(url) + return payload + + monkeypatch.setattr(distro, "_download_archive_or_cached", fake_download) + pkg_info = { + "url": "https://example.com/demo-1.0.zip", + "sha256": "abc123", + "additional_folder": "src/demo_pkg", + } + assert distro.get_package_xml_for_additional_package(pkg_info) == PACKAGE_XML + # A second lookup is served from the cache. + assert distro.get_package_xml_for_additional_package(pkg_info) == PACKAGE_XML + assert downloads == ["https://example.com/demo-1.0.zip"] + + +def test_auth_headers_cover_only_the_hosters_vinca_knows(monkeypatch): + distro = Distro.__new__(Distro) + monkeypatch.setenv("GITHUB_TOKEN", "gh-token") + assert distro._get_auth_headers("https://raw.githubusercontent.com/o/r/f") == { + "Authorization": "token gh-token" + } + # Every other host is left to requests, which resolves the ambient credentials. + assert distro._get_auth_headers("https://artifacts.example.com/demo.zip") == {} + + +def test_get_forces_no_credentials_of_its_own(monkeypatch): + """Only the wiring of _get: that it raises for status and forces no auth. + + What requests then does with the ambient netrc, proxy and CA settings is its + own behaviour and is not re-tested here. + """ + distro = Distro.__new__(Distro) + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + calls = {} + + class FakeResponse: + content = b"payload" + text = "payload" + + def raise_for_status(self): + calls["raised"] = True + + def fake_get(url, **kwargs): + calls["url"] = url + calls["kwargs"] = kwargs + return FakeResponse() + + monkeypatch.setattr(requests, "get", fake_get) + assert distro._get("https://artifacts.example.com/demo.zip").content == b"payload" + assert calls["url"] == "https://artifacts.example.com/demo.zip" + assert calls["raised"] is True + # No auth is forced on the request, so requests applies the ambient credentials. + assert calls["kwargs"]["headers"] == {} + assert "auth" not in calls["kwargs"] + + +def test_requests_strips_authorization_when_a_redirect_changes_host(): + """The redirect behaviour vinca relies on instead of handling it itself. + + An Artifactory server answers a download with a redirect to a presigned URL on a + separate storage host, where the original credentials must not be replayed. + """ + session = requests.Session() + assert session.should_strip_auth( + "https://artifacts.example.com/demo.zip", "https://storage.example.net/signed" + ) + assert not session.should_strip_auth( + "https://artifacts.example.com/demo.zip", "https://artifacts.example.com/other" + ) + + +def test_archive_download_goes_through_get_and_is_cached(monkeypatch): + distro = Distro.__new__(Distro) + distro._last_archive = None + urls = [] + + class FakeResponse: + content = b"payload" + + def fake_get(url): + urls.append(url) + return FakeResponse() + + monkeypatch.setattr(distro, "_get", fake_get) + url = "https://artifacts.example.com/demo.zip" + assert distro._download_archive_or_cached(url) == b"payload" + assert distro._download_archive_or_cached(url) == b"payload" + assert urls == [url] + + +def test_source_reference_switches_between_git_and_archive(): + assert _source_reference( + url="https://example.com/demo-1.0.zip", ref="abc123", ref_type="sha256" + ) == {"url": "https://example.com/demo-1.0.zip", "sha256": "abc123"} + assert _source_reference( + url="https://github.com/org/repo.git", ref="deadbeef", ref_type="rev" + ) == {"git": "https://github.com/org/repo.git", "rev": "deadbeef"}