diff --git a/pungi/phases/base.py b/pungi/phases/base.py index bdfd7af6..42fc2613 100644 --- a/pungi/phases/base.py +++ b/pungi/phases/base.py @@ -26,6 +26,10 @@ class PhaseBase(object): self.finished = False self._skipped = False + # A set of config patterns that were actually used. Starts as None, and + # when config is queried the variable turns into a set of patterns. + self.used_patterns = None + def validate(self): pass @@ -59,6 +63,41 @@ class PhaseBase(object): self.compose.notifier.send('phase-start', phase_name=self.name) self.run() + def get_config_block(self, variant, arch=None): + """In config for current phase, find a block corresponding to given + variant and arch. The arch should be given if and only if the config + uses variant/arch mapping. + """ + self.used_patterns = self.used_patterns or set() + if arch is not None: + return util.get_arch_variant_data(self.compose.conf, self.name, + arch, variant, keys=self.used_patterns) + else: + return util.get_variant_data(self.compose.conf, self.name, + variant, keys=self.used_patterns) + + def get_all_patterns(self): + """Get all variant patterns from config file for this phase.""" + if isinstance(self.compose.conf.get(self.name), dict): + return set(self.compose.conf.get(self.name, {}).keys()) + else: + return set(x[0] for x in self.compose.conf.get(self.name, [])) + + def report_unused_patterns(self): + """Log warning about unused parts of the config. + + This is not technically an error, but can help debug when something + expected is missing. + """ + all_patterns = self.get_all_patterns() + unused_patterns = all_patterns - self.used_patterns + if unused_patterns: + self.compose.log_warning( + '[%s] Patterns in config do not match any variant: %s' + % (self.name.upper(), ', '.join(sorted(unused_patterns)))) + self.compose.log_info( + 'Note that variants can be excluded in configuration file') + def stop(self): if self.finished: return @@ -66,6 +105,9 @@ class PhaseBase(object): self.pool.stop() self.finished = True self.compose.log_info("[DONE ] %s" % self.msg) + if self.used_patterns is not None: + # We only want to report this if the config was actually queried. + self.report_unused_patterns() self.compose.notifier.send('phase-stop', phase_name=self.name) def run(self): diff --git a/pungi/phases/image_build.py b/pungi/phases/image_build.py index 5dd7ce5c..31350ec0 100644 --- a/pungi/phases/image_build.py +++ b/pungi/phases/image_build.py @@ -5,7 +5,7 @@ import os import time from kobo import shortcuts -from pungi.util import get_variant_data, makedirs, get_mtime, get_file_size, failable +from pungi.util import makedirs, get_mtime, get_file_size, failable from pungi.util import translate_path, get_repo_urls, version_generator from pungi.phases import base from pungi.linker import Linker @@ -74,7 +74,7 @@ class ImageBuildPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG for variant in self.compose.get_variants(): arches = set([x for x in variant.arches if x != 'src']) - for image_conf in get_variant_data(self.compose.conf, self.name, variant): + for image_conf in self.get_config_block(variant): # We will modify the data, so we need to make a copy to # prevent problems in next iteration where the original # value is needed. diff --git a/pungi/phases/live_images.py b/pungi/phases/live_images.py index f617f966..470433b4 100644 --- a/pungi/phases/live_images.py +++ b/pungi/phases/live_images.py @@ -27,7 +27,7 @@ from productmd.images import Image from pungi.wrappers.kojiwrapper import KojiWrapper from pungi.wrappers import iso from pungi.phases import base -from pungi.util import get_arch_variant_data, makedirs, get_mtime, get_file_size, failable +from pungi.util import makedirs, get_mtime, get_file_size, failable from pungi.util import get_repo_urls @@ -57,7 +57,7 @@ class LiveImagesPhase(base.PhaseLoggerMixin, base.ImageConfigMixin, base.ConfigG for variant in self.compose.all_variants.values(): for arch in variant.arches + ["src"]: - for data in get_arch_variant_data(self.compose.conf, self.name, arch, variant): + for data in self.get_config_block(variant, arch): subvariant = data.get('subvariant', variant.uid) type = data.get('type', 'live') diff --git a/pungi/phases/livemedia_phase.py b/pungi/phases/livemedia_phase.py index 5c273d1d..a73cf965 100644 --- a/pungi/phases/livemedia_phase.py +++ b/pungi/phases/livemedia_phase.py @@ -4,7 +4,7 @@ import os import time from kobo import shortcuts -from pungi.util import get_variant_data, makedirs, get_mtime, get_file_size, failable +from pungi.util import makedirs, get_mtime, get_file_size, failable from pungi.util import translate_path, get_repo_urls from pungi.phases.base import ConfigGuardedPhase, ImageConfigMixin, PhaseLoggerMixin from pungi.linker import Linker @@ -56,7 +56,7 @@ class LiveMediaPhase(PhaseLoggerMixin, ImageConfigMixin, ConfigGuardedPhase): def run(self): for variant in self.compose.get_variants(): arches = set([x for x in variant.arches if x != 'src']) - for image_conf in get_variant_data(self.compose.conf, self.name, variant): + for image_conf in self.get_config_block(variant): subvariant = image_conf.get('subvariant', variant.uid) name = image_conf.get( 'name', "%s-%s-Live" % (self.compose.ci_base.release.short, subvariant)) diff --git a/pungi/phases/osbs.py b/pungi/phases/osbs.py index 71a312bc..10785d4a 100644 --- a/pungi/phases/osbs.py +++ b/pungi/phases/osbs.py @@ -20,7 +20,7 @@ class OSBSPhase(PhaseLoggerMixin, ConfigGuardedPhase): def run(self): for variant in self.compose.get_variants(): - for conf in util.get_variant_data(self.compose.conf, self.name, variant): + for conf in self.get_config_block(variant): self.pool.add(OSBSThread(self.pool)) self.pool.queue_put((self.compose, variant, conf)) diff --git a/pungi/phases/ostree.py b/pungi/phases/ostree.py index 1a4783af..c7575319 100644 --- a/pungi/phases/ostree.py +++ b/pungi/phases/ostree.py @@ -27,14 +27,14 @@ class OSTreePhase(ConfigGuardedPhase): def run(self): if isinstance(self.compose.conf.get(self.name), dict): for variant in self.compose.get_variants(): - for conf in util.get_variant_data(self.compose.conf, self.name, variant): + for conf in self.get_config_block(variant): for arch in conf.get('arches', []) or variant.arches: self._enqueue(variant, arch, conf) else: # Legacy code path to support original configuration. for variant in self.compose.get_variants(): for arch in variant.arches: - for conf in util.get_arch_variant_data(self.compose.conf, self.name, arch, variant): + for conf in self.get_config_block(variant, arch): self._enqueue(variant, arch, conf) self.pool.start() diff --git a/pungi/phases/ostree_installer.py b/pungi/phases/ostree_installer.py index e6f43bbd..46633cc1 100644 --- a/pungi/phases/ostree_installer.py +++ b/pungi/phases/ostree_installer.py @@ -23,7 +23,7 @@ class OstreeInstallerPhase(PhaseLoggerMixin, ConfigGuardedPhase): def run(self): for variant in self.compose.get_variants(): for arch in variant.arches: - for conf in util.get_arch_variant_data(self.compose.conf, self.name, arch, variant): + for conf in self.get_config_block(variant, arch): self.pool.add(OstreeInstallerThread(self.pool)) self.pool.queue_put((self.compose, variant, arch, conf)) diff --git a/pungi/util.py b/pungi/util.py index e5d509bc..d25eff71 100644 --- a/pungi/util.py +++ b/pungi/util.py @@ -208,7 +208,7 @@ def pkg_is_debug(pkg_obj): # fomat: [(variant_uid_regex, {arch|*: [data]})] -def get_arch_variant_data(conf, var_name, arch, variant): +def get_arch_variant_data(conf, var_name, arch, variant, keys=None): result = [] for conf_variant, conf_data in conf.get(var_name, []): if variant is not None and not re.match(conf_variant, variant.uid): @@ -219,6 +219,8 @@ def get_arch_variant_data(conf, var_name, arch, variant): if conf_arch == "*" and arch == "src": # src is excluded from '*' and needs to be explicitly added to the mapping continue + if keys is not None: + keys.add(conf_variant) if isinstance(conf_data[conf_arch], list): result.extend(conf_data[conf_arch]) else: @@ -295,7 +297,7 @@ def get_arch_data(conf, var_name, arch): return result -def get_variant_data(conf, var_name, variant): +def get_variant_data(conf, var_name, variant, keys=None): """Get configuration for variant. Expected config format is a mapping from variant_uid regexes to lists of @@ -303,12 +305,15 @@ def get_variant_data(conf, var_name, variant): :param var_name: name of configuration key with which to work :param variant: Variant object for which to get configuration + :param keys: A set to which a used pattern from config will be added (optional) :rtype: a list of values """ result = [] for conf_variant, conf_data in conf.get(var_name, {}).iteritems(): if not re.match(conf_variant, variant.uid): continue + if keys is not None: + keys.add(conf_variant) if isinstance(conf_data, list): result.extend(conf_data) else: diff --git a/tests/test_util.py b/tests/test_util.py index 028fceee..aa1e6caa 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -138,6 +138,18 @@ class TestGetVariantData(unittest.TestCase): result = util.get_variant_data({}, 'foo', mock.Mock(uid='Client')) self.assertItemsEqual(result, []) + def test_get_save_pattern(self): + conf = { + 'foo': { + '^Client$': 1, + '^NotClient$': 2, + } + } + patterns = set() + result = util.get_variant_data(conf, 'foo', mock.Mock(uid='Client'), keys=patterns) + self.assertEqual(result, [1]) + self.assertEqual(patterns, set(['^Client$'])) + class TestVolumeIdGenerator(unittest.TestCase): def setUp(self):