diff --git a/pungi/phases/gather/methods/method_hybrid.py b/pungi/phases/gather/methods/method_hybrid.py index fa38b8fe..a7b48396 100644 --- a/pungi/phases/gather/methods/method_hybrid.py +++ b/pungi/phases/gather/methods/method_hybrid.py @@ -79,6 +79,12 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase): self.added_langpacks = set() # Set of NEVRAs of modular packages self.modular_packages = set() + # Arch -> pkg name -> set of pkg object + self.debuginfo = defaultdict(lambda: defaultdict(set)) + + # caches for processed packages + self.processed_multilib = set() + self.processed_debuginfo = set() def _get_pkg_map(self, arch): """Create a mapping from NEVRA to actual package object. This will be @@ -111,6 +117,20 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase): self._prepare_packages() return self.packages[nevra] + def _prepare_debuginfo(self): + """Prepare cache of debuginfo packages for easy access. The cache is + indexed by package architecture and then by package name. There can be + more than one debuginfo package with the same name. + """ + for pkg_arch in self.package_sets[self.arch].rpms_by_arch: + for pkg in self.package_sets[self.arch].rpms_by_arch[pkg_arch]: + self.debuginfo[pkg.arch][pkg.name].add(pkg) + + def _get_debuginfo(self, name, arch): + if not self.debuginfo: + self._prepare_debuginfo() + return self.debuginfo.get(arch, {}).get(name, set()) + def expand_list(self, patterns): """Given a list of globs, create a list of package names matching any of the pattern. @@ -206,6 +226,8 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase): def run_solver(self, variant, arch, packages, platform, filter_packages): repos = [self.compose.paths.work.arch_repo(arch=arch)] + results = set() + result_modules = set() modules = [] if variant.arch_mmds.get(arch): @@ -219,8 +241,6 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase): step = 0 - old_multilib = set() - while True: step += 1 conf_file = self.compose.paths.work.fus_conf(arch, variant, step) @@ -245,31 +265,45 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase): run(cmd, logfile=logfile, show_cmd=True, env=env) output, out_modules = fus.parse_output(logfile) self.compose.log_debug("[DONE ] Running fus") - new_multilib = self.add_multilib(variant, arch, output, old_multilib) - old_multilib = new_multilib - if new_multilib: - input_packages.extend( - _fmt_pkg(pkg_name, pkg_arch) - for pkg_name, pkg_arch in sorted(new_multilib) - ) - continue + # No need to resolve modules again. They are not going to change. + modules = [] + # Reset input packages as well to only solve newly added things. + input_packages = [] + # Preserve the results from this iteration. + results.update(output) + result_modules.update(out_modules) + + new_multilib = self.add_multilib(variant, arch, output) + input_packages.extend( + _fmt_pkg(pkg_name, pkg_arch) + for pkg_name, pkg_arch in sorted(new_multilib) + ) + + new_debuginfo = self.add_debuginfo(arch, output) + input_packages.extend( + _fmt_pkg(pkg_name, pkg_arch) + for pkg_name, pkg_arch in sorted(new_debuginfo) + ) new_langpacks = self.add_langpacks(output) - if new_langpacks: - input_packages.extend(new_langpacks) - continue + input_packages.extend(new_langpacks) - # Nothing new was added, we can stop now. - break + if not input_packages: + # Nothing new was added, we can stop now. + break - return output, out_modules + return results, result_modules - def add_multilib(self, variant, arch, nvrs, old_multilib): + def add_multilib(self, variant, arch, nvrs): added = set() if not self.multilib_methods: return [] for nvr, pkg_arch, flags in nvrs: + if (nvr, pkg_arch) in self.processed_multilib: + continue + self.processed_multilib.add((nvr, pkg_arch)) + if "modular" in flags: continue @@ -289,7 +323,34 @@ class GatherMethodHybrid(pungi.phases.gather.method.GatherMethodBase): if self.multilib.is_multilib(multilib_candidate): added.add((nevr["name"], add_arch)) - return added - old_multilib + return added + + def add_debuginfo(self, arch, nvrs): + added = set() + + for nvr, pkg_arch, flags in nvrs: + if (nvr, pkg_arch) in self.processed_debuginfo: + continue + self.processed_debuginfo.add((nvr, pkg_arch)) + + if "modular" in flags: + continue + + pkg = self._get_package("%s.%s" % (nvr, pkg_arch)) + + # There are two ways how the debuginfo package can be named. We + # want to get them all. + for pattern in ["%s-debuginfo", "%s-debugsource"]: + debuginfo_name = pattern % pkg.name + debuginfo = self._get_debuginfo(debuginfo_name, pkg_arch) + for dbg in debuginfo: + # For each debuginfo package that matches on name and + # architecture, we also need to check if it comes from the + # same build. + if dbg.sourcerpm == pkg.rpm_sourcerpm: + added.add((dbg.name, dbg.arch)) + + return added def add_langpacks(self, nvrs): if not self.langpacks: @@ -477,8 +538,8 @@ def _make_result(paths): def expand_packages(nevra_to_pkg, variant_modules, lookasides, nvrs, filter_packages): - """For each package add source RPM and possibly also debuginfo.""" - # This will server as the final result. We collect sets of paths to the + """For each package add source RPM.""" + # This will serve as the final result. We collect sets of paths to the # packages. rpms = set() srpms = set() @@ -500,12 +561,8 @@ def expand_packages(nevra_to_pkg, variant_modules, lookasides, nvrs, filter_pack # Strip file:// prefix lookaside_packages.add(url[7:]) - # This is used to figure out which debuginfo packages to include. We keep - # track of package architectures from each SRPM. - srpm_arches = defaultdict(set) - - for nvr, arch, flags in nvrs: - pkg = nevra_to_pkg["%s.%s" % (nvr, arch)] + for nvr, pkg_arch, flags in nvrs: + pkg = nevra_to_pkg["%s.%s" % (nvr, pkg_arch)] if pkg.file_path in lookaside_packages: # Package is in lookaside, don't add it and ignore sources and # debuginfo too. @@ -518,11 +575,6 @@ def expand_packages(nevra_to_pkg, variant_modules, lookasides, nvrs, filter_pack try: srpm_nevra = _get_srpm_nevra(pkg) srpm = nevra_to_pkg[srpm_nevra] - if "modular" not in flags: - # Only mark the arch for sources of non-modular packages. The - # debuginfo is explicitly listed in the output, and we don't - # want anything more. - srpm_arches[srpm_nevra].add(arch) if (srpm.name, "src") in filters: # Filtered package, skipping continue @@ -532,17 +584,6 @@ def expand_packages(nevra_to_pkg, variant_modules, lookasides, nvrs, filter_pack # Didn't find source RPM.. this should be logged pass - # Get all debuginfo packages from all included sources. We iterate over all - # available packages and if we see a debug package from included SRPM built - # for architecture that has at least one binary package, we include it too. - for pkg in nevra_to_pkg.values(): - if pkg_is_debug(pkg) and pkg.arch in srpm_arches[_get_srpm_nevra(pkg)]: - if set([(pkg.name, pkg.arch), (pkg.name, None)]) & filters: - # Filtered package, skipping - continue - if pkg.file_path not in lookaside_packages: - debuginfo.add(pkg.file_path) - return _mk_pkg_map(_make_result(rpms), _make_result(srpms), _make_result(debuginfo)) diff --git a/tests/test_gather_method_hybrid.py b/tests/test_gather_method_hybrid.py index 458b0570..d973e99f 100644 --- a/tests/test_gather_method_hybrid.py +++ b/tests/test_gather_method_hybrid.py @@ -87,7 +87,6 @@ class TestMethodHybrid(helpers.PungiTestCase): eg.call_args_list, [mock.call(compose, arch, variant, ["standard"], set_pkg_arch=False)], ) - print(CW.mock_calls) self.assertEqual( CW.mock_calls, [ @@ -328,6 +327,7 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): self.compose = helpers.DummyCompose(self.topdir, {}) self.phase = hybrid.GatherMethodHybrid(self.compose) self.phase.multilib_methods = [] + self.phase.arch = "x86_64" self.logfile1 = os.path.join( self.compose.topdir, "logs/x86_64/hybrid-depsolver-Server-iter-1.x86_64.log" ) @@ -359,7 +359,7 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): peek_context=mock.Mock(return_value="ctx"), ) ] - po.return_value = (mock.Mock(), mock.Mock()) + po.return_value = ([], ["m1"]) res = self.phase.run_solver( self.compose.variants["Server"], @@ -369,7 +369,8 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): filter_packages=[("foo", None)], ) - self.assertEqual(res, po.return_value) + self.assertItemsEqual(res[0], []) + self.assertItemsEqual(res[1], ["m1"]) self.assertEqual(po.call_args_list, [mock.call(self.logfile1)]) self.assertEqual( run.call_args_list, @@ -412,7 +413,9 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): peek_context=mock.Mock(return_value="ctx"), ), } - po.return_value = (mock.Mock(), mock.Mock()) + po.return_value = ([("p-1-1", "x86_64", frozenset())], ["m1"]) + self.phase.packages = {"p-1-1.x86_64": mock.Mock()} + self.phase.package_sets = {"x86_64": mock.Mock(rpms_by_arch={"x86_64": []})} res = self.phase.run_solver( self.compose.variants["Server"], @@ -422,7 +425,7 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): filter_packages=["foo"], ) - self.assertEqual(res, po.return_value) + self.assertEqual(res, (set([("p-1-1", "x86_64", frozenset())]), set(["m1"]))) self.assertEqual(po.call_args_list, [mock.call(self.logfile1)]) self.assertEqual( run.call_args_list, @@ -451,7 +454,9 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): ) def test_with_comps(self, run, gc, po, wc): - po.return_value = (mock.Mock(), mock.Mock()) + self.phase.packages = {"pkg-1.0-1.x86_64": mock.Mock()} + self.phase.debuginfo = {"x86_64": {}} + po.return_value = ([("pkg-1.0-1", "x86_64", frozenset())], []) res = self.phase.run_solver( self.compose.variants["Server"], "x86_64", @@ -460,7 +465,8 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): filter_packages=[], ) - self.assertEqual(res, po.return_value) + self.assertItemsEqual(res[0], po.return_value[0]) + self.assertItemsEqual(res[1], []) self.assertEqual(po.call_args_list, [mock.call(self.logfile1)]) self.assertEqual( run.call_args_list, @@ -487,20 +493,41 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): ], ) - def test_with_langpacks(self, run, gc, po, wc): - self.phase.langpacks = {"pkg": set(["pkg-en"])} - final = [("pkg-1.0-1", "x86_64", []), ("pkg-en-1.0-1", "noarch", [])] - po.side_effect = [([("pkg-1.0-1", "x86_64", [])], set()), (final, [])] - + def test_with_comps_with_debuginfo(self, run, gc, po, wc): + dbg1 = NamedMock(name="pkg-debuginfo", arch="x86_64", sourcerpm="pkg.src.rpm") + dbg2 = NamedMock(name="pkg-debuginfo", arch="x86_64", sourcerpm="x.src.rpm") + self.phase.packages = { + "pkg-1.0-1.x86_64": NamedMock( + name="pkg", arch="x86_64", rpm_sourcerpm="pkg.src.rpm" + ), + "pkg-debuginfo-1.0-1.x86_64": dbg1, + "pkg-debuginfo-1.0-2.x86_64": dbg2, + } + self.phase.debuginfo = { + "x86_64": { + "pkg-debuginfo": [dbg1, dbg2], + }, + } + po.side_effect = [ + ([("pkg-1.0-1", "x86_64", frozenset())], []), + ([("pkg-debuginfo-1.0-1", "x86_64", frozenset())], []), + ] res = self.phase.run_solver( self.compose.variants["Server"], "x86_64", [("pkg", None)], platform=None, - filter_packages=["foo"], + filter_packages=[], ) - self.assertEqual(res, (final, [])) + self.assertItemsEqual( + res[0], + [ + ("pkg-1.0-1", "x86_64", frozenset()), + ("pkg-debuginfo-1.0-1", "x86_64", frozenset()), + ], + ) + self.assertItemsEqual(res[1], []) self.assertEqual( po.call_args_list, [mock.call(self.logfile1), mock.call(self.logfile2)] ) @@ -519,7 +546,73 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): wc.call_args_list, [ mock.call(self.config1, [], ["pkg"]), - mock.call(self.config2, [], ["pkg", "pkg-en"]), + mock.call(self.config2, [], ["pkg-debuginfo.x86_64"]), + ], + ) + self.assertEqual( + gc.call_args_list, + [ + mock.call( + self.config1, + "x86_64", + [self._repo("repo")], + [], + platform=None, + filter_packages=[], + ), + mock.call( + self.config2, + "x86_64", + [self._repo("repo")], + [], + platform=None, + filter_packages=[], + ), + ], + ) + + def test_with_langpacks(self, run, gc, po, wc): + self.phase.langpacks = {"pkg": set(["pkg-en"])} + final = [ + ("pkg-1.0-1", "x86_64", frozenset()), + ("pkg-en-1.0-1", "noarch", frozenset()), + ] + po.side_effect = [([("pkg-1.0-1", "x86_64", frozenset())], []), (final, [])] + self.phase.packages = { + "pkg-1.0-1.x86_64": mock.Mock(), + "pkg-en-1.0-1.noarch": mock.Mock(), + } + self.phase.package_sets = {"x86_64": mock.Mock(rpms_by_arch={"x86_64": []})} + + res = self.phase.run_solver( + self.compose.variants["Server"], + "x86_64", + [("pkg", None)], + platform=None, + filter_packages=["foo"], + ) + + self.assertItemsEqual(res[0], final) + self.assertItemsEqual(res[1], []) + self.assertEqual( + po.call_args_list, [mock.call(self.logfile1), mock.call(self.logfile2)] + ) + self.assertEqual( + run.call_args_list, + [ + mock.call( + gc.return_value, logfile=self.logfile1, show_cmd=True, env=mock.ANY + ), + mock.call( + gc.return_value, logfile=self.logfile2, show_cmd=True, env=mock.ANY + ), + ], + ) + self.assertEqual( + wc.call_args_list, + [ + mock.call(self.config1, [], ["pkg"]), + mock.call(self.config2, [], ["pkg-en"]), ], ) self.assertEqual( @@ -562,14 +655,20 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): } } self.phase.packages = self.phase.package_maps["x86_64"] - final = [ - ("pkg-devel-1.0-1", "x86_64", []), - ("foo-1.0-1", "x86_64", []), - ("pkg-devel-1.0-1", "i686", []), - ] + self.phase.debuginfo = {"x86_64": {}} po.side_effect = [ - ([("pkg-devel-1.0-1", "x86_64", []), ("foo-1.0-1", "x86_64", [])], []), - (final, []), + ( + [ + ("pkg-devel-1.0-1", "x86_64", frozenset()), + ("foo-1.0-1", "x86_64", frozenset()) + ], + frozenset()), + ( + [ + ("pkg-devel-1.0-1", "i686", frozenset()), + ], + [], + ), ] res = self.phase.run_solver( @@ -580,7 +679,15 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): filter_packages=[], ) - self.assertEqual(res, (final, [])) + self.assertItemsEqual( + res[0], + [ + ("pkg-devel-1.0-1", "x86_64", frozenset()), + ("foo-1.0-1", "x86_64", frozenset()), + ("pkg-devel-1.0-1", "i686", frozenset()), + ] + ) + self.assertItemsEqual(res[1], []) self.assertEqual( po.call_args_list, [mock.call(self.logfile1), mock.call(self.logfile2)] ) @@ -599,7 +706,7 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): wc.call_args_list, [ mock.call(self.config1, [], ["foo", "pkg-devel"]), - mock.call(self.config2, [], ["foo", "pkg-devel", "pkg-devel.i686"]), + mock.call(self.config2, [], ["pkg-devel.i686"]), ], ) self.assertEqual( @@ -668,14 +775,21 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): "foo-1.0-1.i686": mock.Mock(), } } - final = [ - ("pkg-devel-1.0-1", "x86_64", []), - ("foo-1.0-1", "x86_64", []), - ("foo-1.0-1", "i686", []), - ] + self.phase.debuginfo = {"x86_64": {}} po.side_effect = [ - ([("pkg-devel-1.0-1", "x86_64", []), ("foo-1.0-1", "x86_64", [])], []), - (final, []), + ( + [ + ("pkg-devel-1.0-1", "x86_64", frozenset()), + ("foo-1.0-1", "x86_64", frozenset()) + ], + [], + ), + ( + [ + ("foo-1.0-1", "i686", frozenset()), + ], + [], + ), ] res = self.phase.run_solver( @@ -686,7 +800,15 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): filter_packages=[], ) - self.assertEqual(res, (final, [])) + self.assertItemsEqual( + res[0], + [ + ("pkg-devel-1.0-1", "x86_64", frozenset()), + ("foo-1.0-1", "x86_64", frozenset()), + ("foo-1.0-1", "i686", frozenset()), + ], + ) + self.assertItemsEqual(res[1], []) self.assertEqual( po.call_args_list, [mock.call(self.logfile1), mock.call(self.logfile2)] ) @@ -705,7 +827,7 @@ class TestRunSolver(HelperMixin, helpers.PungiTestCase): wc.call_args_list, [ mock.call(self.config1, [], ["foo", "pkg-devel"]), - mock.call(self.config2, [], ["foo", "foo.i686", "pkg-devel"]), + mock.call(self.config2, [], ["foo.i686"]), ], ) self.assertEqual( @@ -771,8 +893,8 @@ class TestExpandPackages(helpers.PungiTestCase): }, ) - def test_include_src_and_debuginfo(self): - nevra_to_pkg = self._mk_packages(debug_arch="x86_64") + def test_include_src(self): + nevra_to_pkg = self._mk_packages(src=True) res = hybrid.expand_packages( nevra_to_pkg, {}, [], [("pkg-3:1-2", "x86_64", [])], [] @@ -783,19 +905,19 @@ class TestExpandPackages(helpers.PungiTestCase): { "rpm": [{"path": "/tmp/pkg.rpm", "flags": []}], "srpm": [{"path": "/tmp/pkg.src.rpm", "flags": []}], - "debuginfo": [{"path": "/tmp/pkg-debuginfo.x86_64.rpm", "flags": []}], + "debuginfo": [], }, ) - def test_filter_src_and_debuginfo(self): - nevra_to_pkg = self._mk_packages(debug_arch="x86_64") + def test_filter_src(self): + nevra_to_pkg = self._mk_packages(src=True) res = hybrid.expand_packages( nevra_to_pkg, {}, [], [("pkg-3:1-2", "x86_64", [])], - filter_packages=[("pkg-debuginfo", "x86_64"), ("pkg", "src")], + filter_packages=[("pkg", "src")], ) self.assertEqual( @@ -807,48 +929,8 @@ class TestExpandPackages(helpers.PungiTestCase): }, ) - def test_filter_debuginfo_missing_arch(self): - nevra_to_pkg = self._mk_packages(debug_arch="x86_64") - - res = hybrid.expand_packages( - nevra_to_pkg, - {}, - [], - [("pkg-3:1-2", "x86_64", [])], - filter_packages=[("pkg-debuginfo", None)], - ) - - self.assertEqual( - res, - { - "rpm": [{"path": "/tmp/pkg.rpm", "flags": []}], - "srpm": [{"path": "/tmp/pkg.src.rpm", "flags": []}], - "debuginfo": [], - }, - ) - - def test_filter_debuginfo_different_arch(self): - nevra_to_pkg = self._mk_packages(debug_arch="x86_64") - - res = hybrid.expand_packages( - nevra_to_pkg, - {}, - [], - [("pkg-3:1-2", "x86_64", [])], - filter_packages=[("pkg-debuginfo", "aarch64")], - ) - - self.assertEqual( - res, - { - "rpm": [{"path": "/tmp/pkg.rpm", "flags": []}], - "srpm": [{"path": "/tmp/pkg.src.rpm", "flags": []}], - "debuginfo": [{"path": "/tmp/pkg-debuginfo.x86_64.rpm", "flags": []}], - }, - ) - - def test_modular_include_src_but_not_debuginfo(self): - nevra_to_pkg = self._mk_packages(debug_arch="x86_64") + def test_modular_include_src(self): + nevra_to_pkg = self._mk_packages(src=True) res = hybrid.expand_packages( nevra_to_pkg, {}, [], [("pkg-3:1-2", "x86_64", ["modular"])], [] @@ -879,25 +961,9 @@ class TestExpandPackages(helpers.PungiTestCase): }, ) - def test_skip_debuginfo_for_different_arch(self): - nevra_to_pkg = self._mk_packages(debug_arch="i686") - - res = hybrid.expand_packages( - nevra_to_pkg, {}, [], [("pkg-3:1-2", "x86_64", [])], [] - ) - - self.assertEqual( - res, - { - "rpm": [{"path": "/tmp/pkg.rpm", "flags": []}], - "srpm": [{"path": "/tmp/pkg.src.rpm", "flags": []}], - "debuginfo": [], - }, - ) - @mock.patch("pungi.phases.gather.methods.method_hybrid.cr") - def test_skip_lookaside_source_and_debuginfo(self, cr): - nevra_to_pkg = self._mk_packages(debug_arch="x86_64") + def test_skip_lookaside_source(self, cr): + nevra_to_pkg = self._mk_packages(src=True) lookasides = [mock.Mock()] repo = { "abc": NamedMock( @@ -906,12 +972,6 @@ class TestExpandPackages(helpers.PungiTestCase): location_base="file:///tmp/", location_href="pkg.src.rpm", ), - "def": NamedMock( - name="pkg-debuginfo", - arch="x86_64", - location_base="file:///tmp/", - location_href="pkg-debuginfo.x86_64.rpm", - ), } cr.Metadata.return_value.keys.return_value = repo.keys() cr.Metadata.return_value.get.side_effect = lambda key: repo[key]