From 39962fcab9400a3e3c1aec4868e875a74b36e3b0 Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Wed, 12 Mar 2025 17:52:20 +0100 Subject: [PATCH 1/2] packaging: add a method to get installed binaries for a source Will be useful to fix apport-collect --- apport/packaging.py | 6 ++++++ apport/packaging_impl/apt_dpkg.py | 10 +++++++++- tests/integration/test_packaging_apt_dpkg.py | 11 +++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/apport/packaging.py b/apport/packaging.py index 2fda8e2a2..76d2a150b 100644 --- a/apport/packaging.py +++ b/apport/packaging.py @@ -55,6 +55,12 @@ def get_source(self, package: str) -> str: "this method must be implemented by a concrete subclass" ) + def get_installed_binaries(self, source_package: str) -> set[str]: + """Return all installed binary packages for a given source.""" + raise NotImplementedError( + "this method must be implemented by a concrete subclass" + ) + def get_package_origin(self, package: str) -> str | None: """Return package origin. diff --git a/apport/packaging_impl/apt_dpkg.py b/apport/packaging_impl/apt_dpkg.py index ad06b570b..3c03759c6 100644 --- a/apport/packaging_impl/apt_dpkg.py +++ b/apport/packaging_impl/apt_dpkg.py @@ -360,7 +360,7 @@ def _clear_apt_cache(self) -> None: self._apt_cache = None self._sandbox_apt_cache = None - def _cache(self): + def _cache(self) -> apt.Cache: """Return apt.Cache() (initialized lazily).""" if not self._apt_cache: self._clear_apt_cache() @@ -444,6 +444,14 @@ def get_source(self, package: str) -> str: return self._apt_pkg(package).candidate.source_name raise ValueError(f"package {package} does not exist") + def get_installed_binaries(self, source_package: str) -> set[str]: + """Return all installed binary packages for a given source.""" + return { + pkg.name + for pkg in self._cache() + if pkg.installed and (source_package == pkg.installed.source_name) + } + def get_package_origin(self, package: str) -> str | None: """Return package origin. diff --git a/tests/integration/test_packaging_apt_dpkg.py b/tests/integration/test_packaging_apt_dpkg.py index 60099b516..236d3b1fc 100644 --- a/tests/integration/test_packaging_apt_dpkg.py +++ b/tests/integration/test_packaging_apt_dpkg.py @@ -116,6 +116,17 @@ def test_get_source(self) -> None: self.assertEqual(impl.get_source("bash"), "bash") self.assertIn("glibc", impl.get_source("libc6")) + def test_get_installed_binaries_library(self) -> None: + binaries = impl.get_installed_binaries("glibc") + self.assertIn("libc6", binaries) + self.assertNotIn("glibc", binaries) + + def test_get_installed_binaries_binary(self) -> None: + self.assertIn("bash", impl.get_installed_binaries("bash")) + + def test_get_installed_binaries_none(self) -> None: + self.assertEqual(set(impl.get_installed_binaries("nonexisting")), set()) + def test_get_package_origin(self) -> None: """get_package_origin().""" # determine distro name From 1dc663b985a3fefb5591e72a4e4f3ffe7a12594f Mon Sep 17 00:00:00 2001 From: Simon Chopin Date: Wed, 12 Mar 2025 18:08:38 +0100 Subject: [PATCH 2/2] ui: update_report: fix source/binary confusion Rather than relying on binary packages being named the same as their source package, we actually split the two of them. There's an underlying assumption that we're reporting to a backend that cares about source packages, so that's what we're basing it all on, by adding a field to list the binary packages actually installed from that source package. It might not work perfectly (the Package field will be marked with "not installed" for glibc, for instance), but it's still much better than the current state. Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/apport/+bug/2102147 --- apport/ui.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/apport/ui.py b/apport/ui.py index 02625972e..fd2183f2e 100644 --- a/apport/ui.py +++ b/apport/ui.py @@ -777,27 +777,38 @@ def run_update_report(self) -> bool: # list of affected source packages self.report = apport.Report("Bug") + # FIXME: for now we assume this is a binary package because it's just easier + # and it didn't work with a source package anyway. if self.args.package: - pkgs = [self.args.package.strip()] + binary = self.args.package.strip() + pkgs = {packaging.get_source(binary): set((binary,))} else: - pkgs = self.crashdb.get_affected_packages(self.args.update_report) + pkgs = { + src: packaging.get_installed_binaries(src) + for src in self.crashdb.get_affected_packages(self.args.update_report) + } info_collected = False - for p in pkgs: + assert self.report is not None + for src, binaries in pkgs.items(): # print(f"Collecting apport information for source package {p}...") - self.cur_package = p - self.report["SourcePackage"] = p - self.report["Package"] = p # no way to find this out + assert binaries + self.cur_package = src + self.report["SourcePackage"] = src + self.report["Package"] = src # Slight abuse of the field but it's mandatory + self.report["InstalledBinaries"] = " ".join(sorted(binaries)) # we either must have the package installed or a source package # hook available to collect sensible information try: - packaging.get_version(p) + packaging.get_version(next(iter(binaries))) except ValueError: if not os.path.exists( - os.path.join(apport.report.PACKAGE_HOOK_DIR, f"source_{p}.py") + os.path.join(apport.report.PACKAGE_HOOK_DIR, f"source_{src}.py") ): - print(f"Package {p} not installed and no hook available, ignoring") + print( + f"Package {src} not installed and no hook available, ignoring" + ) continue self.collect_info(ignore_uninstalled=True) info_collected = True