diff --git a/vinca/distro.py b/vinca/distro.py index 8cbce7b..597323c 100644 --- a/vinca/distro.py +++ b/vinca/distro.py @@ -38,6 +38,18 @@ def is_archive_url(url): return path.endswith(ARCHIVE_SUFFIXES) +def is_bloom_release_repository_url(url): + """Return True when url names a Bloom-generated release repository.""" + path = urllib.parse.urlparse(url).path.rstrip("/") + repository_name = posixpath.basename(path).removesuffix(".git").lower() + return repository_name.endswith(("-release", "_release")) + + +def _strip_git_suffix(url): + """Return a repository URL without its optional ``.git`` suffix.""" + return url[:-4] if url.lower().endswith(".git") else url + + def _normalize_member(name): """Return an archive member name without its './' prefix and trailing slash.""" name = name.strip("/") @@ -314,6 +326,38 @@ def get_released_repo(self, pkg_name): release_tag = get_release_tag(repo, pkg_name) return repo.url, release_tag, "tag" + def get_repository_url(self, pkg_name, package_urls=()): + """Return the best declared upstream repository for a package.""" + pkg_info = self._get_snapshot_package_info(pkg_name) + if pkg_info is not None: + if repository := pkg_info.get("repository"): + return _strip_git_suffix(repository) + + additional_info = (self.additional_packages_snapshot or {}).get(pkg_name) + if additional_info is not None: + if repository := additional_info.get("repository"): + return _strip_git_suffix(repository) + else: + package = self._distro.release_packages.get(pkg_name) + if package is not None: + repository = self._distro.repositories[package.repository_name] + source_repository = repository.source_repository + if ( + source_repository is not None + and source_repository.url + and not is_bloom_release_repository_url(source_repository.url) + ): + return _strip_git_suffix(source_repository.url) + + for package_url in package_urls: + if ( + package_url.type == "repository" + and package_url.url + and not is_bloom_release_repository_url(package_url.url) + ): + return _strip_git_suffix(package_url.url) + return None + def check_package(self, pkg_name): # If the package is in the additional_packages_snapshot, it is always considered valid # even if it is not in the released packages, as it is an additional diff --git a/vinca/main.py b/vinca/main.py index 51e16de..6a3f984 100644 --- a/vinca/main.py +++ b/vinca/main.py @@ -479,14 +479,12 @@ def parse_package(pkg, distro, vinca_conf, path): recipe["about"]["maintainers"].append(name) for u in pkg["urls"]: - # if u.type == 'repository' : - # recipe['source']['git'] = u.url - # recipe['source']['tag'] = recipe['package']['version'] if u.type == "website": recipe["about"]["homepage"] = u.url - # if u.type == 'bugtracker' : - # recipe['about']['url_issues'] = u.url + repository = distro.get_repository_url(pkg.name, pkg["urls"]) + if repository: + recipe["about"]["repository"] = repository if not recipe["source"].get("git", None): aux = path.split("/") diff --git a/vinca/recipes.py b/vinca/recipes.py index 6b883d7..dbba622 100644 --- a/vinca/recipes.py +++ b/vinca/recipes.py @@ -286,14 +286,17 @@ def _requirement_sort_key(requirement): ) -def _add_metadata(output: dict[str, Any], package: Any, shortname: str) -> None: - """Populate ``about`` from the package.xml URLs, license and description.""" +def _add_metadata( + output: dict[str, Any], package: Any, shortname: str, distro: Distro +) -> None: + """Populate ``about`` from package.xml and rosdistro metadata.""" about = output["about"] = {} for url in package.urls: if url.type == "website": about["homepage"] = url.url - elif url.type == "repository": - about["repository"] = url.url + repository = distro.get_repository_url(shortname, package.urls) + if repository: + about["repository"] = repository if package.licenses: license_expression = convert_to_spdx_license( [str(license) for license in package.licenses], package_name=shortname @@ -459,5 +462,5 @@ def generate_output( _adjust_requirements(output["requirements"], package_prefix) if dependencies_only: return output["requirements"] - _add_metadata(output, package, shortname) + _add_metadata(output, package, shortname, distro) return output diff --git a/vinca/snapshot.py b/vinca/snapshot.py index cdf10b3..c06fa2b 100644 --- a/vinca/snapshot.py +++ b/vinca/snapshot.py @@ -79,6 +79,8 @@ def main(): continue output[dep] = {"url": url, "version": version, "tag": tag} + if repository := distro.get_repository_url(dep): + output[dep]["repository"] = repository if not args.quiet: print("{0:{2}} {1}".format(dep, version, max_len + 2)) diff --git a/vinca/test_recipes.py b/vinca/test_recipes.py index 1a2f2fd..2809d87 100644 --- a/vinca/test_recipes.py +++ b/vinca/test_recipes.py @@ -40,13 +40,22 @@ def package_xml(name, build_type="ament_cmake", depends=()): class FakeDistro: name = "rolling" - def __init__(self, xml_by_name, ros1=False): + def __init__(self, xml_by_name, ros1=False, repository_by_name=None): self._xml_by_name = xml_by_name self._ros1 = ros1 + self._repository_by_name = repository_by_name or {} def get_release_package_xml(self, name): return self._xml_by_name.get(name) + def get_repository_url(self, name, package_urls=()): + if repository := self._repository_by_name.get(name): + return repository.removesuffix(".git") + for package_url in package_urls: + if package_url.type == "repository": + return package_url.url.removesuffix(".git") + return None + def check_ros1(self): return self._ros1 @@ -90,7 +99,10 @@ def build( dependencies_only=False, **overrides, ): - distro = FakeDistro({name: package_xml(name, build_type, depends)}) + distro = FakeDistro( + {name: package_xml(name, build_type, depends)}, + repository_by_name={name: f"https://github.com/ros2/{name}.git"}, + ) return generate_output( name, make_config(name, **overrides), @@ -154,13 +166,32 @@ def test_generate_output_produces_a_complete_recipe(): }, "about": { "homepage": "https://example.org/demo", - "repository": "https://github.com/example/demo", + "repository": "https://github.com/ros2/demo", "license": "Apache-2.0", "summary": "Description of demo.", }, } +def test_generate_output_uses_rosdistro_repository_instead_of_manifest_url(): + distro = FakeDistro( + {"demo": package_xml("demo")}, + repository_by_name={"demo": "https://github.com/ros2/demo.git"}, + ) + + output = generate_output("demo", make_config("demo"), distro, "1.2.3") + + assert output["about"]["repository"] == "https://github.com/ros2/demo" + + +def test_generate_output_uses_manifest_repository_as_fallback(): + distro = FakeDistro({"demo": package_xml("demo")}) + + output = generate_output("demo", make_config("demo"), distro, "1.2.3") + + assert output["about"]["repository"] == "https://github.com/example/demo" + + def test_cmake_is_build_only_on_emscripten(): output = build("demo", depends=[("exec_depend", "cmake")]) diff --git a/vinca/test_snapshot_metadata.py b/vinca/test_snapshot_metadata.py index 0a57dd8..513e7ee 100644 --- a/vinca/test_snapshot_metadata.py +++ b/vinca/test_snapshot_metadata.py @@ -33,11 +33,13 @@ def make_snapshot_distro(monkeypatch): snapshot = { "snapshot_package": { "url": "https://github.com/example/snapshot-package-release.git", + "repository": "https://github.com/example/snapshot-package.git", "version": "1.0.0", "tag": "release/rolling/snapshot_package/1.0.0-1", }, "snapshot_dependency": { "url": "https://github.com/example/snapshot-dependency-release.git", + "repository": "https://github.com/example/snapshot-dependency.git", "version": "1.0.0", "tag": "release/rolling/snapshot_dependency/1.0.0-1", }, @@ -83,6 +85,10 @@ def test_snapshot_package_xml_and_dependencies_do_not_follow_live_rosdistro( "release/rolling/snapshot_package/1.0.0-1", "tag", ) + assert ( + distro.get_repository_url("snapshot_package") + == "https://github.com/example/snapshot-package" + ) assert distro.get_version("snapshot_package") == "1.0.0" assert "1.0.0" in package_xml_content assert "snapshot_dependency" in package_xml_content @@ -173,6 +179,9 @@ def test_snapshot_metadata_generates_dependency_required_by_pinned_source( "name": "ros2-snapshot-package", "version": "1.0.0", } + assert output["about"]["repository"] == ( + "https://github.com/example/snapshot-package" + ) assert "ros2-snapshot-dependency" in output["requirements"]["host"] assert "ros2-live-dependency" not in output["requirements"]["host"] @@ -189,6 +198,90 @@ def test_snapshot_is_authoritative_for_package_membership(monkeypatch): } +def test_live_repository_url_requires_upstream_source_metadata(): + distro = Distro.__new__(Distro) + distro.snapshot = None + distro.additional_packages_snapshot = None + distro._distro = Mock() + distro._distro.release_packages = { + "source_package": Mock(repository_name="source-package"), + "release_only_package": Mock(repository_name="release-only-package"), + "bloom_source_package": Mock(repository_name="bloom-source-package"), + } + distro._distro.repositories = { + "source-package": Mock( + source_repository=Mock(url="https://github.com/example/source.git"), + release_repository=Mock( + url="https://github.com/example/source-release.git" + ), + ), + "release-only-package": Mock( + source_repository=None, + release_repository=Mock( + url="https://github.com/example/release-only-release.git" + ), + ), + "bloom-source-package": Mock( + source_repository=Mock( + url="https://github.com/example/bloom-source-release.git" + ), + release_repository=Mock( + url="https://github.com/ros2-gbp/bloom-source-release.git" + ), + ), + } + + assert ( + distro.get_repository_url("source_package") + == "https://github.com/example/source" + ) + assert distro.get_repository_url("release_only_package") is None + assert ( + distro.get_repository_url( + "release_only_package", + [Mock(type="repository", url="https://github.com/example/upstream.git")], + ) + == "https://github.com/example/upstream" + ) + assert distro.get_repository_url("bloom_source_package") is None + + +def test_additional_package_repository_must_be_explicit(): + distro = Distro.__new__(Distro) + distro.snapshot = None + distro.additional_packages_snapshot = { + "explicit": { + "url": "https://github.com/example/explicit-release.git", + "repository": "https://github.com/example/explicit.git", + }, + "source_only": {"url": "https://github.com/example/source-only.git"}, + } + + assert ( + distro.get_repository_url("explicit") == "https://github.com/example/explicit" + ) + assert distro.get_repository_url("source_only") is None + assert ( + distro.get_repository_url( + "source_only", + [Mock(type="repository", url="https://github.com/example/source-only.git")], + ) + == "https://github.com/example/source-only" + ) + assert ( + distro.get_repository_url( + "source_only", + [ + Mock( + type="repository", + url="https://github.com/example/source-only-release.git", + ) + ], + ) + is None + ) + + def test_empty_snapshot_keeps_live_rosdistro_behavior(): distro = Distro.__new__(Distro) distro.snapshot = {}