Merge ~albertomilone/ubuntu/+source/software-properties:apt-pkg-transition into ubuntu/+source/software-properties:ubuntu/devel

[Alberto Milone]

This makes software-properties{gtk|qt} compatible with the new ubuntu-drivers-common, and its new apt_pkg code.

[Matthew Ruffell (mruffell)]

Hi Alberto,

I tried to build and test your patch, but it fails on the pyflakes3 test:

$ python3 tests/test_pyflakes.py
/home/matthew/Work/lp1964880/alberto/software-properties/softwareproperties/SoftwareProperties.py:29:1 'apt' imported but unused
/home/matthew/Work/lp1964880/alberto/software-properties/softwareproperties/SoftwareProperties.py:753:9 redefinition of unused 'apt' from line 29
E
======================================================================
ERROR: test_pyflakes3_clean (__main__.TestPyflakesClean)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/matthew/Work/lp1964880/alberto/software-properties/tests/test_pyflakes.py", line 21, in test_pyflakes3_clean
    self.assertEqual(subprocess.check_call(['pyflakes3'] + self.paths), 0)
  File "/usr/lib/python3.10/subprocess.py", line 369, in check_call
    raise CalledProcessError(retcode, cmd)

[Julian Andres Klode (juliank)]

I found a couple of spots worth fixing/considering in the diff

review: Needs Fixing

68577e2... by Alberto Milone on 2022-03-17

softwareproperties/SoftwareProperties.py: import apt only on demand

66473bd... by Alberto Milone on 2022-03-17

Do without a couple of tries, and make get_package_id return the installed package when available

[Alberto Milone (albertomilone)]

I have just pushed a couple of commits which should fix the problems reported by Julian and Matthew. Thanks

[Sebastien Bacher (seb128)]

Thanks, I've merged and uploaded but I can't close that one since it's targetting the wrong Vcs, for reference the git repository is reference in debian/control and https://git.launchpad.net/software-properties

I've also updated the depends on ubuntu-drivers-common since the updated code would error out with a pre-apt-refactoring version of the drivers

review: Approve

[Alberto Milone (albertomilone)]

I understand. Sorry about that. Thanks

Unmerged commits

66473bd... by Alberto Milone on 2022-03-17

Do without a couple of tries, and make get_package_id return the installed package when available

68577e2... by Alberto Milone on 2022-03-17

softwareproperties/SoftwareProperties.py: import apt only on demand

ada2244... by Alberto Milone on 2022-03-15

Use apt_pkg instead of apt, as in ubuntu-drivers-common

Fixes LP: #1964880

diff --git a/softwareproperties/SoftwareProperties.py b/softwareproperties/SoftwareProperties.py
index 3e94731..7fc9862 100644
--- a/softwareproperties/SoftwareProperties.py
+++ b/softwareproperties/SoftwareProperties.py
@@ -26,7 +26,6 @@
 from __future__ import absolute_import, print_function
 
 import apt_pkg
-import apt
 import copy
 from hashlib import md5
 import re
@@ -828,22 +827,35 @@ class SoftwareProperties(object):
     except:
       return False
 
-  def get_package_id(self, ver):
+  def get_package_id(self, apt_cache, pkg):
     """ Return the PackageKit package id """
-    assert isinstance(ver, apt.package.Version)
-    return "%s;%s;%s;" % (ver.package.shortname, ver.version,
-                          ver.package.architecture())
+    assert isinstance(pkg, apt_pkg.Package)
+    cur_ver = pkg.current_ver
+    if cur_ver:
+        ver = cur_ver.ver_str
+        arch = cur_ver.arch
+    else:
+        depcache = apt_pkg.DepCache(apt_cache)
+        candidate = depcache.get_candidate_ver(pkg)
+        ver = candidate.ver_str
+        arch = candidate.arch
+    return "%s;%s;%s;" % (pkg.name, ver, arch)
 
   @staticmethod
-  def get_dependencies(apt_cache, package_name, pattern=None):
+  def get_dependencies(apt_cache, package, pattern=None):
     """ Get the package dependencies, which can be filtered out by a pattern """
+    depcache = apt_pkg.DepCache(apt_cache)
+    candidate = depcache.get_candidate_ver(package)
+
     dependencies = []
-    for or_group in apt_cache[package_name].candidate.dependencies:
-      for dep in or_group:
-        if dep.rawtype in ["Depends", "PreDepends"]:
-          dependencies.append(dep.name)
-    if pattern:
-      dependencies = [ x for x in dependencies if x.find(pattern) != -1 ]
+    try:
+        for dep_list in candidate.depends_list_str.get('Depends'):
+            for dep_name, dep_ver, dep_op in dep_list:
+                if dep_name.find(pattern) != -1:
+                    dependencies.append(apt_cache[dep_name])
+    except (KeyError, TypeError):
+        return []
+
     return dependencies
 
 
diff --git a/softwareproperties/gtk/SoftwarePropertiesGtk.py b/softwareproperties/gtk/SoftwarePropertiesGtk.py
index 9b797ac..01e3d28 100644
--- a/softwareproperties/gtk/SoftwarePropertiesGtk.py
+++ b/softwareproperties/gtk/SoftwarePropertiesGtk.py
@@ -26,7 +26,6 @@
 
 from __future__ import absolute_import, print_function
 
-import apt
 import apt_pkg
 import datetime
 import dbus
@@ -228,7 +227,9 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
         # used to store the handlers of callbacks
         self.handlers = {}
 
-        self.apt_cache = {}
+        # Initialise and store the apt cache
+        self.init_apt_cache()
+
         self.pk_task = None
 
         # Put some life into the user interface:
@@ -290,6 +291,17 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
         self.handlers[self.combobox_release_upgrades] = \
              self.combobox_release_upgrades.connect("changed", 
                                                     self.on_combobox_release_upgrades_changed)
+    def init_apt_cache(self):
+        apt_pkg.init_config()
+        apt_pkg.init_system()
+        self.apt_cache = apt_pkg.Cache(None)
+        self.depcache = apt_pkg.DepCache(self.apt_cache)
+        self.records = apt_pkg.PackageRecords(self.apt_cache)
+
+    def update_apt_cache(self):
+        self.apt_cache = apt_pkg.Cache(None)
+        self.depcache = apt_pkg.DepCache(self.apt_cache)
+        self.records = apt_pkg.PackageRecords(self.apt_cache)
 
     def init_auto_update(self):
         """ Set up the widgets that allow to configure the update automation """
@@ -983,7 +995,7 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
     # dbus events
     def on_config_modified(self):
         """The config was changed and now needs to be saved and reloaded"""
-        apt.apt_pkg.init_config()
+        apt_pkg.init_config()
         self.button_revert.set_sensitive(True)
 
     def on_keys_modified(self):
@@ -1226,7 +1238,7 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
         if not installs_pending:
             self.progress_bar.set_visible(False)
             self.clear_changes()
-            self.apt_cache = apt.Cache()
+            self.update_apt_cache()
             self.set_driver_action_status()
             self.update_label_and_icons_from_status()
             self.button_driver_revert.set_visible(True)
@@ -1239,19 +1251,20 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
         installs = []
         removals = []
 
+        has_nvidia = False
         for pkg in self.driver_changes:
-            if pkg.is_installed:
-                removals.append(self.get_package_id(pkg.installed))
+            if pkg.current_ver:
+                removals.append(self.get_package_id(self.apt_cache, pkg))
                 # The main NVIDIA package is only a metapackage.
                 # We need to collect its dependencies, so that
                 # we can uninstall the driver properly.
-                if 'nvidia' in pkg.shortname:
-                    for dep in self.get_dependencies(self.apt_cache, pkg.shortname, 'nvidia'):
-                        dep_pkg = self.apt_cache[dep]
-                        if dep_pkg.is_installed:
-                            removals.append(self.get_package_id(dep_pkg.installed))
+                if 'nvidia' in pkg.name:
+                    has_nvidia = True
+                    for dep in self.get_dependencies(self.apt_cache, pkg, 'nvidia'):
+                        if dep.current_ver:
+                            removals.append(self.get_package_id(self.apt_cache, dep))
             else:
-                installs.append(self.get_package_id(pkg.candidate))
+                installs.append(self.get_package_id(self.apt_cache, pkg))
 
         self.cancellable = Gio.Cancellable()
         try:
@@ -1269,6 +1282,11 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
                             installs_pending  # callback data
                  )
             if installs:
+                if has_nvidia:
+                    to_install = []
+                    for item in installs:
+                        name = item.split(';')[0]
+                        to_install.append(name)
                 self.pk_task.install_packages_async(installs,
                         self.cancellable,  # cancellable
                         self.on_driver_changes_progress,
@@ -1376,7 +1394,6 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
         # WARNING: This is run in a separate thread.
         self.detect_called = True
         try:
-            self.apt_cache = apt.Cache()
             self.devices = detect.system_device_drivers(self.apt_cache)
         except:
             # Catch all exceptions and feed them to apport.
@@ -1395,22 +1412,16 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
             return
 
         pkg = None
-        try:
-            if pkg_name:
-                pkg = self.apt_cache[pkg_name]
-                # If the package depends on dkms
-                # we need to install the correct linux metapackage
-                # so that users get the latest headers
-                if 'dkms' in pkg.candidate.record['Depends']:
-                    linux_meta = detect.get_linux(self.apt_cache)
-                    if (linux_meta and
-                        linux_meta not in self.driver_changes):
-                        # Install the linux metapackage
-                        lmp = self.apt_cache[linux_meta]
-                        if not lmp.is_installed:
-                            self.driver_changes.append(lmp)
-        except (AttributeError, KeyError):
-            pass
+        if pkg_name:
+            pkg = self.apt_cache[pkg_name]
+            # Add the matching linux modules package when available
+            try:
+                modules_package = detect.get_linux_modules_metapackage(self.apt_cache, pkg_name)
+                modules_package_obj = self.apt_cache[modules_package]
+                if modules_package and not modules_package_obj.current_ver:
+                    self.driver_changes.append(modules_package_obj)
+            except KeyError:
+                pass
 
         if button.get_active():
             if pkg in self.driver_changes:
@@ -1430,7 +1441,7 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
 
             if (pkg is not None
                 and pkg not in self.driver_changes
-                    and pkg.is_installed):
+                    and pkg.current_ver):
                 self.driver_changes.append(pkg)
 
         self.button_driver_revert.set_sensitive(bool(self.driver_changes))
@@ -1492,11 +1503,14 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
 
             try:
                 pkg = self.apt_cache[pkg_driver_name]
-                installed = pkg.is_installed
-                if pkg.candidate is not None:
-                    description = _("Using {} from {}").format(pkg.candidate.summary, pkg.shortname)
+                installed = pkg.current_ver
+                candidate = self.depcache.get_candidate_ver(pkg)
+
+                if candidate is not None:
+                    self.records.lookup(candidate.file_list[0])
+                    description = _("Using {} from {}").format(self.records.short_desc, pkg_driver_name)
                 else:
-                    description = _("Using {}").format(pkg.shortname)
+                    description = _("Using {}").format(pkg_driver_name)
             except KeyError:
                 print("WARNING: a driver ({}) doesn't have any available package associated: {}".format(pkg_driver_name, current_driver))
                 continue
@@ -1626,7 +1640,7 @@ class SoftwarePropertiesGtk(SoftwareProperties, SimpleGtkbuilderApp):
         for device in self.devices:
             for pkg_name in self.devices[device]['drivers']:
                 pkg = self.apt_cache[pkg_name]
-                if not self.devices[device]['drivers'][pkg_name]['free'] and pkg.is_installed:
+                if not self.devices[device]['drivers'][pkg_name]['free'] and pkg.current_ver:
                     self.nonfree_drivers = self.nonfree_drivers + 1
 
         if self.nonfree_drivers > 0:
diff --git a/softwareproperties/qt/SoftwarePropertiesQt.py b/softwareproperties/qt/SoftwarePropertiesQt.py
index 332bf06..e9e5818 100644
--- a/softwareproperties/qt/SoftwarePropertiesQt.py
+++ b/softwareproperties/qt/SoftwarePropertiesQt.py
@@ -55,7 +55,6 @@ from .CdromProgress import CdromProgress
 
 from UbuntuDrivers import detect
 import sys
-import apt
 from functools import partial
 import aptsources.distro
 import logging
@@ -345,6 +344,18 @@ class SoftwarePropertiesQt(SoftwareProperties):
     self.userinterface.combobox_server.addItem(_("Other..."))
     self.other_mirrors_index = self.userinterface.combobox_server.count() - 1
 
+  def init_apt_cache(self):
+    apt_pkg.init_config()
+    apt_pkg.init_system()
+    self.apt_cache = apt_pkg.Cache(None)
+    self.depcache = apt_pkg.DepCache(self.apt_cache)
+    self.records = apt_pkg.PackageRecords(self.apt_cache)
+
+  def update_apt_cache(self):
+    self.apt_cache = apt_pkg.Cache(None)
+    self.depcache = apt_pkg.DepCache(self.apt_cache)
+    self.records = apt_pkg.PackageRecords(self.apt_cache)
+
   def show_distro(self):
     """
     Represent the distro information in the user interface
@@ -736,7 +747,7 @@ class SoftwarePropertiesQt(SoftwareProperties):
 
   def on_restore_clicked(self):
     """Restore the original keys"""
-    self.apt_key.update()
+    self.update_apt_cache()
     self.show_keys()
 
   def on_pktask_progress(self, progress, ptype, udata=(None,)):
@@ -859,7 +870,7 @@ class SoftwarePropertiesQt(SoftwareProperties):
     if not installs_pending:
       self.progress_bar.setVisible(False)
       self.clear_changes()
-      self.apt_cache = apt.Cache()
+      self.update_apt_cache()
       self.set_driver_action_status()
       self.update_label_and_icons_from_status()
       self.button_driver_revert.setVisible(True)
@@ -885,19 +896,20 @@ class SoftwarePropertiesQt(SoftwareProperties):
     installs = []
     removals = []
 
+    has_nvidia = False
     for pkg in self.driver_changes:
-      if pkg.is_installed:
-          removals.append(self.get_package_id(pkg.installed))
+      if pkg.current_ver:
+          removals.append(self.get_package_id(self.apt_cache, pkg))
           # The main NVIDIA package is only a metapackage.
           # We need to collect its dependencies, so that
           # we can uninstall the driver properly.
-          if 'nvidia' in pkg.shortname:
-            for dep in self.get_dependencies(self.apt_cache, pkg.shortname, 'nvidia'):
-              dep_pkg = self.apt_cache[dep]
-              if dep_pkg.is_installed:
-                removals.append(self.get_package_id(dep_pkg.installed))
+          if 'nvidia' in pkg.name:
+            has_nvidia = True
+            for dep in self.get_dependencies(self.apt_cache, pkg, 'nvidia'):
+              if dep.current_ver:
+                removals.append(self.get_package_id(self.apt_cache, dep))
       else:
-        installs.append(self.get_package_id(pkg.candidate))
+        installs.append(self.get_package_id(self.apt_cache, pkg))
 
     self.cancellable = Gio.Cancellable()
     try:
@@ -915,6 +927,11 @@ class SoftwarePropertiesQt(SoftwareProperties):
                   installs_pending  # callback data
          )
       if installs:
+        if has_nvidia:
+          to_install = []
+          for item in installs:
+            name = item.split(';')[0]
+            to_install.append(name)
         self.pk_task.install_packages_async(installs,
               self.cancellable,  # cancellable
               self.on_driver_changes_progress,
@@ -1019,8 +1036,8 @@ class SoftwarePropertiesQt(SoftwareProperties):
   def detect_drivers(self):
     # WARNING: This is run in a separate thread.
     self.detect_called = True
+    self.init_apt_cache()
     try:
-      self.apt_cache = apt.Cache()
       self.devices = detect.system_device_drivers(self.apt_cache)
     except:
       # Catch all exceptions and feed them to apport.
@@ -1039,21 +1056,17 @@ class SoftwarePropertiesQt(SoftwareProperties):
       return
 
     pkg = None
-    try:
-      if pkg_name:
-        pkg = self.apt_cache[pkg_name]
-        # If the package depends on dkms
-        # we need to install the correct linux metapackage
-        # so that users get the latest headers
-        if 'dkms' in pkg.candidate.record['Depends']:
-          linux_meta = detect.get_linux(self.apt_cache)
-          if (linux_meta and linux_meta not in self.driver_changes):
-            # Install the linux metapackage
-            lmp = self.apt_cache[linux_meta]
-            if not lmp.is_installed:
-              self.driver_changes.append(lmp)
-    except KeyError:
-      pass
+
+    if pkg_name:
+      pkg = self.apt_cache[pkg_name]
+      # Add the matching linux modules package when available
+      try:
+        modules_package = detect.get_linux_modules_metapackage(self.apt_cache, pkg_name)
+        modules_package_obj = self.apt_cache[modules_package]
+        if modules_package and not modules_package_obj.current_ver:
+          self.driver_changes.append(modules_package_obj)
+      except KeyError:
+          pass
 
     if button.isChecked():
       if pkg in self.driver_changes:
@@ -1069,7 +1082,7 @@ class SoftwarePropertiesQt(SoftwareProperties):
       if modalias not in self.orig_selection:
           self.orig_selection[modalias] = button
 
-      if (pkg is not None and pkg not in self.driver_changes and pkg.is_installed):
+      if (pkg is not None and pkg not in self.driver_changes and pkg.current_ver):
           self.driver_changes.append(pkg)
 
     self.button_driver_revert.setEnabled(bool(self.driver_changes))
@@ -1137,11 +1150,13 @@ None if not applicable
 
       try:
         pkg = self.apt_cache[pkg_driver_name]
-        installed = pkg.is_installed
-        if pkg.candidate is not None:
-          description = _("Using {} from {}").format(pkg.candidate.summary, pkg.shortname)
+        installed = pkg.current_ver or False
+        candidate = self.depcache.get_candidate_ver(pkg)
+        if candidate is not None:
+          self.records.lookup(candidate.file_list[0])
+          description = _("Using {} from {}").format(self.records.short_desc, pkg_driver_name)
         else:
-          description = _("Using {}").format(pkg.shortname)
+          description = _("Using {}").format(pkg_driver_name)
       except KeyError:
         print("WARNING: a driver ({}) doesn't have any available package associated: {}".format(pkg_driver_name, current_driver))
         continue
@@ -1277,7 +1292,7 @@ licence=licence)
     for device in self.devices:
       for pkg_name in self.devices[device]['drivers']:
         pkg = self.apt_cache[pkg_name]
-        if not self.devices[device]['drivers'][pkg_name]['free'] and pkg.is_installed:
+        if not self.devices[device]['drivers'][pkg_name]['free'] and pkg.current_ver:
           self.nonfree_drivers = self.nonfree_drivers + 1
 
     if self.nonfree_drivers > 0: