hybrid: Refactor handling debuginfo packages

Behavior before this change: debuginfo was added based on source RPMs.
Pungi would get all debuginfo for all architectures that include at
least one binary package. This had a consequence that if foo-x.x86_64
and foo-x.i686 from the same built were included, foo-y-debuginfo.i686
would be included despite foo-y only being present for x86_64.

The patch changes this to work on binary package level: for each
included package `x`, check if there is `x-debuginfo` or
`x-debugsource`, and include them. Packages added in this way have to go
through one iteration of the solver, to account for cases such as
`x-libs-debuginfo` depending on `x-debuginfo` (with only `x` starting
this chain).

To make it slightly faster, the invocation of fus is changed to only
process newly added package in each iteration. It should have no effect
on the results, and subsequent iterations are much faster if they don't
process the same stuff again and again.

JIRA: COMPOSE-3247
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
This commit is contained in:
Lubomír Sedlář 2019-02-27 14:30:01 +01:00
parent b5bef68c60
commit b80efbfd97
2 changed files with 248 additions and 147 deletions

View File

@ -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))

View File

@ -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]