From 45d6e8b23135dbbe99084fab0e1e4f26218d9126 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 2 Sep 2026 14:32:40 +0700 Subject: [PATCH] Snapshot rosdistro dependency metadata Signed-off-by: Maurice --- vinca/distro.py | 81 ++++++++++----------------------- vinca/snapshot.py | 30 +++++++++--- vinca/test_snapshot_metadata.py | 47 +++++++++++++++++-- 3 files changed, 91 insertions(+), 67 deletions(-) diff --git a/vinca/distro.py b/vinca/distro.py index 61b6f60..d934432 100644 --- a/vinca/distro.py +++ b/vinca/distro.py @@ -7,7 +7,6 @@ from concurrent.futures import ThreadPoolExecutor from typing import Iterable, Optional -import catkin_pkg.package import requests from rosdistro import get_cached_distribution, get_index, get_index_url from rosdistro.dependency_walker import DependencyWalker @@ -104,9 +103,12 @@ def __init__( python_version=None, snapshot=None, additional_packages_snapshot=None, + distribution_cache=None, ): index = get_index(get_index_url()) - self._distro = get_cached_distribution(index, distro_name) + self._distro = get_cached_distribution( + index, distro_name, cache=distribution_cache + ) self.distro_name = distro_name self.snapshot = snapshot self.additional_packages_snapshot = additional_packages_snapshot @@ -157,11 +159,6 @@ def get_depends( ignore_pkgs = set(ignore_pkgs or ()) - if self.snapshot: - dependencies = self._get_snapshot_recursive_depends(pkg, ignore_pkgs) - self._depends_cache[cache_key] = set(dependencies) - return dependencies - dependencies = set() visited = {pkg} packages_to_check = {pkg} @@ -182,17 +179,33 @@ def get_depends( self._depends_cache[cache_key] = set(dependencies) return dependencies + def get_direct_depends(self, pkg: str) -> set[str]: + """Return the direct ROS dependencies of a package.""" + return self._get_direct_depends(pkg) + def _get_direct_depends(self, pkg: str) -> set[str]: """Return direct dependencies, caching package metadata across root walks.""" if pkg in self._direct_depends_cache: return set(self._direct_depends_cache[pkg]) - # if pkg comes from additional_packages_snapshot, extract from its package.xml - if ( + snapshot_info = self._get_snapshot_package_info(pkg) + is_additional_package = ( self.additional_packages_snapshot and pkg in self.additional_packages_snapshot - ): + ) + if snapshot_info is not None and not is_additional_package: + if "dependencies" not in snapshot_info: + raise RuntimeError( + f"Snapshot metadata for '{pkg}' has no dependencies; " + "regenerate the rosdistro snapshot" + ) + direct = set(snapshot_info["dependencies"] or []) + self._direct_depends_cache[pkg] = set(direct) + return direct + + # if pkg comes from additional_packages_snapshot, extract from its package.xml + if is_additional_package: pkg_info = self.additional_packages_snapshot[pkg] xml_str = self.get_package_xml_for_additional_package(pkg_info) # parse XML @@ -237,46 +250,6 @@ def _get_direct_depends(self, pkg: str) -> set[str]: self._direct_depends_cache[pkg] = set(direct) return direct - def _get_snapshot_recursive_depends(self, pkg, ignore_pkgs=None): - """Return ROS dependencies using only package manifests pinned by the snapshot.""" - dependencies = set() - ignored = set(ignore_pkgs or []) - packages_to_check = {pkg} - checked_packages = set() - dependency_attributes = ( - "buildtool_depends", - "buildtool_export_depends", - "build_depends", - "build_export_depends", - "run_depends", - "test_depends", - "exec_depends", - ) - - while packages_to_check: - package_name = sorted(packages_to_check)[0] - packages_to_check.remove(package_name) - if package_name in ignored or package_name in checked_packages: - continue - checked_packages.add(package_name) - - package_xml = self.get_release_package_xml(package_name) - package = catkin_pkg.package.parse_package_string(package_xml) - package.evaluate_conditions(os.environ) - direct_dependencies = { - dependency.name - for attribute in dependency_attributes - for dependency in getattr(package, attribute) - if dependency.evaluated_condition is not False - and dependency.name not in ignored - and self.check_package(dependency.name) - } - new_dependencies = direct_dependencies - dependencies - dependencies |= new_dependencies - packages_to_check |= new_dependencies - checked_packages - - return dependencies - def _get_snapshot_package_info(self, pkg_name): if not self.snapshot: return None @@ -350,13 +323,7 @@ def get_version(self, pkg_name): return repo.version.split("-")[0] def live_cache_matches_snapshot(self, pkg_name, snapshot_entry): - """Return whether rosdistro's cached manifest is the pinned snapshot source. - - A snapshot pins the release repository URL, the package-specific release - tag, and the release version. Only an exact match may reuse rosdistro's - local ``DistributionCache``; otherwise the manifest must be read from the - immutable snapshot source. - """ + """Return whether rosdistro's cached manifest is the pinned snapshot source.""" for live_name in (pkg_name, pkg_name.replace("_", "-")): try: diff --git a/vinca/snapshot.py b/vinca/snapshot.py index 28793fd..f3daf0c 100644 --- a/vinca/snapshot.py +++ b/vinca/snapshot.py @@ -1,6 +1,14 @@ import argparse -import yaml import datetime + +import yaml +from rosdistro import ( + DistributionCache, + get_distribution_cache_string, + get_index, + get_index_url, +) + from .distro import Distro @@ -48,9 +56,14 @@ def main(): # Get the current UTC time utc_time = datetime.datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ") - # Note: we intentionally do not pass any kind of additional packages snapshot - # here, as it would pollute the snapshot with additional packages - distro = Distro(args.distro) + # Fetch the cache once and use that exact data for both the package pins and + # the dependency metadata stored in the snapshot. + # TODO: Consider referencing immutable cache release assets instead if they + # become available through https://github.com/ros/rosdistro/pull/50112. + index = get_index(get_index_url()) + cache_yaml = get_distribution_cache_string(index, args.distro) + cache = DistributionCache(args.distro, yaml.safe_load(cache_yaml)) + distro = Distro(args.distro, distribution_cache=cache) if args.package is None: deps = distro.get_package_names() @@ -64,7 +77,7 @@ def main(): output = {} - for dep in deps: + for dep in sorted(deps): try: url, tag, _ = distro.get_released_repo(dep) version = distro.get_version(dep) @@ -76,7 +89,12 @@ def main(): ) continue - output[dep] = {"url": url, "version": version, "tag": tag} + output[dep] = { + "url": url, + "version": version, + "tag": tag, + "dependencies": sorted(distro.get_direct_depends(dep)), + } if not args.quiet: print("{0:{2}} {1}".format(dep, version, max_len + 2)) diff --git a/vinca/test_snapshot_metadata.py b/vinca/test_snapshot_metadata.py index e8a89a8..5b66b02 100644 --- a/vinca/test_snapshot_metadata.py +++ b/vinca/test_snapshot_metadata.py @@ -1,5 +1,7 @@ from unittest.mock import Mock, patch +import pytest + import vinca.main as main from vinca.distro import Distro @@ -34,11 +36,13 @@ def make_snapshot_distro(monkeypatch): "url": "https://github.com/example/snapshot-package-release.git", "version": "1.0.0", "tag": "release/rolling/snapshot_package/1.0.0-1", + "dependencies": ["snapshot_dependency"], }, "snapshot_dependency": { "url": "https://github.com/example/snapshot-dependency-release.git", "version": "1.0.0", "tag": "release/rolling/snapshot_dependency/1.0.0-1", + "dependencies": [], }, } distro = Distro.__new__(Distro) @@ -51,9 +55,6 @@ def make_snapshot_distro(monkeypatch): distro._depends_cache = {} distro._direct_depends_cache = {} distro._distro = Mock() - distro._distro.get_release_package_xml.return_value = LIVE_PACKAGE_XML - distro._walker = Mock() - snapshot_xml_by_url = { "https://raw.githubusercontent.com/example/snapshot-package-release/" "release/rolling/snapshot_package/1.0.0-1/package.xml": (SNAPSHOT_PACKAGE_XML), @@ -67,6 +68,7 @@ def make_snapshot_distro(monkeypatch): "_download_raw_pkg_xml_or_cached", lambda url: snapshot_xml_by_url[url], ) + distro._walker = Mock() return distro @@ -88,7 +90,7 @@ def test_snapshot_package_xml_and_dependencies_do_not_follow_live_rosdistro( assert "live_dependency" not in package_xml_content assert distro.get_depends("snapshot_package") == {"snapshot_dependency"} distro._distro.get_release_package_xml.assert_not_called() - distro._walker.get_recursive_depends.assert_not_called() + distro._walker.get_depends.assert_not_called() def test_snapshot_package_xml_uses_matching_live_distribution_cache(monkeypatch): @@ -103,6 +105,7 @@ def test_snapshot_package_xml_uses_matching_live_distribution_cache(monkeypatch) distro._distro.repositories = { "snapshot-package": Mock(release_repository=release_repository) } + distro._distro.get_release_package_xml.return_value = LIVE_PACKAGE_XML with patch( "vinca.distro.get_release_tag", @@ -139,6 +142,14 @@ def test_snapshot_package_xml_does_not_use_live_cache_after_snapshot_change( distro._distro.get_release_package_xml.assert_not_called() +def test_snapshot_without_dependencies_requires_regeneration(monkeypatch): + distro = make_snapshot_distro(monkeypatch) + del distro.snapshot["snapshot_package"]["dependencies"] + + with pytest.raises(RuntimeError, match="regenerate the rosdistro snapshot"): + distro.get_depends("snapshot_package") + + def test_snapshot_metadata_generates_dependency_required_by_pinned_source( monkeypatch, ): @@ -209,3 +220,31 @@ def test_empty_snapshot_keeps_live_rosdistro_behavior(): assert distro.get_release_package_xml("live_package") == LIVE_PACKAGE_XML assert distro.get_depends("live_package") == {"live_dependency"} assert set(distro.get_package_names()) == {"live_package"} + + +def test_read_snapshot_merges_additional_packages(tmp_path, monkeypatch): + snapshot_path = tmp_path / "rosdistro_snapshot.yaml" + additional_path = tmp_path / "rosdistro_additional_recipes.yaml" + snapshot_path.write_text( + """\ +snapshot_package: + version: 1.0.0 +""" + ) + additional_path.write_text( + """\ +additional_package: + version: 2.0.0 +""" + ) + monkeypatch.chdir(tmp_path) + + snapshot, additional = main.read_snapshot( + { + "rosdistro_snapshot": snapshot_path.name, + "rosdistro_additional_recipes": additional_path.name, + } + ) + + assert set(snapshot) == {"snapshot_package", "additional_package"} + assert additional == {"additional_package": {"version": "2.0.0"}}