Resolve ArtifactInfo.location overloading for verify-installation (#4798)

Add a dedicated ``repo`` field to ``ArtifactInfo`` holding the DNF
repo_ids for repository providers (``None`` for download providers whose
artifacts land in the shared repo). This replaces the workaround in
``_inject_verify_phase()`` that used ``provider.get_repositories()`` as
a proxy and stored a comma-joined string of repo_ids, fixing the
last-writer-wins bug where the same package in multiple providers would
silently drop all but one valid source repo.

The ``verify`` field in ``PrepareVerifyInstallationData`` is changed
from ``dict[str, str]`` to ``dict[str, list[str]]`` so that a package
can be expected from any of multiple repos (OR semantics). A new
``normalize_string_list_dict`` normalizer coerces single string values
to one-element lists, keeping existing fmf plans working without
changes.

The ``verify: false`` workaround in the nvr-priority TC 1.1 test is
removed as it is no longer needed.

Related to #4714

Assisted-by: Claude

Author: vaibhavdaren

tests/prepare/artifact/nvr-priority/data/tc11.fmf

@@ -14,7 +14,6 @@ summary: TC 1.1 - Repository priority overrides package version
           script: mkdir -p /tmp/nvr-test && cp -r tc11-high tc11-low /tmp/nvr-test/
         - name: Install repo files
           how: artifact
-          verify: false
           provide:
             - repository-file:file:tc11-high.repo
             - repository-file:file:tc11-low.repo

tests/unit/artifact/test_base.py

@@ -5,7 +5,11 @@
 
 import tmt.utils
 from tmt.steps.prepare.artifact import PrepareArtifact
-from tmt.steps.prepare.artifact.providers import ArtifactInfo, ArtifactProvider, Version
+from tmt.steps.prepare.artifact.providers import (
+    ArtifactInfo,
+    ArtifactProvider,
+    Version,
+)
 
 
 class MockProvider(ArtifactProvider):
@@ -82,6 +86,7 @@ def test_persist_artifact_metadata(tmp_path, mock_provider):
         },
         "nvra": "mock-1.0-1.x86_64",
         "location": "http://example.com/mock-1.0-1.x86_64.rpm",
+        "repo_id": "tmt-artifact-shared",
     }
     assert artifact == expected
 

tests/unit/artifact/test_copr_repository.py

@@ -58,8 +58,12 @@ def test_enumerate_artifacts(
     mock_guest.package_manager = mock_package_manager
 
     mock_package_manager.list_packages.return_value = [
-        RpmVersion.from_nevra('tmt-1.69.0-1.fc42.noarch'),
-        RpmVersion.from_nevra('tmt-all-0:1.69.0-1.fc42.noarch'),
+        RpmVersion.from_nevra(
+            'tmt-1.69.0-1.fc42.noarch', repo_id='group_teemtee-stable-fedora-42-x86_64'
+        ),
+        RpmVersion.from_nevra(
+            'tmt-all-0:1.69.0-1.fc42.noarch', repo_id='group_teemtee-stable-fedora-42-x86_64'
+        ),
     ]
 
     provider = artifact_provider("copr.repository:@teemtee/stable")

tests/unit/artifact/test_file.py

@@ -74,6 +74,7 @@ def test_download_artifact(tmp_path, artifact_provider):
                 "version": "5.1.8",
                 "release": "6.el9",
                 "arch": "x86_64",
+                "repo_id": None,
             },
         ),
         (
@@ -84,6 +85,7 @@ def test_download_artifact(tmp_path, artifact_provider):
                 "version": "1.61.0.dev17+gf29b2e83e",
                 "release": "1.fc41",
                 "arch": "x86_64",
+                "repo_id": None,
             },
         ),
     ],

tmt/package_managers/_rpm.py

@@ -3,7 +3,7 @@
 """
 
 import re
-from typing import Any
+from typing import Any, Optional
 
 from tmt._compat.typing import Self
 from tmt.container import container
@@ -20,6 +20,9 @@ class RpmVersion(Version):
     Represents an RPM package version.
     """
 
+    #: The repository section ID this package was listed from, if known.
+    repo_id: Optional[str] = None
+
     @classmethod
     def from_rpm_meta(cls, rpm_meta: dict[str, Any]) -> Self:
         """
@@ -69,7 +72,7 @@ def from_filename(cls, filename: str) -> Self:
         return cls(name=name, version=version, release=release, arch=arch, epoch=0)
 
     @classmethod
-    def from_nevra(cls, nevra: str) -> Self:
+    def from_nevra(cls, nevra: str, repo_id: Optional[str] = None) -> Self:
         """
         Version constructed from a NEVRA string as returned by ``dnf repoquery``.
 
@@ -90,4 +93,5 @@ def from_nevra(cls, nevra: str) -> Self:
             version=match.group('version'),
             release=match.group('release'),
             arch=match.group('arch'),
+            repo_id=repo_id,
         )

tmt/package_managers/dnf.py

@@ -185,12 +185,16 @@ def install_repository(self, repository: Repository) -> ShellScript:
         )
 
     def list_packages(self, repository: Repository) -> ShellScript:
-        repo_ids = " ".join(f"--enablerepo={repo_id}" for repo_id in repository.repo_ids)
-        return ShellScript(
-            f"""
-            {self.command.to_script()} repoquery --disablerepo='*' {repo_ids}
-            """
-        )
+        return (
+            self.command
+            + Command(
+                'repoquery',
+                '--disablerepo=*',
+                *[f'--enablerepo={repo_id}' for repo_id in repository.repo_ids],
+                '--queryformat',
+                r'%{repoid};%{name}-%{epoch}:%{version}-%{release}.%{arch}\n',
+            )
+        ).to_script()
 
     def get_package_origin(self, packages: Iterable[str]) -> ShellScript:
         return (
@@ -253,8 +257,14 @@ def list_packages(self, repository: Repository) -> list[Version]:
         if stdout is None:
             raise GeneralError("Repository query provided no output")
 
-        stripped_lines = (line.strip() for line in stdout.strip().splitlines())
-        return [RpmVersion.from_nevra(line) for line in stripped_lines if line]
+        result: list[Version] = []
+        for line in stdout.strip().splitlines():
+            line = line.strip()
+            if not line:
+                continue
+            repo_id, nevra = line.split(';', maxsplit=1)
+            result.append(RpmVersion.from_nevra(nevra, repo_id=repo_id))
+        return result
 
     def check_presence(self, *installables: Installable) -> dict[Installable, bool]:
         try:

tmt/steps/prepare/artifact/__init__.py

@@ -12,6 +12,7 @@
 from tmt.steps.prepare import PreparePlugin, PrepareStepData
 from tmt.steps.prepare.artifact.providers import (
     _PROVIDER_REGISTRY,
+    SHARED_REPO_NAME,
     ArtifactProvider,
     Repository,
 )
@@ -198,7 +199,6 @@ class PrepareArtifact(PreparePlugin[PrepareArtifactData]):
 
     # Shared repository configuration
     SHARED_REPO_DIR_NAME: ClassVar[str] = 'artifact-shared-repo'
-    SHARED_REPO_NAME: ClassVar[str] = 'tmt-artifact-shared'
     ARTIFACTS_METADATA_FILENAME: ClassVar[str] = 'artifacts.yaml'
 
     #: Name of the auto-injected verify-installation phase.
@@ -228,7 +228,7 @@ def go(
             artifact_dir=shared_repo_dir,
             guest=guest,
             logger=logger,
-            repo_name=self.SHARED_REPO_NAME,
+            repo_name=SHARED_REPO_NAME,
             priority=self.data.default_repository_priority,
         )
 
@@ -355,29 +355,12 @@ def _inject_verify_phase(self, providers: list[ArtifactProvider], guest: Guest)
             for pkg in install_phase.data.package:  # pyright: ignore[reportUnknownMemberType,reportUnknownVariableType]
                 pkg_names.add(str(pkg))  # pyright: ignore[reportUnknownArgumentType]
 
-        # Build package → origin repo mapping from all providers.
-        # TODO: ``artifact.location`` is overloaded: a download URL for download
-        # providers and a repo name for repository providers. We use
-        # ``provider.get_repositories()`` as a proxy until ``ArtifactInfo`` gains
-        # a dedicated ``repo`` field. See https://github.com/teemtee/tmt/issues/4714.
-        pkg_to_repo: dict[str, str] = {}
+        # Build package → set of valid repo_ids, filtering to only required packages.
+        pkgs_to_verify: dict[str, set[str]] = {}
         for provider in providers:
-            repositories = provider.get_repositories()
-            if repositories:
-                # Repository provider: collect all repo_ids from all repositories.
-                # A .repo file may declare multiple sections; any of them is a valid origin.
-                all_repo_ids = ','.join(
-                    repo_id for repo in repositories for repo_id in repo.repo_ids
-                )
-                for artifact in provider.artifacts:
-                    pkg_to_repo[artifact.version.name] = all_repo_ids
-            else:
-                # Download provider: packages land in the shared repo.
-                for artifact in provider.artifacts:
-                    pkg_to_repo[artifact.version.name] = self.SHARED_REPO_NAME
-
-        # Only verify packages that are both required and from a known artifact.
-        pkgs_to_verify = {pkg: repo for pkg, repo in pkg_to_repo.items() if pkg in pkg_names}
+            for artifact in provider.artifacts:
+                if artifact.version.name in pkg_names:
+                    pkgs_to_verify.setdefault(artifact.version.name, set()).add(artifact.repo_id)
 
         if not pkgs_to_verify:
             self.verbose('No packages to be installed were found in the provided artifacts.')
@@ -397,8 +380,10 @@ def _inject_verify_phase(self, providers: list[ArtifactProvider], guest: Guest)
         )
 
         if existing_verify is not None:
-            # Merge into existing verify phase.
-            existing_verify.data.verify.update(pkgs_to_verify)  # pyright: ignore[reportUnknownMemberType]
+            # Merge into existing verify phase, extending repo lists rather than replacing them.
+            for verify_pkg, verify_repos in pkgs_to_verify.items():
+                existing = existing_verify.data.verify.setdefault(verify_pkg, [])  # pyright: ignore[reportUnknownMemberType,reportUnknownVariableType]
+                existing.extend(repo_id for repo_id in verify_repos if repo_id not in existing)  # pyright: ignore[reportUnknownMemberType]
         else:
             # Create and add a new verify phase.
             verify_data = PrepareVerifyInstallationData(
@@ -407,7 +392,7 @@ def _inject_verify_phase(self, providers: list[ArtifactProvider], guest: Guest)
                 summary=self.VERIFY_PHASE_SUMMARY,
                 order=tmt.steps.PHASE_ORDER_PREPARE_VERIFY_INSTALLATION,
                 where=list(self.data.where),
-                verify=pkgs_to_verify,
+                verify={pkg: sorted(repo_ids) for pkg, repo_ids in pkgs_to_verify.items()},
             )
             verify_phase = PreparePlugin.delegate(self.step, data=verify_data)  # pyright: ignore[reportUnknownVariableType]
             self.step.add_phase(verify_phase)  # pyright: ignore[reportUnknownArgumentType]

tmt/steps/prepare/artifact/providers/__init__.py

@@ -23,6 +23,9 @@
     r'^(?P<name>.+)-(?:(?P<epoch>\d+):)?(?P<version>.+)-(?P<release>.+)\.(?P<arch>.+)$'
 )
 
+#: Name of the shared repository that download providers contribute RPMs into.
+SHARED_REPO_NAME: str = 'tmt-artifact-shared'
+
 
 class DownloadError(tmt.utils.GeneralError):
     """
@@ -45,6 +48,9 @@ class ArtifactInfo:
     version: Version
     location: str
     provider: "ArtifactProvider"
+    #: Repository ID this artifact is available from. Used during verification
+    #: to confirm the artifact was installed from the expected repository.
+    repo_id: str = SHARED_REPO_NAME
 
     @property
     def id(self) -> str:
@@ -254,11 +260,24 @@ def enumerate_artifacts(self, guest: Guest) -> None:
                 )
                 continue
             for rpm_version in packages:
+                from tmt.package_managers._rpm import RpmVersion
+
+                if not isinstance(rpm_version, RpmVersion):
+                    raise tmt.utils.GeneralError(
+                        f"Unexpected package type '{type(rpm_version).__name__}' "
+                        f"from repository '{repository.name}'."
+                    )
+                if rpm_version.repo_id is None:
+                    raise tmt.utils.GeneralError(
+                        f"Package '{rpm_version}' from repository '{repository.name}' "
+                        f"has no repo_id."
+                    )
                 self._artifacts.append(
                     ArtifactInfo(
                         version=rpm_version,
                         provider=self,
                         location=repository.name,
+                        repo_id=rpm_version.repo_id,
                     )
                 )
             self.logger.debug(
@@ -303,6 +322,7 @@ def artifact_metadata(self) -> list[dict[str, Any]]:
                 'version': vars(artifact.version),
                 'nvra': artifact.version.nvra,
                 'location': artifact.location,
+                'repo_id': artifact.repo_id,
             }
             for artifact in self.artifacts
         ]

tmt/steps/prepare/artifact/providers/copr_build.py

@@ -220,7 +220,9 @@ def make_rpm_artifact(self, rpm_meta: dict[str, str]) -> ArtifactInfo:
             base_url = self.result_url.rstrip("/")
 
         return ArtifactInfo(
-            version=version_info, location=urljoin(base_url + "/", filename), provider=self
+            version=version_info,
+            location=urljoin(base_url + "/", filename),
+            provider=self,
         )
 
     @cached_property

tmt/steps/prepare/artifact/providers/koji.py

@@ -227,7 +227,9 @@ def make_rpm_artifact(self, rpm_meta: dict[str, Any]) -> ArtifactInfo:
         )
 
         return ArtifactInfo(
-            version=version_info, location=urljoin(self._top_url, path), provider=self
+            version=version_info,
+            location=urljoin(self._top_url, path),
+            provider=self,
         )
 
 
@@ -269,7 +271,9 @@ def make_rpm_artifact(self, task_id: int, filename: str) -> ArtifactInfo:  # typ
         url = f"{work_path}/{task_path}/{filename}"
 
         return ArtifactInfo(
-            version=RpmVersion.from_filename(filename), location=url, provider=self
+            version=RpmVersion.from_filename(filename),
+            location=url,
+            provider=self,
         )
 
     @cached_property