pkgset: Include module metadata in the repos

If the package set repo contains any modular package, the module
metadata is added there as well.

This is needed to accomodate change in DNF that refuses to work with
repo with modular packages if the metadata is not there. This DNF change
can cause issues in buildinstall phase.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1623128

The hybrid solver is modified to not create a separate repo with the
module metadata anymore, since it will be available in the repo with
packages. This also allows us to drop code to look into lookaside repos.

We still need to iterate over local modules in order to find out what
platform should be used.

JIRA: COMPOSE-3621
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>

pungi/paths.py

@@ -181,14 +181,6 @@ class WorkPaths(object):
         """
         return self._repo("comps", arch, variant, create_dir=create_dir)
 
-    def module_repo(self, arch=None, variant=None, create_dir=True):
-        """
-        Examples:
-            work/x86_64/module_repo_Server
-            work/global/module_repo
-        """
-        return self._repo("module", arch, variant, create_dir=create_dir)
-
     def arch_repo(self, arch=None, create_dir=True):
         """
         Examples:

pungi/phases/gather/methods/method_hybrid.py

@@ -13,7 +13,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, see <https://gnu.org/licenses/>.
 
-import gzip
 import os
 from collections import defaultdict
 from fnmatch import fnmatch
@@ -23,18 +22,15 @@ import kobo.rpmlib
 from kobo.shortcuts import run
 
 import pungi.phases.gather.method
-from pungi import Modulemd, multilib_dnf
+from pungi import multilib_dnf
 from pungi.arch import get_valid_arches, tree_arch_to_yum_arch
 from pungi.phases.gather import _mk_pkg_map
-from pungi.phases.createrepo import add_modular_metadata
 from pungi.util import (
     get_arch_variant_data,
-    iter_module_defaults,
     pkg_is_debug,
 )
 from pungi.wrappers import fus
 from pungi.wrappers.comps import CompsWrapper
-from pungi.wrappers.createrepo import CreaterepoWrapper
 
 from .method_nodeps import expand_groups
 
@@ -198,7 +194,7 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase):
             set(p.name for p in self.expand_list(multilib_whitelist)),
         )
 
-        platform = create_module_repo(self.compose, variant, arch)
+        platform = get_platform(self.compose, variant, arch)
 
         packages.update(
             expand_groups(self.compose, arch, variant, groups, set_pkg_arch=False)
@@ -227,9 +223,7 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase):
         result_modules = set()
 
         modules = []
-        if variant.arch_mmds.get(arch):
+        for mmd in variant.arch_mmds.get(arch, {}).values():
-            repos.append(self.compose.paths.work.module_repo(arch, variant))
-            for mmd in variant.arch_mmds[arch].values():
             modules.append("%s:%s" % (mmd.peek_name(), mmd.peek_stream()))
 
         input_packages = []
@@ -398,105 +392,22 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase):
         return packages
 
 
-def get_lookaside_modules(lookasides):
+def get_platform(compose, variant, arch):
-    """Get list of NSVC of all modules in all lookaside repos."""
+    """Find platform stream for modules. Raises RuntimeError if there are
-    modules = set()
+    conflicting requests.
+    """
     platforms = set()
-    for repo in lookasides:
+
-        repo = fus._prep_path(repo)
+    for var in compose.all_variants.values():
-        repomd = cr.Repomd(os.path.join(repo, "repodata/repomd.xml"))
+        for mmd in var.arch_mmds.get(arch, {}).values():
-        for rec in repomd.records:
-            if rec.type != "modules":
-                continue
-            # No with statement on Python 2.6 for GzipFile...
-            gzipped_file = gzip.GzipFile(os.path.join(repo, rec.location_href), "r")
-            # This can't use _from_stream, since gobject-introspection
-            # refuses to pass a file object.
-            mmds = Modulemd.objects_from_string(gzipped_file.read())
-            gzipped_file.close()
-            for mmd in mmds:
-                if isinstance(mmd, Modulemd.Module):
-                    modules.add(
-                        "%s:%s:%s:%s"
-                        % (
-                            mmd.peek_name(),
-                            mmd.peek_stream(),
-                            mmd.peek_version(),
-                            mmd.peek_context(),
-                        )
-                    )
             for dep in mmd.peek_dependencies():
                 streams = dep.peek_requires().get("platform")
                 if streams:
                     platforms.update(streams.dup())
-    return modules, platforms
-
-
-def create_module_repo(compose, variant, arch):
-    """Create repository with module metadata. There are no packages otherwise."""
-    createrepo_c = compose.conf["createrepo_c"]
-    createrepo_checksum = compose.conf["createrepo_checksum"]
-    msg = "Creating repo with modular metadata for %s.%s" % (variant, arch)
-
-    repo_path = compose.paths.work.module_repo(arch, variant)
-
-    compose.log_debug("[BEGIN] %s" % msg)
-
-    lookaside_modules, platforms = get_lookaside_modules(
-        pungi.phases.gather.get_lookaside_repos(compose, arch, variant)
-    )
-
-    # Add modular metadata to it
-    included = set()
-    modules = []
-
-    # We need to include metadata for all variants. The packages are in the
-    # set, so we need their metadata.
-    for var in compose.all_variants.values():
-        for mmd in var.arch_mmds.get(arch, {}).values():
-            # Set the arch field, but no other changes are needed.
-            repo_mmd = mmd.copy()
-            repo_mmd.set_arch(tree_arch_to_yum_arch(arch))
-
-            for dep in repo_mmd.peek_dependencies():
-                streams = dep.peek_requires().get("platform")
-                if streams:
-                    platforms.update(streams.dup())
-
-            nsvc = "%s:%s:%s:%s" % (
-                repo_mmd.peek_name(),
-                repo_mmd.peek_stream(),
-                repo_mmd.peek_version(),
-                repo_mmd.peek_context(),
-            )
-            if nsvc not in lookaside_modules and nsvc not in included:
-                modules.append(repo_mmd)
-                included.add(nsvc)
 
     if len(platforms) > 1:
         raise RuntimeError("There are conflicting requests for platform.")
 
-    module_names = set([x.get_name() for x in modules])
-    defaults_dir = compose.paths.work.module_defaults_dir()
-    for mmddef in iter_module_defaults(defaults_dir):
-        if mmddef.peek_module_name() in module_names:
-            modules.append(mmddef)
-
-    if modules:
-        # Initialize empty repo
-        repo = CreaterepoWrapper(createrepo_c=createrepo_c)
-        cmd = repo.get_createrepo_cmd(
-            repo_path, database=False, outputdir=repo_path, checksum=createrepo_checksum
-        )
-        logfile = "module_repo-%s" % variant
-        run(cmd, logfile=compose.paths.log.log_file(arch, logfile), show_cmd=True)
-
-        log_file = compose.paths.log.log_file(
-            arch, "gather-modifyrepo-modules-%s" % variant
-        )
-        add_modular_metadata(repo, repo_path, modules, log_file)
-
-    compose.log_debug("[DONE ] %s" % msg)
     return list(platforms)[0] if platforms else None
 
 

pungi/phases/pkgset/common.py

@@ -22,7 +22,8 @@ from kobo.threads import run_in_threads
 import pungi.phases.pkgset.pkgsets
 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, find_old_compose, iter_module_defaults
+from pungi.phases.createrepo import add_modular_metadata
 
 
 # TODO: per arch?
@@ -116,4 +117,13 @@ def _create_arch_repo(worker_thread, args, task_num):
                                   baseurl="file://%s" % path_prefix, workers=compose.conf["createrepo_num_workers"],
                                   update_md_path=repo_dir_global, checksum=createrepo_checksum)
     run(cmd, logfile=compose.paths.log.log_file(arch, "arch_repo"), show_cmd=True)
+    # Add modulemd to the repo for all modules in all variants on this architecture.
+    mmds = list(iter_module_defaults(compose.paths.work.module_defaults_dir()))
+    for variant in compose.get_variants(arch=arch):
+        mmds.extend(variant.arch_mmds.get(arch, {}).values())
+    if mmds:
+        add_modular_metadata(
+            repo, repo_dir, mmds, compose.paths.log.log_file(arch, "arch_repo_modulemd")
+        )
+
     compose.log_info("[DONE ] %s" % msg)

tests/test_gather_method_hybrid.py

@@ -28,8 +28,8 @@ class TestMethodHybrid(helpers.PungiTestCase):
     @mock.patch("pungi.phases.gather.get_lookaside_repos")
     @mock.patch("pungi.phases.gather.methods.method_hybrid.expand_groups")
     @mock.patch("pungi.phases.gather.methods.method_hybrid.expand_packages")
-    @mock.patch("pungi.phases.gather.methods.method_hybrid.create_module_repo")
+    @mock.patch("pungi.phases.gather.methods.method_hybrid.get_platform")
-    def test_call_method(self, cmr, ep, eg, glr, CW):
+    def test_call_method(self, gp, ep, eg, glr, CW):
         compose = helpers.DummyCompose(self.topdir, {})
         m = hybrid.GatherMethodHybrid(compose)
         m.run_solver = mock.Mock(return_value=(mock.Mock(), mock.Mock()))
@@ -58,7 +58,7 @@ class TestMethodHybrid(helpers.PungiTestCase):
         )
 
         self.assertEqual(res, ep.return_value)
-        self.assertEqual(cmr.call_args_list, [mock.call(compose, variant, arch)])
+        self.assertEqual(gp.call_args_list, [mock.call(compose, variant, arch)])
         self.assertEqual(
             m.run_solver.call_args_list,
             [
@@ -66,7 +66,7 @@ class TestMethodHybrid(helpers.PungiTestCase):
                     variant,
                     arch,
                     set(["pkg", "foo", "bar", ("prep", "noarch")]),
-                    cmr.return_value,
+                    gp.return_value,
                     [],
                 )
             ],
@@ -250,62 +250,26 @@ class HelperMixin(object):
         return os.path.join(self.compose.topdir, "work/x86_64/%s" % name)
 
 
-@mock.patch("pungi.phases.gather.methods.method_hybrid.add_modular_metadata")
+class TestGetPlatform(HelperMixin, helpers.PungiTestCase):
-@mock.patch("pungi.phases.gather.methods.method_hybrid.run")
-class TestCreateModuleRepo(HelperMixin, helpers.PungiTestCase):
     def setUp(self):
-        super(TestCreateModuleRepo, self).setUp()
+        super(TestGetPlatform, self).setUp()
         self.compose = helpers.DummyCompose(self.topdir, {})
         self.variant = self.compose.variants["Server"]
 
-    def test_no_modules(self, run, Modulemd):
+    def test_no_modules(self):
-        plat = hybrid.create_module_repo(self.compose, self.variant, "x86_64")
+        plat = hybrid.get_platform(self.compose, self.variant, "x86_64")
-
         self.assertIsNone(plat)
-        self.assertEqual(run.call_args_list, [])
-        self.assertEqual(Modulemd.mock_calls, [])
 
-    def test_more_than_one_platform(self, run, add_modular_metadata):
+    def test_more_than_one_platform(self):
         self.variant.arch_mmds["x86_64"] = {
             "mod:1": MockModule("mod", platform="f29"),
             "mod:2": MockModule("mod", platform="f30"),
         }
 
         with self.assertRaises(RuntimeError) as ctx:
-            hybrid.create_module_repo(self.compose, self.variant, "x86_64")
+            hybrid.get_platform(self.compose, self.variant, "x86_64")
 
         self.assertIn("conflicting requests for platform", str(ctx.exception))
-        self.assertEqual(run.call_args_list, [])
-        self.assertEqual(add_modular_metadata.mock_calls, [])
-
-    @mock.patch("pungi.phases.gather.methods.method_hybrid.iter_module_defaults")
-    def test_creating_repo_with_module_and_default(self, imd, run, add_modular_metadata):
-        mod = MockModule("mod", platform="f29")
-        self.variant.arch_mmds["x86_64"] = {"mod:1": mod}
-        default = mock.Mock(peek_module_name=mock.Mock(return_value="mod"))
-        imd.return_value = [default]
-
-        plat = hybrid.create_module_repo(self.compose, self.variant, "x86_64")
-
-        self.assertEqual(plat, "f29")
-
-        self.assertEqual(
-            add_modular_metadata.call_args_list,
-            [
-                mock.call(
-                    mock.ANY,
-                    self._repo("module_repo_Server"),
-                    [mod, default],
-                    mock.ANY,
-                ),
-            ],
-        )
-        self.assertEqual(
-            # Get first positional argument of the first call, and since it's
-            # an array, take first two elements.
-            run.call_args_list[0][0][0][:2],
-            ["createrepo_c", self._repo("module_repo_Server")]
-        )
 
 
 class ModifiedMagicMock(mock.MagicMock):
@@ -389,7 +353,7 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase):
                 mock.call(
                     self.config1,
                     "x86_64",
-                    [self._repo("repo"), self._repo("module_repo_Server")],
+                    [self._repo("repo")],
                     [],
                     platform="pl",
                     filter_packages=[("foo", None)],
@@ -445,7 +409,7 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase):
                 mock.call(
                     self.config1,
                     "x86_64",
-                    [self._repo("repo"), self._repo("module_repo_Server")],
+                    [self._repo("repo")],
                     [],
                     platform="pl",
                     filter_packages=["foo"],