Add Compose.old_compose_path and use it when searching for files in old compose.

The current code calls `find_old_compose` followed by multiple `os.path.*`
calls to find out if particular file exists in the old compose. This
duplicates code a lot and makes it harder to read.

In this commit, new `Compose.old_compose_path` is introduced and
used instead of direct calls of `find_old_compose`.

Signed-off-by: Jan Kaluza <jkaluza@redhat.com>

pungi/paths.py

@@ -20,11 +20,13 @@ __all__ = ("Paths",)
 import errno
 import os
 
-from pungi.util import makedirs
+from kobo.shortcuts import relative_path
+from pungi.util import makedirs, find_old_compose
 
 
 class Paths(object):
     def __init__(self, compose):
+        self._compose = compose
         paths_module_name = compose.conf.get("paths_module")
         if paths_module_name:
             # custom paths
@@ -44,6 +46,46 @@ class Paths(object):
             self.work = WorkPaths(compose)
         # self.metadata ?
 
+    def get_old_compose_topdir(self, **kwargs):
+        """
+        Finds old compose using the `find_old_compose` function and returns
+        the path to it. The `kwargs` are passed to `find_old_compose`.
+        """
+        is_layered = self._compose.ci_base.release.is_layered
+        return find_old_compose(
+            self._compose.old_composes,
+            self._compose.ci_base.release.short,
+            self._compose.ci_base.release.version,
+            self._compose.ci_base.release.type_suffix,
+            self._compose.ci_base.base_product.short if is_layered else None,
+            self._compose.ci_base.base_product.version if is_layered else None,
+            **kwargs
+        )
+
+    def old_compose_path(self, path, **kwargs):
+        """
+        Translates `path` to the topdir of old compose.
+
+        :param str path: Path to translate.
+        :param kwargs: The kwargs passed to `find_old_compose` function.
+        :return: None if old compose cannot be used or if `path` does not exist
+            in the old compose topdir. Otherwise path translated to old_compose
+            topdir.
+
+        Example:
+            old_repo_dir = compose.old_compose_path(
+                compose.paths.work.pkgset_repo(pkgset.name, arch="global"))
+        """
+        old_compose_topdir = self.get_old_compose_topdir(**kwargs)
+        if not old_compose_topdir:
+            return None
+
+        rel_path = relative_path(path, self._compose.topdir.rstrip("/") + "/")
+        old_path = os.path.join(old_compose_topdir, rel_path)
+        if not os.path.exists(old_path):
+            return None
+        return old_path
+
 
 class LogPaths(object):
     def __init__(self, compose):

pungi/phases/createrepo.py

@@ -31,7 +31,7 @@ from kobo.shortcuts import run, relative_path
 from ..wrappers.scm import get_dir_from_scm
 from ..wrappers.createrepo import CreaterepoWrapper
 from .base import PhaseBase
-from ..util import find_old_compose, get_arch_variant_data, temp_dir
+from ..util import get_arch_variant_data, temp_dir
 from ..module_util import Modulemd, collect_module_defaults
 
 import productmd.rpms
@@ -367,24 +367,13 @@ def _get_old_package_dirs(compose, repo_dir):
     """
     if not compose.conf["createrepo_deltas"]:
         return None
-    old_compose_path = find_old_compose(
+    old_package_dirs = compose.paths.old_compose_path(
-        compose.old_composes,
+        repo_dir, allowed_statuses=["FINISHED", "FINISHED_INCOMPLETE"]
-        compose.ci_base.release.short,
-        compose.ci_base.release.version,
-        compose.ci_base.release.type_suffix,
-        compose.ci_base.base_product.short
-        if compose.ci_base.release.is_layered
-        else None,
-        compose.ci_base.base_product.version
-        if compose.ci_base.release.is_layered
-        else None,
-        allowed_statuses=["FINISHED", "FINISHED_INCOMPLETE"],
     )
-    if not old_compose_path:
+    if not old_package_dirs:
         compose.log_info("No suitable old compose found in: %s" % compose.old_composes)
         return None
-    rel_dir = relative_path(repo_dir, compose.topdir.rstrip("/") + "/")
+    old_package_dirs = os.path.join(old_package_dirs, "Packages")
-    old_package_dirs = os.path.join(old_compose_path, rel_dir, "Packages")
     if compose.conf["hashed_directories"]:
         old_package_dirs = _find_package_dirs(old_package_dirs)
     return old_package_dirs

pungi/phases/pkgset/common.py

@@ -17,12 +17,12 @@
 import os
 import threading
 
-from kobo.shortcuts import run, relative_path
+from kobo.shortcuts import run
 from kobo.threads import run_in_threads
 
 from pungi.arch import get_valid_arches
 from pungi.wrappers.createrepo import CreaterepoWrapper
-from pungi.util import is_arch_multilib, find_old_compose
+from pungi.util import is_arch_multilib
 from pungi.module_util import Modulemd, collect_module_defaults
 from pungi.phases.createrepo import add_modular_metadata
 
@@ -55,31 +55,16 @@ def get_create_global_repo_cmd(compose, path_prefix, repo_dir_global, pkgset):
     pkgset.save_file_cache(compose.paths.work.pkgset_file_cache(pkgset.name))
 
     # find an old compose suitable for repodata reuse
-    old_compose_path = None
     update_md_path = None
-    if compose.old_composes:
+    old_repo_dir = compose.paths.old_compose_path(
-        is_layered = compose.ci_base.release.is_layered
+        compose.paths.work.pkgset_repo(pkgset.name, arch="global")
-        old_compose_path = find_old_compose(
+    )
-            compose.old_composes,
+    if old_repo_dir:
-            compose.ci_base.release.short,
+        if os.path.isdir(os.path.join(old_repo_dir, "repodata")):
-            compose.ci_base.release.version,
+            compose.log_info("Using old repodata from: %s", old_repo_dir)
-            compose.ci_base.release.type_suffix,
+            update_md_path = old_repo_dir
-            compose.ci_base.base_product.short if is_layered else None,
+    else:
-            compose.ci_base.base_product.version if is_layered else None,
+        compose.log_info("No suitable old compose found in: %s", compose.old_composes)
-        )
-        if old_compose_path is None:
-            compose.log_info(
-                "No suitable old compose found in: %s", compose.old_composes
-            )
-        else:
-            repo_dir = compose.paths.work.pkgset_repo(pkgset.name, arch="global")
-            rel_path = relative_path(
-                repo_dir, os.path.abspath(compose.topdir).rstrip("/") + "/"
-            )
-            old_repo_dir = os.path.join(old_compose_path, rel_path)
-            if os.path.isdir(os.path.join(old_repo_dir, "repodata")):
-                compose.log_info("Using old repodata from: %s", old_repo_dir)
-                update_md_path = old_repo_dir
 
     # IMPORTANT: must not use --skip-stat here -- to make sure that correctly
     # signed files are pulled in

pungi/phases/pkgset/sources/source_koji.py

@@ -21,13 +21,13 @@ from fnmatch import fnmatch
 from itertools import groupby
 
 from kobo.rpmlib import parse_nvra
-from kobo.shortcuts import force_list, relative_path
+from kobo.shortcuts import force_list
 
 import pungi.wrappers.kojiwrapper
 from pungi.wrappers.comps import CompsWrapper
 import pungi.phases.pkgset.pkgsets
 from pungi.arch import getBaseArch
-from pungi.util import retry, find_old_compose, get_arch_variant_data
+from pungi.util import retry, get_arch_variant_data
 from pungi.module_util import Modulemd
 
 from pungi.phases.pkgset.common import MaterializedPackageSet, get_all_arches
@@ -549,34 +549,6 @@ def _get_modules_from_koji_tags(
         )
 
 
-def _find_old_file_cache_path(compose, tag_name):
-    """
-    Finds the old compose with "pkgset_file_cache.pickled" and returns
-    the path to it. If no compose is found, returns None.
-    """
-    old_compose_path = find_old_compose(
-        compose.old_composes,
-        compose.ci_base.release.short,
-        compose.ci_base.release.version,
-        compose.ci_base.release.type_suffix,
-        compose.ci_base.base_product.short
-        if compose.ci_base.release.is_layered
-        else None,
-        compose.ci_base.base_product.version
-        if compose.ci_base.release.is_layered
-        else None,
-    )
-    if not old_compose_path:
-        return None
-
-    old_file_cache_dir = compose.paths.work.pkgset_file_cache(tag_name)
-    rel_dir = relative_path(old_file_cache_dir, compose.topdir.rstrip("/") + "/")
-    old_file_cache_path = os.path.join(old_compose_path, rel_dir)
-    if not os.path.exists(old_file_cache_path):
-        return None
-    return old_file_cache_path
-
-
 def populate_global_pkgset(compose, koji_wrapper, path_prefix, event):
     all_arches = get_all_arches(compose)
 
@@ -686,7 +658,9 @@ def populate_global_pkgset(compose, koji_wrapper, path_prefix, event):
 
         # Check if we have cache for this tag from previous compose. If so, use
         # it.
-        old_cache_path = _find_old_file_cache_path(compose, compose_tag)
+        old_cache_path = compose.paths.old_compose_path(
+            compose.paths.work.pkgset_file_cache(compose_tag)
+        )
         if old_cache_path:
             pkgset.set_old_file_cache(
                 pungi.phases.pkgset.pkgsets.KojiPackageSet.load_old_file_cache(

tests/test_createrepophase.py

@@ -485,6 +485,10 @@ class TestCreateVariantRepo(PungiTestCase):
         repo = CreaterepoWrapperCls.return_value
         copy_fixture("server-rpms.json", compose.paths.compose.metadata("rpms.json"))
 
+        os.makedirs(
+            self.topdir + "/old/test-1.0-20151203.0/compose/Server/x86_64/os/Packages"
+        )
+
         create_variant_repo(
             compose, "x86_64", compose.variants["Server"], "rpm", self.pkgset
         )
@@ -538,6 +542,9 @@ class TestCreateVariantRepo(PungiTestCase):
             os.path.join(self.topdir, "old", "test-1.0-20151203.0", "STATUS"),
             "FINISHED",
         )
+        os.makedirs(
+            self.topdir + "/old/test-1.0-20151203.0/compose/Server/x86_64/os/Packages"
+        )
 
         repo = CreaterepoWrapperCls.return_value
         copy_fixture("server-rpms.json", compose.paths.compose.metadata("rpms.json"))
@@ -806,7 +813,7 @@ class TestCreateVariantRepo(PungiTestCase):
                     update=True,
                     update_md_path="/repo/x86_64",
                     deltas=True,
-                    oldpackagedirs=[],
+                    oldpackagedirs=None,
                     use_xz=False,
                     extra_args=[],
                 )