PR#1258: Permit adding alternate variants - pungi

pungi

#1258 Permit adding alternate variants

Opened 4 years ago by jcpunk. Modified 4 years ago

jcpunk/pungi note-variant-variant into master

Permit adding alternate variants

Pat Riehecky • 4 years ago

7f6d4ef

pungi/metadata.py

file modified

+15

		`@@ -282,6 +282,21 @@`
		`if os.path.isfile(repomd_path):`
		`ti.checksums.add(repomd_path, createrepo_checksum, root_dir=os_tree)`

		`+ for i in variant.get_variants(types=["variant"], arch=arch):`
		`+ related_variant = productmd.treeinfo.Variant(ti)`
		`+ related_variant.id = i.id`
		`+ related_variant.uid = i.uid`
		`+ related_variant.name = i.name`
		`+ related_variant.type = i.type`
		`+`
		`+ real_variant = copy.deepcopy(i)`
		`+ real_variant.parent = None`
		`+`
		`+ os_tree = compose.paths.compose.os_tree(arch=arch, variant=variant).rstrip("/") + "/"`
		`+ related_variant.paths.packages = relative_path(compose.paths.compose.packages(arch=arch, variant=real_variant, create_dir=False).rstrip("/") + "/", os_tree).rstrip("/") or "."`
		`+ related_variant.paths.repository = relative_path(compose.paths.compose.repository(arch=arch, variant=real_variant, create_dir=False).rstrip("/") + "/", os_tree).rstrip("/") or "."`
		`+ ti.variants.add(related_variant)`
		`+`
		`class LoraxProduct(productmd.treeinfo.Release):`
		`def _validate_short(self):`
		`# HACK: set self.short so .treeinfo produced by lorax can be read`

pungi/wrappers/variants.py

file modified

+6 -3

		`@@ -193,12 +193,15 @@`
		`self.add_child(optional, variant)`

		`for ref in variant_node.xpath("variants/ref/@id"):`
		`- try:`
		`+ if ref in self.addons:`
		`child_variant = self.parse_variant_node(self.addons[ref], variant)`
		`- except KeyError:`
		`+ self.add_child(child_variant, variant)`
		`+ elif ref in self.variants:`
		`+ child_variant = copy.deepcopy(self.variants[ref])`
		`+ self.add_child(child_variant, variant)`
		`+ else:`
		`raise RuntimeError("Variant %s references non-existing variant %s"`
		`% (variant.uid, ref))`
		`- self.add_child(child_variant, variant)`

		`# XXX: top-level optional`
		`# for ref in variant_node.xpath("variants/ref/@id"):`

tests/data/dummy-pungi.conf

file modified

		`@@ -101,6 +101,15 @@`
		`'*': True,`
		`'src': True`
		`}),`
		`+ ('^Everything-Client$', {`
		`+ '*': True`
		`+ }),`
		`+ ]`
		`+`
		`+ buildinstall_skip = [`
		`+ ('^Everything-Client$', {`
		`+ '*': True`
		`+ }),`
		`]`

		`extra_isos = {`

tests/data/dummy-variants.xml

file modified

+10 -7

		`@@ -30,13 +30,6 @@`
		`</groups>`
		`</variant>`

		`- <variant id="Everything" name="Everything" type="variant">`
		`- <arches>`
		`- <arch>i386</arch>`
		`- <arch>x86_64</arch>`
		`- </arches>`
		`- </variant>`
		`-`
		`<variant id="Client" name="Client" type="variant" has_optional="true">`
		`<arches>`
		`<arch>i386</arch>`
		`@@ -55,6 +48,16 @@`
		`</environments>`
		`</variant>`

		`+ <variant id="Everything" name="Everything" type="variant">`
		`+ <arches>`
		`+ <arch>i386</arch>`
		`+ <arch>x86_64</arch>`
		`+ </arches>`
		`+ <variants>`
		`+ <ref id="Client"/>`
		`+ </variants>`
		`+ </variant>`
		`+`
		`<variant id="Server" name="Server" type="variant" has_optional="true">`
		`<arches>`
		`<arch>x86_64</arch>`

jcpunk commented 4 years ago

I think this resolves https://pagure.io/pungi/issue/1256

It requires https://pagure.io/pungi/pull-request/1257 first.

It provides a clear way to specify, "this variant should also include this other variant in the .treeinfo file automatically."

onosek commented 4 years ago

It fails 'test_validate_dummy_config' in 'tests/test_config_validate_script.py'.

rebased onto 7f6d4ef

4 years ago

jcpunk commented 4 years ago

I've rebased off of master, and I'm not showing an error from tests/test_config_validate_script.py in my test box.

[root@17180f0b680f pungi]# git status
On branch note-variant-variant
nothing to commit, working tree clean
[root@17180f0b680f pungi]# make test
nosetests --exe 
..............................................................................................................................................................................................................SS.......................................................................................S..S...........................................................S..................................................................................................................................SSS........................................SSSS...........................................................................................................................................................................................................................................................................................................S....................SSSS...........................................................................................................................................................................................
----------------------------------------------------------------------
Ran 1047 tests in 19.192s

OK (SKIP=17)
[root@17180f0b680f pungi]# cat /etc/os-release 
NAME=Fedora
VERSION="29 (Container Image)"
ID=fedora
VERSION_ID=29
VERSION_CODENAME=""
PLATFORM_ID="platform:f29"
PRETTY_NAME="Fedora 29 (Container Image)"
ANSI_COLOR="0;34"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:29"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f29/system-administrators-guide/"
SUPPORT_URL="https://fedoraproject.org/wiki/Communicating_and_getting_help"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=29
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=29
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="Container Image"
VARIANT_ID=container
[root@17180f0b680f pungi]#

lsedlar commented 4 years ago

I like the idea. I'm not sure how exactly the files were generated for RHEL 8 beta, but this could produce similar results. I will test and report back.

I'm not seeing any failures in tests either.

onosek commented 4 years ago

After sources were rebased, unittests are passing.

lsedlar commented 4 years ago

I'm not sure it works as intended. It will modify the .treeinfo file, but even in the dummy test config it leads to weird stuff. In the final compose, there is both Client and Everything-Client. That does not seem right. Ideally I would expect this patch to only affect the .treeinfo. Duplicating a variant is not really correct.