Merge lp:~jawn-smith/update-manager/package-removal-grouping into lp:update-manager

[William Wilson]

LP: #1912718 disabling ubuntu base grouping for kernel autoremove and duplicate packages

The bug in LP: #1912718 was caused by having the kernel packages grouped under kernel_autoremove->ubuntu_base rather than just kernel_autoremove. With this update the packages will not be grouped under Ubuntu Base if they are staged for removal, but they still will be grouped under Ubuntu Base if they are staged for install/upgrade.

In response to the comment: "I don't know what the right answer for bug 1902025 is but certainly matching all kernel packages when we're trying to find meta packages isn't":
The variable names meta_pkgs and meta_grp were misleading. In the original code before the change for bug 1902025 there was a comment for the included packages that said "Return all binary packages made by the linux-meta source package". While working this we decided to include more than just the packages created by linux-meta. We have expanded the "Ubuntu Base" grouping to include packages created by linux-meta-aws as well as packages such as linux-firmware-raspi2. I have renamed the variable to ubuntu_base_pkgs to make that more clear.

[Brian Murray (brian-murray)]

Thanks for renaming the variables I think that does make it clearer. I've a few in-line comments.

review: Needs Fixing

[Brian Murray (brian-murray)]

Is there a way that we can setup an automated test for LP: #1912718?

review: Needs Information

[lp:~jawn-smith/update-manager/package-removal-grouping]

2912. By William Wilson on 2021-02-05

LP: #1902025 change to a regex from a static list for packages to group under Ubuntu Base

[William Wilson (jawn-smith)]

I added an automated test for LP: #1912718 but it appears I have a merge conflict in changelog, so hold off on reviewing for now.

[lp:~jawn-smith/update-manager/package-removal-grouping]

2913. By William Wilson on 2021-02-05

Removing commented out debugging line from test

2914. By William Wilson on 2021-02-05

resolving merge conflict and deleting extra teardown

2915. By William Wilson on 2021-02-05

Adding tearDown function back in that was deleted to resolve conflict

[Brian Murray (brian-murray)]

Thanks for adding those additional tests, I've merged and upload this.

review: Approve

=== modified file 'UpdateManager/Core/UpdateList.py'
--- UpdateManager/Core/UpdateList.py	2021-01-29 11:39:00 +0000
+++ UpdateManager/Core/UpdateList.py	2021-02-05 22:13:57 +0000
@@ -36,6 +36,7 @@
 import os
 import random
 import glob
+import re
 
 from gi.repository import Gio
 
@@ -384,43 +385,6 @@
                 return True
         return False
 
-    def _get_linux_packages(self):
-        "Return all binary packages made by the linux-meta source package"
-        # Hard code this rather than generate from source info in cache because
-        # that might only be available if we have deb-src lines.  I think we
-        # could also generate it by iterating over all the binary package info
-        # we have, but that is costly.  These don't change often.
-        return ['linux',
-                'linux-cloud-tools-generic',
-                'linux-cloud-tools-lowlatency',
-                'linux-cloud-tools-virtual',
-                'linux-crashdump',
-                'linux-generic',
-                'linux-generic-lpae',
-                'linux-headers-generic',
-                'linux-headers-generic-lpae',
-                'linux-headers-lowlatency',
-                'linux-headers-lowlatency-lpae',
-                'linux-headers-server',
-                'linux-headers-virtual',
-                'linux-image',
-                'linux-image-extra-virtual',
-                'linux-image-generic',
-                'linux-image-generic-lpae',
-                'linux-image-lowlatency',
-                'linux-image-virtual',
-                'linux-lowlatency',
-                'linux-signed-generic',
-                'linux-signed-image-generic',
-                'linux-signed-image-lowlatency',
-                'linux-signed-lowlatency',
-                'linux-source',
-                'linux-tools-generic',
-                'linux-tools-generic-lpae',
-                'linux-tools-lowlatency',
-                'linux-tools-virtual',
-                'linux-virtual']
-
     def _make_groups(self, cache, pkgs, eventloop_callback, to_remove=False):
         if not pkgs:
             return []
@@ -452,15 +416,41 @@
         if ungrouped_pkgs:
             # Separate out system base packages. If we have already found an
             # application for all updates, don't bother.
-            meta_group = UpdateGroup(None, None, None, to_remove)
+            linux_names = ("linux$",
+                           "linux-.*-buildinfo.*",
+                           "linux-.*-dev.*",
+                           "linux-.*-generic.*",
+                           "linux-.*-headers.*",
+                           "linux-.*-hwe.*",
+                           "linux-.*-gcp.*",
+                           "linux-.*-kernel.*",
+                           "linux-.*-lowlatency.*",
+                           "linux-.*-modules.*",
+                           "linux-.*-raspi.*",
+                           "linux-.*-tools.*",
+                           "linux-.*-virtual.*",
+                           "linux-base.*",
+                           "linux-crashdump.*",
+                           "linux-doc.*")
+            linux_regexp = re.compile("(" + "|".join(
+                ["^" + n for n in linux_names]) + ")")
+            ubuntu_base_group = UpdateGroup(None, None, None, to_remove)
             flavor_package = utils.get_ubuntu_flavor_package(cache=cache)
-            meta_pkgs = [flavor_package, "ubuntu-standard", "ubuntu-minimal"]
-            meta_pkgs.extend(self._get_linux_packages())
-            for pkg in meta_pkgs:
-                if pkg in cache:
-                    meta_group.add(cache[pkg])
+            ubuntu_base_pkgs = [flavor_package,
+                                "ubuntu-standard",
+                                "ubuntu-minimal"]
+            for pkg in cache:
+                if linux_regexp.match(pkg.name):
+                    ubuntu_base_pkgs.append(pkg.name)
+            for pkg in ubuntu_base_pkgs:
+                if pkg in cache and not to_remove:
+                    # the check for to_remove avoids creating groups in
+                    # kernel_autoremove and duplicate package sections
+                    # which fixes LP: #1912718
+                    ubuntu_base_group.add(cache[pkg])
             for pkg in ungrouped_pkgs:
-                if meta_group.is_dependency(pkg, cache, eventloop_callback):
+                if ubuntu_base_group.is_dependency(pkg, cache,
+                                                   eventloop_callback):
                     if system_group is None:
                         system_group = UpdateSystemGroup(cache, to_remove)
                     system_group.add(pkg)

=== modified file 'debian/changelog'
--- debian/changelog	2021-02-04 18:44:07 +0000
+++ debian/changelog	2021-02-05 22:13:57 +0000
@@ -1,7 +1,14 @@
 update-manager (1:21.04.8) UNRELEASED; urgency=medium
 
+  [ Brian Murray ]
   * Clean up apt cache binary files created by the unit tests thanks to
     William Wilson for the patch.
+  
+  [ William Wilson ]
+  * Changing to a regex from a static list of packages to be included under
+    "Ubuntu Base". This regex exands the list of packages under "Ubuntu Base"
+    to include all kernel packages as well as binary packages built by
+    linux-meta source packages (LP: #1902025)
 
  -- Brian Murray <[e-mail]>  Thu, 04 Feb 2021 10:42:55 -0800
 

=== modified file 'tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_main_binary-amd64_Packages'
--- tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_main_binary-amd64_Packages	2021-01-29 11:39:00 +0000
+++ tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_main_binary-amd64_Packages	2021-02-05 22:13:57 +0000
@@ -78,3 +78,27 @@
 Recommends: base-pkg
 Description: meta package
 Origin: Ubuntu
+
+Package: linux-test-hwe
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 1
+Size: 1
+Recommends: base-pkg
+Description: positive test for linux package names
+Origin: Ubuntu
+
+Package: linux-show-player
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 1
+Size: 1
+Recommends: base-pkg
+Description: negative test for linux package names
+Origin: Ubuntu

=== modified file 'tests/aptroot-grouping-test/var/lib/dpkg/status'
--- tests/aptroot-grouping-test/var/lib/dpkg/status	2021-01-29 11:39:00 +0000
+++ tests/aptroot-grouping-test/var/lib/dpkg/status	2021-02-05 22:13:57 +0000
@@ -80,3 +80,73 @@
 Architecture: all
 Version: 0.1
 Description: an installed package that only one app depends on
+
+Package: ubuntu-minimal
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: meta package
+
+Package: linux-test-hwe
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: positive test for linux package names
+
+Package: linux-show-player
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: negative test for linux package names
+
+Package: linux-generic-to-remove
+Status: install ok installed
+Priority: optional
+Section: kernel
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: linux kernel autoremove packages that should not be grouped
+
+Package: linux-headers-to-remove
+Status: install ok installed
+Priority: optional
+Section: kernel
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: linux kernel autoremove packages that should not be grouped
+
+Package: linux-generic-dup1
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: duplicate package that should not be grouped
+
+Package: linux-generic-dup2
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: duplicate package that should not be grouped

=== modified file 'tests/test_update_list.py'
--- tests/test_update_list.py	2021-02-04 18:44:07 +0000
+++ tests/test_update_list.py	2021-02-05 22:13:57 +0000
@@ -119,15 +119,28 @@
         apt.apt_pkg.config.set("APT::Architecture", "amd64")
         self.addCleanup(
             lambda: apt.apt_pkg.config.set("APT::Architecture", real_arch))
+
+        # override the kernel autoremove regex
+        real_versioned_kernel_pkgs = apt.apt_pkg.config.value_list(
+            "APT::VersionedKernelPackages")
+        apt.apt_pkg.config.set("APT::VersionedKernelPackages", "linux-.*")
+        self.addCleanup(
+            lambda: apt.apt_pkg.config.set("APT::VersionedKernelPackages",
+                                           str(real_versioned_kernel_pkgs)))
+
         self.aptroot = os.path.join(CURDIR,
                                     "aptroot-grouping-test")
         self.cache = MyCache(apt.progress.base.OpProgress(),
                              rootdir=self.aptroot)
         self.cache.open()
+        self.cache["linux-generic-to-remove"].mark_auto(True)
+        self.cache["linux-headers-to-remove"].mark_auto(True)
         mock_installed.__get__ = self.fake_installed_files
         mock_desktop.side_effect = self.fake_desktop
         self.updates_list = UpdateList.UpdateList(parent=None, dist='lucid')
-        self.updates_list.update(self.cache)
+        self.updates_list.update(self.cache,
+                                 duplicate_packages=["linux-generic-dup1",
+                                                     "linux-generic-dup2"])
 
     def tearDown(self):
         try:
@@ -181,9 +194,18 @@
         self.assertListEqual([x.pkg.name for x in group.items],
                              ['installed-pkg'])
 
+    def test_negative_regex_match(self):
+        self.assertGreater(len(self.updates_list.update_groups), 3)
+        group = self.updates_list.update_groups[3]
+        self.assertIsInstance(group, UpdateList.UpdatePackageGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertEqual(group.core_item.pkg.name, 'linux-show-player')
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['linux-show-player'])
+
     def test_pkg_multiple_deps(self):
-        self.assertEqual(len(self.updates_list.update_groups), 4)
-        group = self.updates_list.update_groups[3]
+        self.assertGreater(len(self.updates_list.update_groups), 4)
+        group = self.updates_list.update_groups[4]
         self.assertIsInstance(group, UpdateList.UpdatePackageGroup)
         self.assertIsNotNone(group.core_item)
         self.assertEqual(group.core_item.pkg.name,
@@ -198,6 +220,49 @@
         self.assertIsNone(group.core_item)
         self.assertListEqual([x.pkg.name for x in group.items], ['base-pkg'])
 
+    def test_base_grouping(self):
+        self.assertEqual(len(self.updates_list.update_groups), 6)
+        group = self.updates_list.update_groups[5]
+        self.assertIsInstance(group, UpdateList.UpdateSystemGroup)
+        self.assertIsNone(group.core_item)
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['ubuntu-minimal',
+                              'linux-test-hwe'])
+
+    def test_autoremove_grouping(self):
+        # test that packages staged for removal aren't grouped if they are
+        # kernel autoremove packages (LP: #1912718)
+        self.assertGreater(len(self.updates_list.kernel_autoremove_groups), 0)
+        group = self.updates_list.kernel_autoremove_groups[0]
+        self.assertIsInstance(group, UpdateList.UpdatePackageGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['linux-generic-to-remove'])
+
+        self.assertEqual(len(self.updates_list.kernel_autoremove_groups), 2)
+        group = self.updates_list.kernel_autoremove_groups[1]
+        self.assertIsInstance(group, UpdateList.UpdatePackageGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['linux-headers-to-remove'])
+
+    def test_duplicate_grouping(self):
+        # test that packages staged for removal aren't grouped if they are
+        # duplicate packages (LP: #1912718)
+        self.assertGreater(len(self.updates_list.duplicate_groups), 0)
+        group = self.updates_list.duplicate_groups[0]
+        self.assertIsInstance(group, UpdateList.UpdatePackageGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['linux-generic-dup1'])
+
+        self.assertEqual(len(self.updates_list.duplicate_groups), 2)
+        group = self.updates_list.duplicate_groups[1]
+        self.assertIsInstance(group, UpdateList.UpdatePackageGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['linux-generic-dup2'])
+
 
 if __name__ == "__main__":
     unittest.main()