From 364d7f5229dd36dd9c5f778f3575133be538cc47 Mon Sep 17 00:00:00 2001
From: Lubomír Sedlář <lsedlar@redhat.com>
Date: Feb 01 2018 13:31:09 +0000
Subject: Support multiple sources in one variant


With this patch the gather_source option is no longer used. Instead, all
sources are always used. If they return at least some input packages,
then a configured method is used and the returned lists of packages from
all sources are merged.

The method used for gathering can be configured for each variant and
gather source separately.

Additional packages are only added to the comps source.

Each gathering step is logged separately. All the logs are preserved for
later inspection.

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

---

diff --git a/doc/configuration.rst b/doc/configuration.rst
index a140831..dbe5ba0 100644
--- a/doc/configuration.rst
+++ b/doc/configuration.rst
@@ -35,7 +35,6 @@ Minimal Config Example
     createrepo_checksum = "sha256"
 
     # GATHER
-    gather_source = "comps"
     gather_method = "deps"
     greedy_method = "build"
     check_deps = False
@@ -546,16 +545,13 @@ Gather Settings
 Options
 -------
 
-**gather_source** [mandatory]
-    (*str*) -- from where to read initial package list; expected values:
-    ``comps``, ``none``, ``module``
-
-    When ``comps`` is selected, you have to specify ``comps_file`` option. When
-    ``module`` is selected, you have to :ref:`configure PDC API url <pdc-settings>`.
-
 **gather_method** [mandatory]
-    (*str*) -- Options are ``deps`` and ``nodeps``. Specifies whether package
-    dependencies should be pulled in as well.
+    (*str*|*dict*) -- Options are ``deps`` and ``nodeps``. Specifies whether
+    package dependencies should be pulled in as well. Either a single value or
+    a dictionary mapping variant UID and source type to a value. Make sure only
+    one regex matches each variant, as there is no guarantee which value will
+    be used if there are multiple matching ones. All used sources must have a
+    configured method.
 
 **gather_fulltree** = False
     (*bool*) -- When set to ``True`` all RPMs built from an SRPM will always be
@@ -671,9 +667,9 @@ Options
     such cases are still reported as warnings in the log.
 
 **gather_source_mapping**
-    (*str*) -- Only use when ``gather_source = "json"``. The value should be a
-    path to JSON file with following mapping: ``{variant: {arch: {rpm_name:
-    [rpm_arch|None]}}}``.
+    (*str*) -- JSON mapping with initial packages for the compose. The value
+    should be a path to JSON file with following mapping: ``{variant: {arch:
+    {rpm_name: [rpm_arch|None]}}}``.
 
 **gather_profiler** = False
     (*bool*) -- When set to ``True`` the gather tool will produce additional
@@ -685,12 +681,24 @@ Example
 -------
 ::
 
-    gather_source = "comps"
     gather_method = "deps"
     greedy_method = "build"
     check_deps = False
     hashed_directories = True
 
+    gather_method = {
+        "^Everything$": {
+            "comps": "deps"     # traditional content defined by comps groups
+        },
+        "^Modular$": {
+            "module": "nodeps"  # Modules do not need dependencies
+        },
+        "^Mixed$": {            # Mixed content in one variant
+            "comps": "deps",
+            "module": "nodeps"
+        }
+    }
+
     additional_packages = [
         # bz#123456
         ('^(Workstation|Server)$', {
diff --git a/doc/gathering.rst b/doc/gathering.rst
index 5111f61..08b348b 100644
--- a/doc/gathering.rst
+++ b/doc/gathering.rst
@@ -8,9 +8,9 @@ a subset of the content targeted at a particular use case.
 There are different types of variants. The type affects how packages are
 gathered into the variant.
 
-The inputs for gathering are defined by the ``gather_source`` option. It
-provides a list of package names, comps groups names and a list of packages
-that should be filtered out.
+The inputs for gathering are defined by various gather sources. Packages from
+all sources are collected to create a big list of package names, comps groups
+names and a list of packages that should be filtered out.
 
 .. note::
    The inputs for both explicit package list and comps file are interpreted as
diff --git a/pungi/checks.py b/pungi/checks.py
index faf18a2..9b317ab 100644
--- a/pungi/checks.py
+++ b/pungi/checks.py
@@ -554,12 +554,30 @@ def make_schema():
             },
 
             "gather_method": {
-                "type": "string",
-                "enum": ["deps", "nodeps"],
+                "oneOf": [
+                    {
+                        "type": "object",
+                        "patternProperties": {
+                            ".+": {
+                                "type": "object",
+                                "patternProperties": {
+                                    "^module|comps|json$": {
+                                        "type": "string",
+                                        "enum": ["deps", "nodeps"],
+                                    }
+                                }
+                            }
+                        },
+                        "additionalProperties": False,
+                    },
+                    {
+                        "type": "string",
+                        "enum": ["deps", "nodeps"],
+                    }
+                ],
             },
             "gather_source": {
-                "type": "string",
-                "enum": ["module", "json", "comps", "none"],
+                "deprecated": "remove it",
             },
             "gather_fulltree": {
                 "type": "boolean",
@@ -706,7 +724,7 @@ def make_schema():
                 "type": "string",
                 "enum": ["lorax", "buildinstall"],
             },
-            "buildinstall_topdir":  {"type": "string"},
+            "buildinstall_topdir": {"type": "string"},
             "buildinstall_kickstart": {"$ref": "#/definitions/str_or_scm_dict"},
             "buildinstall_use_guestmount": {"type": "boolean", "default": True},
 
@@ -1067,7 +1085,7 @@ def make_schema():
                      "release_is_layered",
                      "variants_file", "sigkeys",
                      "runroot", "pkgset_source",
-                     "gather_source", "gather_method"],
+                     "gather_method"],
         "additionalProperties": False,
     }
 
@@ -1118,15 +1136,6 @@ def get_num_cpus():
 # encountered and its value satisfies the lambda, an error is reported for each
 # missing (for requires) option in the list.
 CONFIG_DEPS = {
-    "gather_source": {
-        "conflicts": [
-            (lambda val: val != 'json', ['gather_source_mapping']),
-        ],
-        "requires": [
-            (lambda val: val == 'json', ['gather_source_mapping']),
-            (lambda val: val == 'comps', ['comps_file']),
-        ]
-    },
     "productimg": {
         "requires": (
             (lambda x: bool(x), ["productimg_install_class"]),
diff --git a/pungi/paths.py b/pungi/paths.py
index 42d0e3e..0e50679 100644
--- a/pungi/paths.py
+++ b/pungi/paths.py
@@ -107,32 +107,37 @@ class WorkPaths(object):
         path = os.path.join(path, file_name)
         return path
 
-    def pungi_conf(self, arch=None, variant=None, create_dir=True):
+    def pungi_conf(self, arch=None, variant=None, create_dir=True, source_name=None):
         """
         Examples:
             work/x86_64/pungi/x86_64.conf
             work/x86_64/pungi/Server.x86_64.conf
         """
         arch = arch or "global"
-        if variant is None:
-            file_name = "%s.conf" % arch
-        else:
-            file_name = "%s.%s.conf" % (variant.uid, arch)
+        file_name = ''
+        if variant:
+            file_name += variant.uid + '.'
+        file_name += arch + '.'
+        if source_name:
+            file_name += source_name + '.'
+        file_name += 'conf'
         path = os.path.join(self.topdir(arch, create_dir=create_dir), "pungi")
         if create_dir:
             makedirs(path)
         path = os.path.join(path, file_name)
         return path
 
-    def pungi_log(self, arch=None, variant=None, create_dir=True):
+    def pungi_log(self, arch=None, variant=None, create_dir=True, source_name=None):
         """
         Examples:
             work/x86_64/pungi/x86_64.log
             work/x86_64/pungi/Server.x86_64.log
         """
         path = self.pungi_conf(arch, variant, create_dir=create_dir)
-        path = path[:-5] + ".log"
-        return path
+        path = path[:-5]
+        if source_name:
+            path += '.' + source_name
+        return path + ".log"
 
     def pungi_cache_dir(self, arch, variant=None, create_dir=True):
         """
diff --git a/pungi/phases/gather/__init__.py b/pungi/phases/gather/__init__.py
index 23b9079..bc056e2 100644
--- a/pungi/phases/gather/__init__.py
+++ b/pungi/phases/gather/__init__.py
@@ -24,7 +24,7 @@ from productmd.rpms import Rpms
 from pungi.wrappers.scm import get_file_from_scm
 from .link import link_files
 
-from pungi.util import get_arch_variant_data, get_arch_data
+from pungi.util import get_arch_variant_data, get_arch_data, get_variant_data
 from pungi.phases.base import PhaseBase
 from pungi.arch import split_name_arch, get_compatible_arches
 
@@ -68,10 +68,13 @@ class GatherPhase(PhaseBase):
         except ValueError as exc:
             errors = exc.message.split('\n')
 
-        if self.compose.conf['gather_source'] == 'module':
-            from pungi.phases.pkgset.sources import source_koji
-            if not source_koji.WITH_MODULES:
-                errors.append('Modular compose requires pdc_client and modulemd packages.')
+        # This must be imported here to avoid circular deps problems.
+        from pungi.phases.pkgset.sources import source_koji
+        if not source_koji.WITH_MODULES:
+            # Modules are not supported, check if we need them
+            for variant in self.compose.variants.values():
+                if variant.modules:
+                    errors.append('Modular compose requires pdc_client and modulemd packages.')
 
         if errors:
             raise ValueError('\n'.join(errors))
@@ -126,7 +129,14 @@ def gather_packages(compose, arch, variant, package_sets, fulltree_excludes=None
     # multilib white/black-list is per-arch, common for all variants
     multilib_whitelist = get_multilib_whitelist(compose, arch)
     multilib_blacklist = get_multilib_blacklist(compose, arch)
-    GatherMethod = get_gather_method(compose.conf["gather_method"])
+    methods = compose.conf["gather_method"]
+    global_method_name = methods
+    if isinstance(methods, dict):
+        try:
+            methods = get_variant_data(compose.conf, 'gather_method', variant)[-1]
+            global_method_name = None
+        except IndexError:
+            raise RuntimeError("Variant %s has no configured gather_method" % variant.uid)
 
     msg = "Gathering packages (arch: %s, variant: %s)" % (arch, variant)
 
@@ -136,17 +146,43 @@ def gather_packages(compose, arch, variant, package_sets, fulltree_excludes=None
 
     compose.log_info("[BEGIN] %s" % msg)
 
-    packages, groups, filter_packages = get_variant_packages(compose, arch, variant, package_sets)
+    result = {
+        "rpm": [],
+        "srpm": [],
+        "debuginfo": [],
+    }
+
     prepopulate = get_prepopulate_packages(compose, arch, variant)
     fulltree_excludes = fulltree_excludes or set()
 
-    method = GatherMethod(compose)
-    pkg_map = method(arch, variant, packages, groups, filter_packages,
-                     multilib_whitelist, multilib_blacklist, package_sets,
-                     fulltree_excludes=fulltree_excludes, prepopulate=prepopulate)
+    for source_name in ('module', 'comps', 'json'):
+
+        packages, groups, filter_packages = get_variant_packages(compose, arch, variant,
+                                                                 source_name, package_sets)
+        if not packages and not groups:
+            # No inputs, nothing to do really.
+            continue
+
+        try:
+            method_name = global_method_name or methods[source_name]
+        except KeyError:
+            raise RuntimeError("Variant %s has no configured gather_method for source %s"
+                               % (variant.uid, source_name))
+
+        GatherMethod = get_gather_method(method_name)
+        method = GatherMethod(compose)
+        method.source_name = source_name
+        compose.log_debug("Gathering source %s, method %s" % (source_name, method_name))
+        pkg_map = method(arch, variant, packages, groups, filter_packages,
+                         multilib_whitelist, multilib_blacklist, package_sets,
+                         fulltree_excludes=fulltree_excludes,
+                         prepopulate=prepopulate if source_name == 'comps' else set())
+
+        for t in ('rpm', 'srpm', 'debuginfo'):
+            result[t].extend(pkg_map.get(t, []))
 
     compose.log_info("[DONE ] %s" % msg)
-    return pkg_map
+    return result
 
 
 def write_packages(compose, arch, variant, pkg_map, path_prefix):
@@ -415,7 +451,7 @@ def get_lookaside_repos(compose, arch, variant):
     return get_arch_variant_data(compose.conf, "gather_lookaside_repos", arch, variant)
 
 
-def get_variant_packages(compose, arch, variant, package_sets=None):
+def get_variant_packages(compose, arch, variant, source_name, package_sets=None):
     """Find inputs for depsolving of variant.arch combination.
 
     Returns a triple: a list of input packages, a list of input comps groups
@@ -429,17 +465,27 @@ def get_variant_packages(compose, arch, variant, package_sets=None):
     When system-release packages should be filtered, the ``package_sets``
     argument is required.
     """
-    GatherSource = get_gather_source(compose.conf["gather_source"])
+    packages, groups, filter_packages = set(), set(), set()
+    GatherSource = get_gather_source(source_name)
     source = GatherSource(compose)
-    packages, groups = source(arch, variant)
-    filter_packages = set()
+    p, g = source(arch, variant)
+
+    if source_name != "comps" and not p and not g:
+        # For modules and json source, if the source did not return anything,
+        # we should skip all further work. Additional packages and possibly
+        # system-release will be added to comps source.
+        return packages, groups, filter_packages
+
+    packages |= p
+    groups |= g
 
     if variant is None:
         # no variant -> no parent -> we have everything we need
         # doesn't make sense to do any package filtering
         return packages, groups, filter_packages
 
-    packages |= get_additional_packages(compose, arch, variant)
+    if source_name == 'comps':
+        packages |= get_additional_packages(compose, arch, variant)
     filter_packages |= get_filter_packages(compose, arch, variant)
 
     if compose.conf['filter_system_release_packages']:
@@ -452,13 +498,13 @@ def get_variant_packages(compose, arch, variant, package_sets=None):
         for var in variant.parent.get_variants(
                 arch=arch, types=["self", "variant", "addon", "layered-product"]):
             var_packages, var_groups, _ = get_variant_packages(
-                compose, arch, var, package_sets=package_sets)
+                compose, arch, var, source_name, package_sets=package_sets)
             packages |= var_packages
             groups |= var_groups
 
     if variant.type in ["addon", "layered-product"]:
         var_packages, var_groups, _ = get_variant_packages(
-            compose, arch, variant.parent, package_sets=package_sets)
+            compose, arch, variant.parent, source_name, package_sets=package_sets)
         packages |= var_packages
         groups |= var_groups
 
@@ -517,9 +563,6 @@ def get_packages_to_gather(compose, arch=None, variant=None, include_arch=True,
     """
     Returns the list of names of packages and list of names of groups which
     would be included in a compose as GATHER phase result.
-    This works only for "comps" or "json" gather_source. For "module"
-    gather_source, this always return an empty list, because it is not clear
-    what packages will end up in a compose before the gather phase is run.
 
     :param str arch: Arch to return packages for. If not set, returns packages
         for all arches.
@@ -531,30 +574,28 @@ def get_packages_to_gather(compose, arch=None, variant=None, include_arch=True,
     :param include_prepopulated: When True, the prepopulated packages will
         be included in a list of packages.
     """
-    if compose.conf["gather_source"] == "module":
-        return ([], [])
+    packages = set([])
+    groups = set([])
+    for source_name in ('module', 'comps', 'json'):
+        GatherSource = get_gather_source(source_name)
+        src = GatherSource(compose)
 
-    arches = [arch] if arch else compose.get_arches()
+        arches = [arch] if arch else compose.get_arches()
 
-    GatherSource = get_gather_source(compose.conf["gather_source"])
-    src = GatherSource(compose)
+        for arch in arches:
+            pkgs, grps = src(arch, variant)
+            groups = groups.union(set(grps))
 
-    packages = set([])
-    groups = set([])
-    for arch in arches:
-        pkgs, grps = src(arch, variant)
-        groups = groups.union(set(grps))
-
-        additional_packages = get_additional_packages(compose, arch, None)
-        for pkg_name, pkg_arch in pkgs | additional_packages:
-            if not include_arch or pkg_arch is None:
-                packages.add(pkg_name)
-            else:
-                packages.add("%s.%s" % (pkg_name, pkg_arch))
+            additional_packages = get_additional_packages(compose, arch, None)
+            for pkg_name, pkg_arch in pkgs | additional_packages:
+                if not include_arch or pkg_arch is None:
+                    packages.add(pkg_name)
+                else:
+                    packages.add("%s.%s" % (pkg_name, pkg_arch))
 
-        if include_prepopulated:
-            prepopulated = get_prepopulate_packages(
-                compose, arch, variant, include_arch)
-            packages = packages.union(prepopulated)
+            if include_prepopulated:
+                prepopulated = get_prepopulate_packages(
+                    compose, arch, variant, include_arch)
+                packages = packages.union(prepopulated)
 
     return list(packages), list(groups)
diff --git a/pungi/phases/gather/methods/method_deps.py b/pungi/phases/gather/methods/method_deps.py
index f4d0d6f..7c9e8fb 100644
--- a/pungi/phases/gather/methods/method_deps.py
+++ b/pungi/phases/gather/methods/method_deps.py
@@ -40,8 +40,9 @@ class GatherMethodDeps(pungi.phases.gather.method.GatherMethodBase):
 
         write_pungi_config(self.compose, arch, variant, packages, groups, filter_packages,
                            multilib_whitelist, multilib_blacklist,
-                           fulltree_excludes=fulltree_excludes, prepopulate=prepopulate)
-        result, missing_deps = resolve_deps(self.compose, arch, variant)
+                           fulltree_excludes=fulltree_excludes, prepopulate=prepopulate,
+                           source_name=self.source_name)
+        result, missing_deps = resolve_deps(self.compose, arch, variant, source_name=self.source_name)
         check_deps(self.compose, arch, variant, missing_deps)
         return result
 
@@ -61,10 +62,10 @@ def _format_packages(pkgs):
 
 def write_pungi_config(compose, arch, variant, packages, groups, filter_packages,
                        multilib_whitelist, multilib_blacklist, fulltree_excludes=None,
-                       prepopulate=None):
+                       prepopulate=None, source_name=None):
     """write pungi config (kickstart) for arch/variant"""
     pungi_wrapper = PungiWrapper()
-    pungi_cfg = compose.paths.work.pungi_conf(variant=variant, arch=arch)
+    pungi_cfg = compose.paths.work.pungi_conf(variant=variant, arch=arch, source_name=source_name)
     msg = "Writing pungi config (arch: %s, variant: %s): %s" % (arch, variant, pungi_cfg)
 
     if compose.DEBUG and os.path.isfile(pungi_cfg):
@@ -95,9 +96,9 @@ def write_pungi_config(compose, arch, variant, packages, groups, filter_packages
         prepopulate=prepopulate)
 
 
-def resolve_deps(compose, arch, variant):
+def resolve_deps(compose, arch, variant, source_name=None):
     pungi_wrapper = PungiWrapper()
-    pungi_log = compose.paths.work.pungi_log(arch, variant)
+    pungi_log = compose.paths.work.pungi_log(arch, variant, source_name=source_name)
 
     msg = "Running pungi (arch: %s, variant: %s)" % (arch, variant)
     if compose.DEBUG and os.path.exists(pungi_log):
@@ -107,7 +108,7 @@ def resolve_deps(compose, arch, variant):
             return res, broken_deps
 
     compose.log_info("[BEGIN] %s" % msg)
-    pungi_conf = compose.paths.work.pungi_conf(arch, variant)
+    pungi_conf = compose.paths.work.pungi_conf(arch, variant, source_name=source_name)
 
     multilib_methods = get_arch_variant_data(compose.conf, 'multilib', arch, variant)
 
diff --git a/pungi/phases/gather/methods/method_nodeps.py b/pungi/phases/gather/methods/method_nodeps.py
index 69249da..ffc8e46 100644
--- a/pungi/phases/gather/methods/method_nodeps.py
+++ b/pungi/phases/gather/methods/method_nodeps.py
@@ -28,7 +28,10 @@ class GatherMethodNodeps(pungi.phases.gather.method.GatherMethodBase):
     enabled = True
 
     def __call__(self, arch, variant, *args, **kwargs):
-        log_file = self.compose.paths.log.log_file(arch, 'gather-nodeps-%s' % variant.uid)
+        fname = 'gather-nodeps-%s' % variant.uid
+        if self.source_name:
+            fname += '-' + self.source_name
+        log_file = self.compose.paths.log.log_file(arch, fname)
         with open(log_file, 'w') as log:
             return self.worker(log, arch, variant, *args, **kwargs)
 
diff --git a/tests/data/dummy-pungi.conf b/tests/data/dummy-pungi.conf
index faaebb5..2d1f21f 100644
--- a/tests/data/dummy-pungi.conf
+++ b/tests/data/dummy-pungi.conf
@@ -36,8 +36,12 @@ createrepo_checksum = "sha256"
 
 
 # GATHER
-gather_source = "comps"
-gather_method = "deps"
+gather_method = {
+    "^.*$": {
+        "module": "nodeps",
+        "comps": "deps",
+    }
+}
 greedy_method = "build"
 check_deps = False
 hashed_directories = True
diff --git a/tests/helpers.py b/tests/helpers.py
index 3f258ba..f069635 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -186,7 +186,6 @@ BASE_CONFIG = dict(
     runroot=False,
     createrepo_checksum='sha256',
     gather_method='deps',
-    gather_source='none',
     sigkeys=[],
 )
 
diff --git a/tests/test_config.py b/tests/test_config.py
index 5cfdcbb..b2c1c8f 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -213,31 +213,6 @@ class CreaterepoConfigTestCase(ConfigTestCase):
 
 
 class GatherConfigTestCase(ConfigTestCase):
-    def test_source_comps_requires_comps(self):
-        cfg = load_config(
-            pkgset_source='koji',
-            pkgset_koji_tag="f25",
-            gather_source='comps',
-            gather_source_mapping='foo'
-        )
-
-        self.assertValidation(
-            cfg,
-            [checks.REQUIRES.format('gather_source', 'comps', 'comps_file'),
-             checks.CONFLICTS.format('gather_source', 'comps', 'gather_source_mapping')])
-
-    def test_source_json_requires_mapping(self):
-        cfg = load_config(
-            pkgset_source='koji',
-            pkgset_koji_tag="f25",
-            gather_source='json',
-            comps_file='comps',
-        )
-
-        self.assertValidation(
-            cfg,
-            [checks.REQUIRES.format('gather_source', 'json', 'gather_source_mapping')])
-
     def test_dnf_backend_is_default_on_py3(self):
         cfg = load_config(
             pkgset_source='koji',
diff --git a/tests/test_gather_phase.py b/tests/test_gather_phase.py
index 28598fa..1630ef3 100644
--- a/tests/test_gather_phase.py
+++ b/tests/test_gather_phase.py
@@ -460,7 +460,7 @@ class TestGetVariantPackages(helpers.PungiTestCase):
     def test_no_variant(self):
         compose = helpers.DummyCompose(self.topdir, {})
         packages, groups, filter_packages = gather.get_variant_packages(
-            compose, 'x86_64', None)
+            compose, 'x86_64', None, 'comps')
         self.assertItemsEqual(packages, [])
         self.assertItemsEqual(groups, [])
         self.assertItemsEqual(filter_packages, [])
@@ -473,7 +473,7 @@ class TestGetVariantPackages(helpers.PungiTestCase):
         compose = helpers.DummyCompose(self.topdir, {})
 
         packages, groups, filter_packages = gather.get_variant_packages(
-            compose, 'x86_64', compose.variants['Server'])
+            compose, 'x86_64', compose.variants['Server'], 'comps')
         self.assertItemsEqual(packages, ['foo'])
         self.assertItemsEqual(groups, ['core'])
         self.assertItemsEqual(filter_packages, [])
@@ -490,7 +490,7 @@ class TestGetVariantPackages(helpers.PungiTestCase):
         )
 
         packages, groups, filter_packages = gather.get_variant_packages(
-            compose, 'x86_64', compose.variants['Server'], package_sets={'x86_64': pkgset})
+            compose, 'x86_64', compose.variants['Server'], 'comps', package_sets={'x86_64': pkgset})
         self.assertItemsEqual(packages, [('system-release-server', None)])
         self.assertItemsEqual(groups, [])
         self.assertItemsEqual(filter_packages, [('system-release', None)])
@@ -509,7 +509,7 @@ class TestGetVariantPackages(helpers.PungiTestCase):
         )
 
         packages, groups, filter_packages = gather.get_variant_packages(
-            compose, 'x86_64', compose.variants['Server'], package_sets={'x86_64': pkgset})
+            compose, 'x86_64', compose.variants['Server'], 'comps', package_sets={'x86_64': pkgset})
         self.assertItemsEqual(packages, [])
         self.assertItemsEqual(groups, [])
         self.assertItemsEqual(filter_packages, [])
@@ -534,7 +534,7 @@ class TestGetVariantPackages(helpers.PungiTestCase):
         )
 
         packages, groups, filter_packages = gather.get_variant_packages(
-            compose, 'x86_64', compose.all_variants['Server-optional'])
+            compose, 'x86_64', compose.all_variants['Server-optional'], 'comps')
         self.assertItemsEqual(packages, ['server-pkg', 'addon-pkg', 'opt-pkg'])
         self.assertItemsEqual(groups, ['server-group', 'addon-group', 'opt-group'])
         self.assertItemsEqual(filter_packages, [])
@@ -554,7 +554,7 @@ class TestGetVariantPackages(helpers.PungiTestCase):
         )
 
         packages, groups, filter_packages = gather.get_variant_packages(
-            compose, 'x86_64', compose.all_variants['Server-optional'])
+            compose, 'x86_64', compose.all_variants['Server-optional'], 'comps')
         self.assertItemsEqual(packages, [])
         self.assertItemsEqual(groups, [])
         self.assertItemsEqual(filter_packages, [])
@@ -572,7 +572,7 @@ class TestGetVariantPackages(helpers.PungiTestCase):
         )
 
         packages, groups, filter_packages = gather.get_variant_packages(
-            compose, 'x86_64', compose.all_variants['Server'])
+            compose, 'x86_64', compose.all_variants['Server'], 'comps')
         self.assertItemsEqual(packages, [('pkg', None), ('foo', 'x86_64')])
         self.assertItemsEqual(groups, [])
         self.assertItemsEqual(filter_packages, [])
@@ -591,7 +591,7 @@ class TestGetVariantPackages(helpers.PungiTestCase):
 
         with self.assertRaises(ValueError) as ctx:
             packages, groups, filter_packages = gather.get_variant_packages(
-                compose, 'x86_64', compose.all_variants['Server'])
+                compose, 'x86_64', compose.all_variants['Server'], 'comps')
 
         self.assertIn('Incompatible package arch', str(ctx.exception))
 
@@ -641,17 +641,19 @@ class TestGatherPackages(helpers.PungiTestCase):
         pkg_set = mock.Mock()
         self.assertEqual(
             gather.gather_packages(compose, 'x86_64', compose.variants['Server'], pkg_set),
-            get_gather_method.return_value.return_value.return_value
+            {'rpm': [], 'srpm': [], 'debuginfo': []}
         )
         self.assertEqual(get_gather_method.call_args_list,
-                         [mock.call(compose.conf['gather_method'])])
+                         [mock.call(compose.conf['gather_method'])] * 3)
         self.assertEqual(get_variant_packages.call_args_list,
-                         [mock.call(compose, 'x86_64', compose.variants['Server'], pkg_set)])
+                         [mock.call(compose, 'x86_64', compose.variants['Server'], 'module', pkg_set),
+                          mock.call(compose, 'x86_64', compose.variants['Server'], 'comps', pkg_set),
+                          mock.call(compose, 'x86_64', compose.variants['Server'], 'json', pkg_set)])
         self.assertEqual(
             get_gather_method.return_value.return_value.call_args_list,
             [mock.call('x86_64', compose.variants['Server'], packages, groups,
                        filters, set(), set(), pkg_set, fulltree_excludes=set(),
-                       prepopulate=set())]
+                       prepopulate=set())] * 3
         )
 
     @mock.patch('pungi.phases.gather.get_variant_packages')
@@ -679,19 +681,36 @@ class TestGatherPackages(helpers.PungiTestCase):
         pkg_set = mock.Mock()
         self.assertEqual(
             gather.gather_packages(compose, 'x86_64', compose.variants['Server'], pkg_set),
-            get_gather_method.return_value.return_value.return_value
+            {'rpm': [], 'srpm': [], 'debuginfo': []}
         )
         self.assertEqual(get_gather_method.call_args_list,
-                         [mock.call(compose.conf['gather_method'])])
+                         [mock.call(compose.conf['gather_method'])] * 3)
         self.assertEqual(get_variant_packages.call_args_list,
-                         [mock.call(compose, 'x86_64', compose.variants['Server'], pkg_set)])
+                         [mock.call(compose, 'x86_64', compose.variants['Server'], 'module', pkg_set),
+                          mock.call(compose, 'x86_64', compose.variants['Server'], 'comps', pkg_set),
+                          mock.call(compose, 'x86_64', compose.variants['Server'], 'json', pkg_set)])
         self.assertEqual(
             get_gather_method.return_value.return_value.call_args_list,
             [mock.call('x86_64', compose.variants['Server'], packages, groups,
                        filters, set(['white']), set(['black']), pkg_set,
-                       fulltree_excludes=set(), prepopulate=set())]
+                       fulltree_excludes=set(), prepopulate=set())] * 3
         )
 
+    @mock.patch('pungi.phases.gather.get_variant_packages')
+    @mock.patch('pungi.phases.gather.get_gather_method')
+    def test_per_source_method(self, get_gather_method, get_variant_packages):
+        packages, groups, filters = mock.Mock(), mock.Mock(), mock.Mock()
+        get_variant_packages.return_value = (packages, groups, filters)
+        compose = helpers.DummyCompose(self.topdir, {
+            'multilib_whitelist': {'*': ['white']},
+            'multilib_blacklist': {'*': ['black']},
+            'gather_method': {'^Server$': {'comps': 'deps', 'module': 'nodeps', 'json': 'deps'}},
+        })
+        pkg_set = mock.Mock()
+        gather.gather_packages(compose, 'x86_64', compose.variants['Server'], pkg_set),
+        self.assertEqual(get_gather_method.call_args_list,
+                         [mock.call('nodeps'), mock.call('deps'), mock.call('deps')])
+
 
 class TestWritePrepopulate(helpers.PungiTestCase):
     def test_without_config(self):
diff --git a/tests/test_pkgset_source_koji.py b/tests/test_pkgset_source_koji.py
index 6ea1716..b1e1308 100644
--- a/tests/test_pkgset_source_koji.py
+++ b/tests/test_pkgset_source_koji.py
@@ -204,7 +204,6 @@ class TestPopulateGlobalPkgset(helpers.PungiTestCase):
                                          pickle_dumps):
         self.compose = helpers.DummyCompose(self.topdir, {
             'gather_method': 'nodeps',
-            'gather_source': 'none',
             'pkgset_koji_tag': 'f25',
             'sigkeys': mock.Mock(),
             'additional_packages': [