PR#917: gather: Resolve weak dependencies - pungi

pungi

#917 gather: Resolve weak dependencies

Closed 5 years ago by lsedlar. Opened 6 years ago by lsedlar.

lsedlar/pungi weak-deps into master

Lubomír Sedlář • 6 years ago

6306fb2

bin/pungi-gather

file modified

		`@@ -70,6 +70,11 @@`
		`metavar="[METHOD]",`
		`action="append",`
		`)`
		`+ group.add_argument(`
		`+ "--weak-deps",`
		`+ action="store_true",`
		`+ help="follow Recommends and Supplements dependencies",`
		`+ )`
		`return parser`


		`@@ -102,6 +107,9 @@`
		`if ns.nodeps:`
		`gather_opts.resolve_deps = False`

		`+ if ns.weak_deps:`
		`+ gather_opts.weak_deps = True`
		`+`
		`ksparser = pungi.ks.get_ksparser(ns.config)`

		`# read repos from ks`

doc/configuration.rst

file modified

		`@@ -588,6 +588,11 @@`
		(bool) -- When set to ``True``, Pungi will build a self-hosting tree by
		following build dependencies. Only use when ``gather_method = "deps"``.

		`+ gather_follow_weak_deps = False`
		+ (bool) -- When set to ``True``, Pungi will treat ``Recommends`` as
		+ ``Requires``. It will also include packages that ``Supplements`` something
		`+ in the compose. The default is to ignore weak dependencies.`
		`+`
		`greedy_method`
		`(str) -- This option controls how package requirements are satisfied in`
		case a particular ``Requires`` has multiple candidates.

pungi/checks.py

file modified

		`@@ -587,6 +587,10 @@`
		`"type": "boolean",`
		`"default": False,`
		`},`
		`+ "gather_follow_weak_deps": {`
		`+ "type": "boolean",`
		`+ "default": False,`
		`+ },`
		`"gather_prepopulate": {"$ref": "#/definitions/str_or_scm_dict"},`
		`"gather_source_mapping": {"type": "string"},`
		`"gather_backend": {`

pungi/gather_dnf.py

file modified

+31

		`@@ -71,6 +71,8 @@`

		`self.package_whitelist = set()`

		`+ self.weak_deps = False`
		`+`
		`self.merge_options(**kwargs)`

		`def __str__(self):`
		`@@ -78,6 +80,7 @@`
		`'fulltree=%s' % self.fulltree,`
		`'fulltree_excludes=%d items' % len(self.fulltree_excludes),`
		`'resolve_deps=%s' % self.resolve_deps,`
		`+ 'weak_deps=%s' % self.weak_deps,`
		`'selfhosting=%s' % self.selfhosting,`
		`'greedy_method=%s' % self.greedy_method,`
		`'langpacks=%s' % self.langpacks,`
		`@@ -205,6 +208,7 @@`
		`self.finished_add_source_package_deps = {} # {pkg: [deps]}`

		`self.finished_get_package_deps_reqs = {}`
		`+ self.finished_supplements = set()`

		`self.finished_add_conditional_packages = {} # {pkg: [pkgs]}`
		`self.finished_add_source_packages = {} # {pkg: src-pkg\|None}`
		`@@ -316,6 +320,9 @@`
		`getattr(pkg, 'requires_pre', []) +`
		`getattr(pkg, 'requires_post', []))`

		`+ if self.opts.weak_deps:`
		`+ requires.extend(pkg.recommends)`
		`+`
		`q = self.q_binary_packages.filter(provides=requires).apply()`
		`for req in requires:`
		`deps = self.finished_get_package_deps_reqs.setdefault(str(req), set())`
		`@@ -511,6 +518,26 @@`

		`return added`

		`+ @Profiler("Gather.add_supplements()")`
		`+ def add_supplements(self):`
		`+ added = set()`
		`+`
		`+ if not self.opts.resolve_deps:`
		`+ return added`
		`+`
		`+ for pkg in self.result_binary_packages.copy():`
		`+ if pkg in self.finished_supplements:`
		`+ continue`
		`+ for prov in pkg.provides:`
		`+ supplements = self.q_binary_packages.filter(`
		`+ supplements=prov, arch=[pkg.arch] + self.dnf.arch_wrapper.native_arches`
		`+ ).apply()`
		`+ self._add_packages(supplements, pulled_by=pkg, reason='supplements')`
		`+ added.update(supplements)`
		`+ self.finished_supplements.add(pkg)`
		`+`
		`+ return added`
		`+`
		`@Profiler("Gather.add_conditional_packages()")`
		`def add_conditional_packages(self):`
		`"""`
		`@@ -827,6 +854,10 @@`
		`if self.log_count('BINARY DEPS', self.add_binary_package_deps):`
		`continue`

		`+ if self.opts.weak_deps:`
		`+ if self.log_count('SUPPLEMENTS', self.add_supplements):`
		`+ continue`
		`+`
		`if self.log_count('SOURCE DEPS', self.add_source_package_deps):`
		`continue`

pungi/phases/gather/methods/method_deps.py

file modified

+2 -1

		`@@ -170,6 +170,7 @@`
		`# variant`
		`fulltree = compose.conf["gather_fulltree"]`
		`selfhosting = compose.conf["gather_selfhosting"]`
		`+ weak_deps = compose.conf["gather_follow_weak_deps"]`

		`# profiling`
		`profiler = compose.conf["gather_profiler"]`
		`@@ -202,7 +203,7 @@`
		`selfhosting=selfhosting, fulltree=fulltree, arch=yum_arch,`
		`full_archlist=True, greedy=greedy_method, cache_dir=cache_dir,`
		`lookaside_repos=lookaside_repos, multilib_methods=multilib_methods,`
		`- profiler=profiler)`
		`+ profiler=profiler, weak_deps=weak_deps)`
		`# Use temp working directory directory as workaround for`
		`# https://bugzilla.redhat.com/show_bug.cgi?id=795137`
		`with temp_dir(prefix='pungi_') as tmp_dir:`

pungi/wrappers/pungi.py

file modified

+14 -2

		`@@ -96,7 +96,11 @@`

		`kickstart.close()`

		`- def get_pungi_cmd(self, config, destdir, name, version=None, flavor=None, selfhosting=False, fulltree=False, greedy=None, nodeps=False, nodownload=True, full_archlist=False, arch=None, cache_dir=None, lookaside_repos=None, multilib_methods=None, profiler=False):`
		`+ def get_pungi_cmd(self, config, destdir, name, version=None, flavor=None,`
		`+ selfhosting=False, fulltree=False, greedy=None,`
		`+ nodeps=False, nodownload=True, full_archlist=False,`
		`+ arch=None, cache_dir=None, lookaside_repos=None,`
		`+ multilib_methods=None, profiler=False, weak_deps=False):`
		`cmd = ["pungi"]`

		`# Gather stage`
		`@@ -156,7 +160,12 @@`

		`return cmd`

		`- def get_pungi_cmd_dnf(self, config, destdir, name, version=None, flavor=None, selfhosting=False, fulltree=False, greedy=None, nodeps=False, nodownload=True, full_archlist=False, arch=None, cache_dir=None, lookaside_repos=None, multilib_methods=None, profiler=False):`
		`+ def get_pungi_cmd_dnf(self, config, destdir, name, version=None,`
		`+ flavor=None, selfhosting=False, fulltree=False,`
		`+ greedy=None, nodeps=False, nodownload=True,`
		`+ full_archlist=False, arch=None, cache_dir=None,`
		`+ lookaside_repos=None, multilib_methods=None,`
		`+ profiler=False, weak_deps=False):`
		`cmd = ["pungi-gather"]`

		`# path to a kickstart file`
		`@@ -193,6 +202,9 @@`
		`if profiler:`
		`cmd.append("--profiler")`

		`+ if weak_deps:`
		`+ cmd.append("--weak-deps")`
		`+`
		`return cmd`

		`def parse_log(self, f):`

tests/data/specs/center.spec

file added

+29

		`@@ -0,0 +1,29 @@`
		`+ Name: center`
		`+ Version: 1.0`
		`+ Release: 1`
		`+ License: GPLv2`
		`+ Summary: A dummy package`
		`+`
		`+ BuildArch: noarch`
		`+`
		`+ Recommends: left`
		`+`
		`+ %description`
		`+ A dummy package`
		`+`
		`+`
		`+ %build`
		`+ echo OK`
		`+`
		`+`
		`+ %install`
		`+ rm -rf $RPM_BUILD_ROOT`
		`+ mkdir $RPM_BUILD_ROOT`
		`+`
		`+`
		`+ %files`
		`+`
		`+`
		`+ %changelog`
		`+ * Thu Apr 19 2018 Lubomír Sedlář <lsedlar@redhat.com> - 1.0-1`
		`+ - First release`

tests/data/specs/left.spec

file added

+27

		`@@ -0,0 +1,27 @@`
		`+ Name: left`
		`+ Version: 1.0`
		`+ Release: 1`
		`+ License: GPLv2`
		`+ Summary: A dummy package`
		`+`
		`+ BuildArch: noarch`
		`+`
		`+ %description`
		`+ A dummy package`
		`+`
		`+`
		`+ %build`
		`+ echo OK`
		`+`
		`+`
		`+ %install`
		`+ rm -rf $RPM_BUILD_ROOT`
		`+ mkdir $RPM_BUILD_ROOT`
		`+`
		`+`
		`+ %files`
		`+`
		`+`
		`+ %changelog`
		`+ * Thu Apr 19 2018 Lubomír Sedlář <lsedlar@redhat.com> - 1.0-1`
		`+ - First release`

tests/data/specs/right.spec

file added

+29

		`@@ -0,0 +1,29 @@`
		`+ Name: right`
		`+ Version: 1.0`
		`+ Release: 1`
		`+ License: GPLv2`
		`+ Summary: A dummy package`
		`+`
		`+ BuildArch: noarch`
		`+`
		`+ Supplements: center`
		`+`
		`+ %description`
		`+ A dummy package`
		`+`
		`+`
		`+ %build`
		`+ echo OK`
		`+`
		`+`
		`+ %install`
		`+ rm -rf $RPM_BUILD_ROOT`
		`+ mkdir $RPM_BUILD_ROOT`
		`+`
		`+`
		`+ %files`
		`+`
		`+`
		`+ %changelog`
		`+ * Thu Apr 19 2018 Lubomír Sedlář <lsedlar@redhat.com> - 1.0-1`
		`+ - First release`

tests/fixtures/repos/weak-deps/repodata/2e8307f870376f824e88f06c10b1273a68a823332535faa793493e71c15069ad-filelists.xml.gz

file added

empty or binary file added

tests/fixtures/repos/weak-deps/repodata/94a2ffb08b780b7a2f8ee6207d75476a99b6fac8390d7c6c94b9d1d81b0d6700-other.xml.gz

file added

empty or binary file added

tests/fixtures/repos/weak-deps/repodata/d90d2ee5940947c29d6ce453ddac9805661fafbf2ecefe35d4dc99f11b4893e4-primary.xml.gz

file added

empty or binary file added

tests/fixtures/repos/weak-deps/repodata/repomd.xml

file added

+28

		`@@ -0,0 +1,28 @@`
		`+ <?xml version="1.0" encoding="UTF-8"?>`
		`+ <repomd xmlns="http://linux.duke.edu/metadata/repo" xmlns:rpm="http://linux.duke.edu/metadata/rpm">`
		`+ <revision>1524120542</revision>`
		`+ <data type="primary">`
		`+ <checksum type="sha256">d90d2ee5940947c29d6ce453ddac9805661fafbf2ecefe35d4dc99f11b4893e4</checksum>`
		`+ <open-checksum type="sha256">6dcfb1f728c1780165d610e65e1ba9d3a6ad88bfefbd70ef3aebabd1a5c4fb15</open-checksum>`
		`+ <location href="repodata/d90d2ee5940947c29d6ce453ddac9805661fafbf2ecefe35d4dc99f11b4893e4-primary.xml.gz"/>`
		`+ <timestamp>1524120542</timestamp>`
		`+ <size>991</size>`
		`+ <open-size>5194</open-size>`
		`+ </data>`
		`+ <data type="filelists">`
		`+ <checksum type="sha256">2e8307f870376f824e88f06c10b1273a68a823332535faa793493e71c15069ad</checksum>`
		`+ <open-checksum type="sha256">d901fa3bab460ad2fab5f04732d9f39190fb281d4f4209523167bde4c72bb520</open-checksum>`
		`+ <location href="repodata/2e8307f870376f824e88f06c10b1273a68a823332535faa793493e71c15069ad-filelists.xml.gz"/>`
		`+ <timestamp>1524120542</timestamp>`
		`+ <size>487</size>`
		`+ <open-size>1166</open-size>`
		`+ </data>`
		`+ <data type="other">`
		`+ <checksum type="sha256">94a2ffb08b780b7a2f8ee6207d75476a99b6fac8390d7c6c94b9d1d81b0d6700</checksum>`
		`+ <open-checksum type="sha256">016cde85a9d2f207333d2399bf0b0d515854ec9ed8c4ccc504c20c0af75b0635</open-checksum>`
		`+ <location href="repodata/94a2ffb08b780b7a2f8ee6207d75476a99b6fac8390d7c6c94b9d1d81b0d6700-other.xml.gz"/>`
		`+ <timestamp>1524120542</timestamp>`
		`+ <size>557</size>`
		`+ <open-size>1888</open-size>`
		`+ </data>`
		`+ </repomd>`

tests/test_gather.py

file modified

+19

		`@@ -1910,3 +1910,22 @@`
		`])`
		`self.assertItemsEqual(pkg_map["debuginfo"], [`
		`])`
		`+`
		`+ def test_pull_in_weak_deps(self):`
		`+ self.repo = os.path.join(os.path.dirname(__file__), "fixtures/repos/weak-deps")`
onosek commented 5 years ago Is self.repo used somehow? Tests work even without this row.
lsedlar commented 5 years ago It should be used in the `go` method. If this line is removed, the default repo without the packages would be used and the test should fail.
		`+`
		`+ packages = ['center']`
		`+`
		`+ pkg_map = self.go(packages, None, weak_deps=True)`
		`+`
		`+ self.assertItemsEqual(pkg_map["rpm"], [`
		`+ 'center-1.0-1.noarch.rpm',`
		`+ 'left-1.0-1.noarch.rpm', # Recommended by center`
		`+ 'right-1.0-1.noarch.rpm', # Supplements center`
		`+ ])`
		`+ self.assertItemsEqual(pkg_map["srpm"], [`
		`+ 'center-1.0-1.src.rpm',`
		`+ 'left-1.0-1.src.rpm',`
		`+ 'right-1.0-1.src.rpm',`
		`+ ])`
		`+ self.assertItemsEqual(pkg_map["debuginfo"], [])`

lsedlar commented 6 years ago

This patch updates depsolving via DNF to treat Recommends the same as Requires, and adds a separate step to pull in packages that Supplement something that is already in the compose.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1569242

lsedlar commented 6 years ago

Unfortunately I don't see a good way to test it. I'll run a test compose in stage, but that does not really show how the production compose would differ.

rebased onto ad4933191906574eac48a683a869e2ec837c8d48

6 years ago

rebased onto 0daa22d18e6a5ee69678d543bb74d0f1bef7c999

6 years ago

lsedlar commented 6 years ago

Supplements handling needs to be fixed, it does weird things with pulling multilib packages into the compose.

rebased onto a9459a46ea210dc6081fe0322dd0a500d2a6ebae

6 years ago

rebased onto 6306fb2

6 years ago

lsedlar commented 6 years ago

There is still quite a big difference:
https://paste.fedoraproject.org/paste/4FWKrCIoo00PT1HPeK3oRQ
Multilib handling might be correct, for example glibc-langpack-wae.i686 gets in because it supplements glibc.i686 which gets in via usual multilib rules.