diff --git a/pungi/compose.py b/pungi/compose.py index 69511a84..8935a0fa 100644 --- a/pungi/compose.py +++ b/pungi/compose.py @@ -202,9 +202,11 @@ class Compose(kobo.log.LoggingBase): shutil.copy2(os.path.join(tmp_dir, file_name), variants_file) shutil.rmtree(tmp_dir) - file_obj = open(variants_file, "r") tree_arches = self.conf.get("tree_arches", None) - self.variants = VariantsXmlParser(file_obj, tree_arches).parse() + tree_variants = self.conf.get("tree_variants", None) + with open(variants_file, "r") as file_obj: + parser = VariantsXmlParser(file_obj, tree_arches, tree_variants, logger=self._logger) + self.variants = parser.parse() # populate ci_base with variants - needed for layered-products (compose_id) ####FIXME - compose_to_composeinfo is no longer needed and has been @@ -215,27 +217,17 @@ class Compose(kobo.log.LoggingBase): def get_variants(self, types=None, arch=None, recursive=False): result = [] types = types or ["variant", "optional", "addon", "layered-product"] - tree_variants = self.conf.get("tree_variants", None) - for i in self.variants.values(): - if tree_variants and i.name not in tree_variants: - continue - if i.type in types: - if arch and arch not in i.arches: - continue + for i in self.variants.itervalues(): + if i.type in types and (not arch or arch in i.arches): result.append(i) result.extend(i.get_variants(types=types, arch=arch, recursive=recursive)) return sorted(set(result)) def get_arches(self): result = set() - tree_arches = self.conf.get("tree_arches", None) for variant in self.get_variants(): for arch in variant.arches: - if tree_arches: - if arch in tree_arches: - result.add(arch) - else: - result.add(arch) + result.add(arch) return sorted(result) @property diff --git a/pungi/wrappers/variants.py b/pungi/wrappers/variants.py index 9448020f..5efa53b5 100755 --- a/pungi/wrappers/variants.py +++ b/pungi/wrappers/variants.py @@ -42,12 +42,14 @@ if not os.path.isfile(VARIANTS_DTD): class VariantsXmlParser(object): - def __init__(self, file_obj, tree_arches=None): + def __init__(self, file_obj, tree_arches=None, tree_variants=None, logger=None): self.tree = lxml.etree.parse(file_obj) self.dtd = lxml.etree.DTD(open(VARIANTS_DTD, "r")) self.addons = {} self.layered_products = {} self.tree_arches = tree_arches + self.tree_variants = tree_variants + self.logger = logger self.validate() def _is_true(self, value): @@ -62,7 +64,7 @@ class VariantsXmlParser(object): errors = [str(i) for i in self.dtd.error_log.filter_from_errors()] raise ValueError("Variants XML doesn't validate:\n%s" % "\n".join(errors)) - def parse_variant_node(self, variant_node): + def parse_variant_node(self, variant_node, parent=None): variant_dict = { "id": str(variant_node.attrib["id"]), "name": str(variant_node.attrib["name"]), @@ -72,9 +74,14 @@ class VariantsXmlParser(object): "environments": [], "buildinstallpackages": [], "is_empty": bool(variant_node.attrib.get("is_empty", False)), + "parent": parent, } if self.tree_arches: variant_dict["arches"] = [i for i in variant_dict["arches"] if i in self.tree_arches] + if not variant_dict["arches"]: + if self.logger: + self.logger.info('Excluding variant %s: all its arches are filtered.' % variant_dict['id']) + return None for grouplist_node in variant_node.xpath("groups"): for group_node in grouplist_node.xpath("group"): @@ -121,19 +128,21 @@ class VariantsXmlParser(object): contains_optional = False for child_node in variant_node.xpath("variants/variant"): - child_variant = self.parse_variant_node(child_node) - variant.add_variant(child_variant) + child_variant = self.parse_variant_node(child_node, variant) + if not self.add_child(child_variant, variant): + continue if child_variant.type == "optional": contains_optional = True has_optional = self._is_true(variant_node.attrib.get("has_optional", "false")) if has_optional and not contains_optional: - optional = Variant(id="optional", name="optional", type="optional", arches=variant.arches, groups=[]) - variant.add_variant(optional) + optional = Variant(id="optional", name="optional", type="optional", + arches=variant.arches, groups=[], parent=variant) + self.add_child(optional, variant) for ref in variant_node.xpath("variants/ref/@id"): - child_variant = self.parse_variant_node(self.addons[ref]) - variant.add_variant(child_variant) + child_variant = self.parse_variant_node(self.addons[ref], variant) + self.add_child(child_variant, variant) # XXX: top-level optional # for ref in variant_node.xpath("variants/ref/@id"): @@ -141,6 +150,19 @@ class VariantsXmlParser(object): return variant + def _is_excluded(self, variant): + if self.tree_variants and variant.uid not in self.tree_variants: + if self.logger: + self.logger.info('Excluding variant %s: filtered by configuration.' % variant) + return True + return False + + def add_child(self, child, parent): + if not child or self._is_excluded(child): + return None + parent.add_variant(child) + return child + def parse(self): # we allow top-level addon definitions which can be referenced in variants for variant_node in self.tree.xpath("/variants/variant[@type='addon']"): @@ -154,6 +176,8 @@ class VariantsXmlParser(object): result = {} for variant_node in self.tree.xpath("/variants/variant[@type='variant']"): variant = self.parse_variant_node(variant_node) + if not variant or self._is_excluded(variant): + continue result[variant.id] = variant for variant_node in self.tree.xpath("/variants/variant[not(@type='variant' or @type='addon' or @type='layered-product')]"): @@ -163,7 +187,8 @@ class VariantsXmlParser(object): class Variant(object): - def __init__(self, id, name, type, arches, groups, environments=None, buildinstallpackages=None, is_empty=False): + def __init__(self, id, name, type, arches, groups, environments=None, + buildinstallpackages=None, is_empty=False, parent=None): if not id.isalnum(): raise ValueError("Variant ID must contain only alphanumeric characters: %s" % id) @@ -178,7 +203,7 @@ class Variant(object): self.environments = sorted(copy.deepcopy(environments), lambda x, y: cmp(x["name"], y["name"])) self.buildinstallpackages = sorted(buildinstallpackages) self.variants = {} - self.parent = None + self.parent = parent self.is_empty = is_empty def __getitem__(self, name): diff --git a/tests/fixtures/variants.xml b/tests/fixtures/variants.xml new file mode 100644 index 00000000..6237a114 --- /dev/null +++ b/tests/fixtures/variants.xml @@ -0,0 +1,82 @@ + + + + + + + x86_64 + + + resilient-storage + + + + + + x86_64 + + + + + + ppc64le + + + + + + + x86_64 + + + gluster + + + + + + i386 + x86_64 + + + core + standard + text-internet + firefox + skype + + + minimal + desktop + + + + + + x86_64 + s390x + + + core + standard + text-internet + + + minimal + + + + + + + x86_64 + s390x + + + firefox + + + + + + diff --git a/tests/test_compose.py b/tests/test_compose.py index 0f7457f8..8cd0e537 100755 --- a/tests/test_compose.py +++ b/tests/test_compose.py @@ -13,6 +13,12 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) from pungi.compose import Compose +class ConfigWrapper(dict): + def __init__(self, *args, **kwargs): + super(ConfigWrapper, self).__init__(*args, **kwargs) + self._open_file = '%s/fixtures/config.conf' % os.path.abspath(os.path.dirname(__file__)) + + class ComposeTestCase(unittest.TestCase): def setUp(self): self.tmp_dir = tempfile.mkdtemp() @@ -123,6 +129,158 @@ class ComposeTestCase(unittest.TestCase): self.assertEqual(compose.image_release, '20160107.n.2') + @mock.patch('pungi.compose.ComposeInfo') + def test_get_variant_arches_without_filter(self, ci): + conf = ConfigWrapper( + variants_file={'scm': 'file', + 'repo': None, + 'file': 'variants.xml'}, + release_name='Test', + release_version='1.0', + release_short='test', + ) + + compose = Compose(conf, self.tmp_dir) + compose.read_variants() + + self.assertEqual(sorted([v.uid for v in compose.variants.itervalues()]), + ['Client', 'Crashy', 'Live', 'Server']) + self.assertEqual(sorted([v.uid for v in compose.variants['Server'].variants.itervalues()]), + ['Server-Gluster', 'Server-ResilientStorage', 'Server-optional']) + self.assertItemsEqual(compose.variants['Client'].arches, + ['i386', 'x86_64']) + self.assertItemsEqual(compose.variants['Crashy'].arches, + ['ppc64le']) + self.assertItemsEqual(compose.variants['Live'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].arches, + ['s390x', 'x86_64']) + self.assertItemsEqual(compose.variants['Server'].variants['Gluster'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].variants['ResilientStorage'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].variants['optional'].arches, + ['s390x', 'x86_64']) + + self.assertEqual([v.uid for v in compose.get_variants()], + ['Client', 'Crashy', 'Live', 'Server', 'Server-Gluster', + 'Server-ResilientStorage', 'Server-optional']) + self.assertEqual(compose.get_arches(), ['i386', 'ppc64le', 's390x', 'x86_64']) + + @mock.patch('pungi.compose.ComposeInfo') + def test_get_variant_arches_with_arch_filter(self, ci): + conf = ConfigWrapper( + variants_file={'scm': 'file', + 'repo': None, + 'file': 'variants.xml'}, + release_name='Test', + release_version='1.0', + release_short='test', + tree_arches=['x86_64'], + ) + + compose = Compose(conf, self.tmp_dir) + compose.read_variants() + + self.assertEqual(sorted([v.uid for v in compose.variants.itervalues()]), + ['Client', 'Live', 'Server']) + self.assertEqual(sorted([v.uid for v in compose.variants['Server'].variants.itervalues()]), + ['Server-Gluster', 'Server-ResilientStorage', 'Server-optional']) + self.assertItemsEqual(compose.variants['Client'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Live'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].variants['Gluster'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].variants['ResilientStorage'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].variants['optional'].arches, + ['x86_64']) + + self.assertEqual(compose.get_arches(), ['x86_64']) + self.assertEqual([v.uid for v in compose.get_variants()], + ['Client', 'Live', 'Server', 'Server-Gluster', + 'Server-ResilientStorage', 'Server-optional']) + + @mock.patch('pungi.compose.ComposeInfo') + def test_get_variant_arches_with_variant_filter(self, ci): + ci.return_value.compose.respin = 2 + ci.return_value.compose.date = '20160107' + ci.return_value.compose.type = 'production' + ci.return_value.compose.type_suffix = '.n' + + conf = ConfigWrapper( + variants_file={'scm': 'file', + 'repo': None, + 'file': 'variants.xml'}, + release_name='Test', + release_version='1.0', + release_short='test', + tree_variants=['Server', 'Client', 'Server-Gluster'], + ) + + compose = Compose(conf, self.tmp_dir) + compose.read_variants() + + self.assertEqual(sorted([v.uid for v in compose.variants.itervalues()]), + ['Client', 'Server']) + self.assertItemsEqual(compose.variants['Client'].arches, + ['i386', 'x86_64']) + self.assertItemsEqual(compose.variants['Server'].arches, + ['s390x', 'x86_64']) + self.assertItemsEqual(compose.variants['Server'].variants['Gluster'].arches, + ['x86_64']) + + self.assertEqual(compose.get_arches(), ['i386', 's390x', 'x86_64']) + self.assertEqual([v.uid for v in compose.get_variants()], + ['Client', 'Server', 'Server-Gluster']) + + @mock.patch('pungi.compose.ComposeInfo') + def test_get_variant_arches_with_both_filters(self, ci): + ci.return_value.compose.respin = 2 + ci.return_value.compose.date = '20160107' + ci.return_value.compose.type = 'production' + ci.return_value.compose.type_suffix = '.n' + + logger = mock.Mock() + + conf = ConfigWrapper( + variants_file={'scm': 'file', + 'repo': None, + 'file': 'variants.xml'}, + release_name='Test', + release_version='1.0', + release_short='test', + tree_variants=['Server', 'Client', 'Server-optional'], + tree_arches=['x86_64'], + ) + + compose = Compose(conf, self.tmp_dir, logger=logger) + compose.read_variants() + + self.assertEqual(sorted([v.uid for v in compose.variants.itervalues()]), + ['Client', 'Server']) + self.assertItemsEqual(compose.variants['Client'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].arches, + ['x86_64']) + self.assertItemsEqual(compose.variants['Server'].variants['optional'].arches, + ['x86_64']) + + self.assertEqual(compose.get_arches(), ['x86_64']) + self.assertEqual([v.uid for v in compose.get_variants()], + ['Client', 'Server', 'Server-optional']) + + self.assertItemsEqual( + logger.info.call_args_list, + [mock.call('Excluding variant Live: filtered by configuration.'), + mock.call('Excluding variant Crashy: all its arches are filtered.'), + mock.call('Excluding variant Server-ResilientStorage: filtered by configuration.'), + mock.call('Excluding variant Server-Gluster: filtered by configuration.')] + ) + class StatusTest(unittest.TestCase): def setUp(self):