PR#586: Report better errors when gather config is wrong - pungi

pungi

#586 Report better errors when gather config is wrong

Merged 8 years ago by lsedlar. Opened 8 years ago by lsedlar.

lsedlar/pungi comps-errors into master

gather: Clean up method deps

Lubomír Sedlář • 8 years ago

dcc1750

gather: Report error if there is no input

Lubomír Sedlář • 8 years ago

0168388

init: Warn when variants mentions non-existing comps group

Lubomír Sedlář • 8 years ago

b5efb67

pungi/phases/gather/methods/method_deps.py

file modified

+28 -18

		`@@ -37,13 +37,26 @@`
		`# "debuginfo": [],`
		`# }`

		`- write_pungi_config(self.compose, arch, variant, packages, groups, filter_packages, multilib_whitelist, multilib_blacklist, package_set=package_sets[arch], fulltree_excludes=fulltree_excludes, prepopulate=prepopulate)`
		`+ write_pungi_config(self.compose, arch, variant, packages, groups, filter_packages,`
		`+ multilib_whitelist, multilib_blacklist,`
		`+ fulltree_excludes=fulltree_excludes, prepopulate=prepopulate)`
		`result = resolve_deps(self.compose, arch, variant)`
		`check_deps(self.compose, arch, variant)`
		`return result`


		`- def write_pungi_config(compose, arch, variant, packages, groups, filter_packages, multilib_whitelist, multilib_blacklist, repos=None, comps_repo=None, package_set=None, fulltree_excludes=None, prepopulate=None):`
		`+ def _format_packages(pkgs):`
		`+ """Sort packages and merge name with arch."""`
		`+ for pkg_name, pkg_arch in sorted(pkgs):`
		`+ if pkg_arch:`
		`+ yield '%s.%s' % (pkg_name, pkg_arch)`
		`+ else:`
		`+ yield pkg_name`
		`+`
		`+`
		`+ def write_pungi_config(compose, arch, variant, packages, groups, filter_packages,`
		`+ multilib_whitelist, multilib_blacklist, fulltree_excludes=None,`
		`+ prepopulate=None):`
		`"""write pungi config (kickstart) for arch/variant"""`
		`pungi_wrapper = PungiWrapper()`
		`pungi_cfg = compose.paths.work.pungi_conf(variant=variant, arch=arch)`
		`@@ -55,29 +68,26 @@`

		`compose.log_info(msg)`

		`- if not repos:`
		`- repo_path = compose.paths.work.arch_repo(arch=arch)`
		`- repos = {"pungi-repo": repo_path}`
		`+ repos = {"pungi-repo": compose.paths.work.arch_repo(arch=arch)}`

		`lookaside_repos = {}`
		`for i, repo_url in enumerate(pungi.phases.gather.get_lookaside_repos(compose, arch, variant)):`
		`lookaside_repos["lookaside-repo-%s" % i] = repo_url`

		`- packages_str = []`
		`- for pkg_name, pkg_arch in sorted(packages):`
		`- if pkg_arch:`
		`- packages_str.append("%s.%s" % (pkg_name, pkg_arch))`
		`- else:`
		`- packages_str.append(pkg_name)`
		`+ packages_str = list(_format_packages(packages))`
		`+ filter_packages_str = list(_format_packages(filter_packages))`

		`- filter_packages_str = []`
		`- for pkg_name, pkg_arch in sorted(filter_packages):`
		`- if pkg_arch:`
		`- filter_packages_str.append("%s.%s" % (pkg_name, pkg_arch))`
		`- else:`
		`- filter_packages_str.append(pkg_name)`
		`+ if not groups and not packages_str and not prepopulate:`
		`+ raise RuntimeError(`
		`+ 'No packages included in %s.%s (no comps groups, no input packages, no prepopulate)'`
		`+ % (variant.uid, arch))`

		`- pungi_wrapper.write_kickstart(ks_path=pungi_cfg, repos=repos, groups=groups, packages=packages_str, exclude_packages=filter_packages_str, comps_repo=comps_repo, lookaside_repos=lookaside_repos, fulltree_excludes=fulltree_excludes, multilib_whitelist=multilib_whitelist, multilib_blacklist=multilib_blacklist, prepopulate=prepopulate)`
		`+ pungi_wrapper.write_kickstart(`
		`+ ks_path=pungi_cfg, repos=repos, groups=groups, packages=packages_str,`
		`+ exclude_packages=filter_packages_str,`
		`+ lookaside_repos=lookaside_repos, fulltree_excludes=fulltree_excludes,`
		`+ multilib_whitelist=multilib_whitelist, multilib_blacklist=multilib_blacklist,`
		`+ prepopulate=prepopulate)`


		`def resolve_deps(compose, arch, variant):`

pungi/phases/init.py

file modified

+6 -1

		`@@ -100,6 +100,9 @@`
		`compose.paths.work.comps(arch="global")])`


		`+ UNMATCHED_GROUP_MSG = 'Variant %s.%s requires comps group %s which does not match anything in input comps file'`
		`+`
		`+`
		`def write_variant_comps(compose, arch, variant):`
		`comps_file = compose.paths.work.comps(arch=arch, variant=variant)`
		`msg = "Writing comps file (arch: %s, variant: %s): %s" % (arch, variant, comps_file)`
		`@@ -121,7 +124,9 @@`
		`"--output=%s" % comps_file, compose.paths.work.comps(arch="global")])`

		`comps = CompsWrapper(comps_file)`
		`- comps.filter_groups(variant.groups)`
		`+ unmatched = comps.filter_groups(variant.groups)`
		`+ for grp in unmatched:`
		`+ compose.log_warning(UNMATCHED_GROUP_MSG % (variant.uid, arch, grp))`
		`if compose.conf["comps_filter_environments"]:`
		`comps.filter_environments(variant.environments)`
		`comps.write_comps()`

pungi/wrappers/comps.py

file modified

+11

		`@@ -333,6 +333,17 @@`
		`if key in to_remove:`
		`del self.comps._groups[key]`

		`+ # Sanity check to report warnings on unused group_dicts`
		`+ unmatched = set()`
		`+ for group_dict in group_dicts:`
		`+ matcher = fnmatch.fnmatch if group_dict["glob"] else lambda x, y: x == y`
		`+ for group_obj in self.comps.groups:`
		`+ if matcher(group_obj.groupid, group_dict["name"]):`
		`+ break`
		`+ else:`
		`+ unmatched.add(group_dict["name"])`
		`+ return unmatched`
		`+`
		`def filter_packages(self, pkglist):`
		`rv = []`
		`for group_obj in self.comps.get_groups():`

tests/test_gather_method_deps.py

file added

+69

		`@@ -0,0 +1,69 @@`
		`+ # -- coding: utf-8 --`
		`+`
		`+ import mock`
		`+ import os`
		`+ import sys`
		`+`
		`+ sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))`
		`+`
		`+ from pungi.phases.gather.methods import method_deps as deps`
		`+ from tests import helpers`
		`+`
		`+`
		`+ class TestWritePungiConfig(helpers.PungiTestCase):`
		`+ def setUp(self):`
		`+ super(TestWritePungiConfig, self).setUp()`
		`+ self.compose = helpers.DummyCompose(self.topdir, {})`
		`+ self.compose.DEBUG = False`
		`+`
		`+ def assertWritten(self, PungiWrapper, **kwargs):`
		`+ wrapper = PungiWrapper.return_value`
		`+ self.assertEqual(wrapper.mock_calls,`
		`+ [mock.call.write_kickstart(**kwargs)])`
		`+`
		`+ @mock.patch('pungi.phases.gather.methods.method_deps.PungiWrapper')`
		`+ def test_correct(self, PungiWrapper):`
		`+ pkgs = [('pkg1', None), ('pkg2', 'x86_64')]`
		`+ grps = ['grp1']`
		`+ filter = [('pkg3', None), ('pkg4', 'x86_64')]`
		`+ white = mock.Mock()`
		`+ black = mock.Mock()`
		`+ prepopulate = mock.Mock()`
		`+ fulltree = mock.Mock()`
		`+ deps.write_pungi_config(self.compose, 'x86_64', self.compose.variants['Server'],`
		`+ pkgs, grps, filter, white, black,`
		`+ prepopulate=prepopulate, fulltree_excludes=fulltree)`
		`+ self.assertWritten(PungiWrapper, packages=['pkg1', 'pkg2.x86_64'],`
		`+ ks_path=self.topdir + '/work/x86_64/pungi/Server.x86_64.conf',`
		`+ lookaside_repos={}, multilib_whitelist=white, multilib_blacklist=black,`
		`+ groups=['grp1'], prepopulate=prepopulate,`
		`+ repos={'pungi-repo': self.topdir + '/work/x86_64/repo'},`
		`+ exclude_packages=['pkg3', 'pkg4.x86_64'],`
		`+ fulltree_excludes=fulltree)`
		`+`
		`+ @mock.patch('pungi.phases.gather.get_lookaside_repos')`
		`+ @mock.patch('pungi.phases.gather.methods.method_deps.PungiWrapper')`
		`+ def test_with_lookaside(self, PungiWrapper, glr):`
		`+ glr.return_value = ['http://example.com/repo']`
		`+ pkgs = [('pkg1', None)]`
		`+ deps.write_pungi_config(self.compose, 'x86_64', self.compose.variants['Server'],`
		`+ pkgs, [], [], [], [])`
		`+ self.assertWritten(PungiWrapper, packages=['pkg1'],`
		`+ ks_path=self.topdir + '/work/x86_64/pungi/Server.x86_64.conf',`
		`+ lookaside_repos={'lookaside-repo-0': 'http://example.com/repo'},`
		`+ multilib_whitelist=[], multilib_blacklist=[],`
		`+ groups=[], prepopulate=None,`
		`+ repos={'pungi-repo': self.topdir + '/work/x86_64/repo'},`
		`+ exclude_packages=[], fulltree_excludes=None)`
		`+ self.assertEqual(glr.call_args_list,`
		`+ [mock.call(self.compose, 'x86_64', self.compose.variants['Server'])])`
		`+`
		`+ @mock.patch('pungi.phases.gather.methods.method_deps.PungiWrapper')`
		`+ def test_without_input(self, PungiWrapper):`
		`+ with self.assertRaises(RuntimeError) as ctx:`
		`+ deps.write_pungi_config(self.compose, 'x86_64', self.compose.variants['Server'],`
		`+ [], [], [], [], [])`
		`+ self.assertEqual(`
		`+ str(ctx.exception),`
		`+ 'No packages included in Server.x86_64 (no comps groups, no input packages, no prepopulate)')`
		`+ self.assertEqual(PungiWrapper.return_value.mock_calls, [])`

tests/test_initphase.py

file modified

+29 -1

		`@@ -201,6 +201,8 @@`
		`compose = DummyCompose(self.topdir, {})`
		`compose.DEBUG = False`
		`variant = compose.variants['Server']`
		`+ comps = CompsWrapper.return_value`
		`+ comps.filter_groups.return_value = []`

		`init.write_variant_comps(compose, 'x86_64', variant)`

		`@@ -211,11 +213,37 @@`
		`self.topdir + '/work/global/comps/comps-global.xml'])])`
		`self.assertEqual(CompsWrapper.call_args_list,`
		`[mock.call(self.topdir + '/work/x86_64/comps/comps-Server.x86_64.xml')])`
		`+ self.assertEqual(comps.filter_groups.call_args_list, [mock.call(variant.groups)])`
		`+ self.assertEqual(comps.filter_environments.mock_calls,`
		`+ [mock.call(variant.environments)])`
		`+ self.assertEqual(comps.write_comps.mock_calls, [mock.call()])`
		`+`
		`+ @mock.patch('pungi.phases.init.run')`
		`+ @mock.patch('pungi.phases.init.CompsWrapper')`
		`+ def test_run_report_unmatched(self, CompsWrapper, run):`
		`+ compose = DummyCompose(self.topdir, {})`
		`+ compose.DEBUG = False`
		`+ variant = compose.variants['Server']`
		`comps = CompsWrapper.return_value`
		`- self.assertEqual(comps.filter_groups.mock_calls, [mock.call(variant.groups)])`
		`+ comps.filter_groups.return_value = ['foo', 'bar']`
		`+`
		`+ init.write_variant_comps(compose, 'x86_64', variant)`
		`+`
		`+ self.assertEqual(run.mock_calls,`
		`+ [mock.call(['comps_filter', '--arch=x86_64', '--keep-empty-group=conflicts',`
		`+ '--keep-empty-group=conflicts-server',`
		`+ '--output=%s/work/x86_64/comps/comps-Server.x86_64.xml' % self.topdir,`
		`+ self.topdir + '/work/global/comps/comps-global.xml'])])`
		`+ self.assertEqual(CompsWrapper.call_args_list,`
		`+ [mock.call(self.topdir + '/work/x86_64/comps/comps-Server.x86_64.xml')])`
		`+ self.assertEqual(comps.filter_groups.call_args_list, [mock.call(variant.groups)])`
		`self.assertEqual(comps.filter_environments.mock_calls,`
		`[mock.call(variant.environments)])`
		`self.assertEqual(comps.write_comps.mock_calls, [mock.call()])`
		`+ self.assertEqual(`
		`+ compose.log_warning.call_args_list,`
		`+ [mock.call(init.UNMATCHED_GROUP_MSG % ('Server', 'x86_64', 'foo')),`
		`+ mock.call(init.UNMATCHED_GROUP_MSG % ('Server', 'x86_64', 'bar'))])`

		`@mock.patch('pungi.phases.init.run')`
		`@mock.patch('pungi.phases.init.CompsWrapper')`

lsedlar commented 8 years ago

If a variants XML mentions a non-existing comps group, a warning is logged. This is not necessarily an error, but having this information can be useful.

The second part is reporting explicitly that there is a variant with no comps group, prepopulate or additional packages, and it's not marked as empty. Without this patch we would run depsolving and end up with an error complaining about "No packages found".

Fixes #585.