Fix failed download and copy of artifact rpms (#5108)

Fixes #4886 and
#5018.

Second commit might be irrelevant since the first one raises on the
no-download problem. Not sure if a scenario where the `try/except` block
would actually go with the `except` clause could happen now, but
#5020 should introduce an option to
allow for this behavior, so I kept the checks.

Co-authored-by: Petr Šplíchal <psplicha@redhat.com>

Author: bajertom

tests/prepare/artifact/providers/file/test.sh

@@ -55,6 +55,14 @@ rlJournalStart
                 prepare --how artifact --provide file:$LIB_DIR/../rpms/bar" \
                 0 "Run directory"
         rlPhaseEnd
+
+        rlPhaseStartTest "$phase_prefix Fail on nonexistent file"
+            rlRun -s "tmt run -i $run --scratch -vvv --all \
+                provision -h $PROVISION_HOW --image $image \
+                prepare --how artifact --provide file:/no/such/package.rpm" \
+                2 "Nonexistent file should fail"
+            rlAssertGrep "No artifacts were downloaded" $rlRun_LOG
+        rlPhaseEnd
     done <<< "$IMAGES"
 
     rlPhaseStartCleanup

tmt/steps/prepare/artifact/__init__.py

@@ -285,11 +285,12 @@ def go(
                 provider.fetch_contents(guest, download_path)
 
                 # Then, have the provider contribute to the shared repository
-                provider.contribute_to_shared_repo(
-                    guest=guest,
-                    source_path=download_path,
-                    shared_repo_dir=shared_repo_dir,
-                )
+                if provider.downloads_artifacts:
+                    provider.contribute_to_shared_repo(
+                        guest=guest,
+                        source_path=download_path,
+                        shared_repo_dir=shared_repo_dir,
+                    )
 
             except tmt.utils.PrepareError:
                 raise

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

@@ -8,7 +8,7 @@
 from functools import cached_property
 from re import Pattern
 from shlex import quote
-from typing import TYPE_CHECKING, Any, Optional
+from typing import TYPE_CHECKING, Any, ClassVar, Optional
 
 import tmt.log
 import tmt.utils
@@ -115,6 +115,10 @@ class ArtifactProvider(ABC):
     #: The PrepareArtifact phase that owns this provider.
     parent: "PrepareArtifact"
 
+    #: Whether this provider downloads individual artifact files.
+    #: Providers that only register repositories should set this to ``False``.
+    downloads_artifacts: ClassVar[bool] = True
+
     def __post_init__(self) -> None:
         self.id = self._extract_provider_id(self.raw_id)
 
@@ -219,6 +223,11 @@ def fetch_contents(
                     f"Unexpected error downloading '{artifact}'."
                 ) from error
 
+        if not downloaded_paths:
+            raise tmt.utils.PrepareError(
+                f"No artifacts were downloaded for provider '{self.raw_id}'. "
+                f"Verify the provider identifier is correct."
+            )
         self.logger.info(f"Successfully downloaded '{len(downloaded_paths)}' artifacts.")
         return downloaded_paths
 
@@ -284,15 +293,11 @@ def enumerate_artifacts(self, guest: Guest) -> None:
                 f"Enumerated {len(packages)} packages from repository '{repository.name}'."
             )
 
-    # B027: "... is an empty method in an abstract base class, but has
-    # no abstract decorator" - expected, it's a default implementation
-    # provided for subclasses. It is acceptable to do nothing.
-    def contribute_to_shared_repo(  # noqa: B027
+    def contribute_to_shared_repo(
         self,
         guest: Guest,
         source_path: Path,
         shared_repo_dir: Path,
-        exclude_patterns: Optional[list[Pattern[str]]] = None,
     ) -> None:
         """
         Contribute artifacts to the shared repository.
@@ -305,10 +310,20 @@ def contribute_to_shared_repo(  # noqa: B027
         :param source_path: path where the artifacts are located (source for contribution).
         :param shared_repo_dir: path to the shared repository directory where
             artifacts should be contributed.
-        :param exclude_patterns: if set, artifacts whose names match any
-            of the given regular expressions would not be contributed.
         """
-        pass
+        try:
+            guest.execute(
+                ShellScript(f"cp {quote(str(source_path))}/*.rpm {quote(str(shared_repo_dir))}")
+            )
+        except tmt.utils.RunError as error:
+            if error.stderr and "No such file" in error.stderr:
+                self.logger.warning(f"No artifacts to contribute from '{source_path}'.")
+                return
+            raise tmt.utils.PrepareError(
+                f"Failed to copy artifacts from '{source_path}' to '{shared_repo_dir}'."
+            ) from error
+
+        self.logger.info(f"Contributed artifacts from '{source_path}' to '{shared_repo_dir}'.")
 
     @property
     def artifact_metadata(self) -> list[dict[str, Any]]:

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

@@ -5,23 +5,20 @@
 import types
 from collections.abc import Sequence
 from functools import cached_property
-from shlex import quote
 from typing import TYPE_CHECKING, Any, Optional
 from urllib.parse import urljoin
 
 import tmt.log
 import tmt.utils
 import tmt.utils.hints
 from tmt.container import container, simple_field
-from tmt.guest import Guest
 from tmt.package_managers._rpm import RpmVersion
 from tmt.steps.prepare.artifact.providers import (
     ArtifactInfo,
     ArtifactProvider,
     ArtifactProviderId,
     provides_artifact_provider,
 )
-from tmt.utils import ShellScript
 
 if TYPE_CHECKING:
     from munch import Munch
@@ -216,15 +213,3 @@ def artifacts(self) -> Sequence[ArtifactInfo]:
         rpm_metas = self._fetch_results_json() if self.is_pulp else self.build_packages
 
         return [self.make_rpm_artifact(rpm_meta) for rpm_meta in rpm_metas]
-
-    def contribute_to_shared_repo(
-        self,
-        guest: Guest,
-        source_path: tmt.utils.Path,
-        shared_repo_dir: tmt.utils.Path,
-        exclude_patterns: Optional[list[tmt.utils.Pattern[str]]] = None,
-    ) -> None:
-        guest.execute(
-            ShellScript(f"cp {quote(str(source_path))}/*.rpm {quote(str(shared_repo_dir))}")
-        )
-        self.logger.info(f"Contributed artifacts from '{source_path}' to '{shared_repo_dir}'.")

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

@@ -67,6 +67,7 @@ class CoprRepositoryProvider(ArtifactProvider):
     """
 
     repository: Optional[Repository] = None
+    downloads_artifacts = False
 
     @classmethod
     def _extract_provider_id(cls, raw_id: str) -> ArtifactProviderId:

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

@@ -2,8 +2,7 @@
 import urllib.parse
 from collections.abc import Sequence
 from functools import cached_property
-from shlex import quote
-from typing import ClassVar, Optional
+from typing import ClassVar
 
 import tmt.utils
 from tmt.container import container, simple_field
@@ -16,7 +15,6 @@
     DownloadError,
     provides_artifact_provider,
 )
-from tmt.utils import ShellScript
 
 
 @provides_artifact_provider("file")
@@ -126,15 +124,3 @@ def _download_artifact(
             self.logger.info(f"Successfully downloaded: '{artifact.id}'.")
         except Exception as error:
             raise DownloadError(f"Failed to download '{artifact}'.") from error
-
-    def contribute_to_shared_repo(
-        self,
-        guest: Guest,
-        source_path: tmt.utils.Path,
-        shared_repo_dir: tmt.utils.Path,
-        exclude_patterns: Optional[list[tmt.utils.Pattern[str]]] = None,
-    ) -> None:
-        guest.execute(
-            ShellScript(f"cp {quote(str(source_path))}/*.rpm {quote(str(shared_repo_dir))}")
-        )
-        self.logger.info(f"Contributed artifacts from '{source_path}' to '{shared_repo_dir}'.")

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

@@ -6,23 +6,20 @@
 from abc import abstractmethod
 from collections.abc import Iterator, Sequence
 from functools import cached_property
-from shlex import quote
 from typing import Any, Optional, TypeVar
 from urllib.parse import urljoin
 
 import tmt.log
 import tmt.utils
 import tmt.utils.hints
 from tmt.container import container, simple_field
-from tmt.guest import Guest
 from tmt.package_managers._rpm import RpmVersion
 from tmt.steps.prepare.artifact.providers import (
     ArtifactInfo,
     ArtifactProvider,
     ArtifactProviderId,
     provides_artifact_provider,
 )
-from tmt.utils import ShellScript
 
 koji: Optional[types.ModuleType] = None
 
@@ -178,18 +175,6 @@ def _extract_provider_id(cls, raw_id: str) -> ArtifactProviderId:
             ) from exc
         return value
 
-    def contribute_to_shared_repo(
-        self,
-        guest: Guest,
-        source_path: tmt.utils.Path,
-        shared_repo_dir: tmt.utils.Path,
-        exclude_patterns: Optional[list[tmt.utils.Pattern[str]]] = None,
-    ) -> None:
-        guest.execute(
-            ShellScript(f"cp {quote(str(source_path))}/*.rpm {quote(str(shared_repo_dir))}")
-        )
-        self.logger.info(f"Contributed artifacts from '{source_path}' to '{shared_repo_dir}'.")
-
     def make_rpm_artifact(self, rpm_meta: dict[str, Any]) -> ArtifactInfo:
         """
         Create a normal build RPM artifact from metadata returned by listBuildRPMs.

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

@@ -44,6 +44,7 @@ class RepositoryFileProvider(ArtifactProvider):
     """
 
     repository: Repository = simple_field(init=False)
+    downloads_artifacts = False
 
     @classmethod
     def _extract_provider_id(cls, raw_id: str) -> ArtifactProviderId:

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

@@ -42,6 +42,7 @@ class RepositoryUrlProvider(ArtifactProvider):
     #        is made dynamic.
 
     repository: Repository = simple_field(init=False)
+    downloads_artifacts = False
 
     @classmethod
     def _extract_provider_id(cls, raw_id: str) -> ArtifactProviderId: