Merge pull request #150 from baszalmstra/fix/populate-package-repository

fix: populate package repository metadata

Author: baszalmstra

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

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("/")

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

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))

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")])
 

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 "<version>1.0.0</version>" 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 = {}