Merge lp:~marcustomlinson/update-manager/update-manager into lp:update-manager

[Marcus Tomlinson]

UpdateManager/backend/__init__.py: once apt upgrade completes, use the deb2snap.json file from ubuntu-release-upgrader to perform snap updates.

[Iain Lane (laney)]

Free-form thoughts

  - This causes polkit prompts from snapd, is that acceptable?
  - I'm not sure that putting it after apt updating and before apt installing is right - what if we:
    - update ubuntu-release-upgrader-core (new transitions to know about)
    - Change the seeds (things get marked as removable that weren't before)
    ?
  - Why is there a spawned thread and how is it supposed to be waited on? (Could be fine, I just can't tell)
  - Would probably be better to use snapd-glib

I'll probably follow up with more later!

[Iain Lane (laney)]

review: Needs Fixing

[Iain Lane (laney)]

I also do not see where update-manager performs autoremoval; does that actually exist? If not, we would need to do the apt removal here too (we already identified the package and that it's autoremovable, so should be possible).

[Marcus Tomlinson (marcustomlinson)]

Hey Iain, feeling a bit ill so forgive me if I ramble or make no sense anywhere. I'll try explain myself as best as I can.

The reason I'm doing the transitions immediately after update is to ensure everyone gets them, even apt users. I, for example, will usually just close update-manager whenever it prompts and perform my updates via terminal when I feel like it. Transitioning before the prompt is - granted - a bit of a hack to force everyone to get them (I don't know who wouldn't want them really).

I don't actually see the autoremoval as part of this logic. Once we update the seeds on desktop-minimal that propagates to the deb being marked for removal and thus update-manager and apt do their thing as usual. This is why I can do the cache[deb].marked_delete check the way I am, as that is my indication that the deb has been unseeded, is currently installed, and should be removed.

[Iain Lane (laney)]

"update-manager [...] do their thing as usual"

Where is that in update-manager though? It could happen, but I cannot find the code for it - except for kernel autoremovals.

Not commenting on the rest for now.

[Sebastien Bacher (seb128)]

> I feel like it. Transitioning before the prompt is -
> granted - a bit of a hack to force everyone to get them
> (I don't know who wouldn't want them really).

Commenting just on that but I think that's a weird logic/user experience, we shouldn't get out of our way doing weird UI just to force technical users to be force migrated. Those who close update-manager and go to the command line should be able to transition between snap&deb by themselve

[Ken VanDine (ken-vandine)]

>
> The reason I'm doing the transitions immediately after update is to ensure
> everyone gets them, even apt users. I, for example, will usually just close
> update-manager whenever it prompts and perform my updates via terminal when I
> feel like it. Transitioning before the prompt is - granted - a bit of a hack
> to force everyone to get them (I don't know who wouldn't want them really).

I don't think we should actually do the transition before the user actually clicks on the button in the UI. Doing it without the user interaction there feels wrong to me.

[Marcus Tomlinson (marcustomlinson)]

Ok I misunderstood what we were aiming for here. Apologies.

[Steve Langasek (vorlon)]

> I also do not see where update-manager performs autoremoval;
> does that actually exist? If not, we would need to do the apt
> removal here too (we already identified the package and that
> it's autoremovable, so should be possible).

We should not, as a policy, do autoremovals of other debs as part of an ordinary update-manager run because we don't know why the package has become autoremovable and this can significantly impact the usability of the user's system.

We do autoremovals only on upgrade between releases, because that's a significant event where the possibility of some fallout is to be expected.

[Iain Lane (laney)]

On Wed, Mar 18, 2020 at 03:08:13PM -0000, Steve Langasek wrote:
> > I also do not see where update-manager performs autoremoval;
> > does that actually exist? If not, we would need to do the apt
> > removal here too (we already identified the package and that
> > it's autoremovable, so should be possible).
>
> We should not, as a policy, do autoremovals of other debs as part of an ordinary update-manager run because we don't know why the package has become autoremovable and this can significantly impact the usability of the user's system.

This is specifically limited to cases where we know a deb has been
replaced by a snap.

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[Iain Lane (laney)]

I'll look into the requested changes.

[Iain Lane (laney)]

> I'll look into the requested changes.

Struggling to find time for this :(

[Marcus Tomlinson (marcustomlinson)]

Alright, changes made:

- I've split the large update_snaps() function into smaller parts.
- Snap updates now occur _after_ the user has opted in to the updates.
- snapd-glib for (un)installs means just one polkit prompt now for the all snap updates.
- debs with a snap replacement that were not installed manually get marked for deletion.
- The thread is still there, sorry, though I'm not sure why it's a bad idea really.

[Marcus Tomlinson (marcustomlinson)]

Sorry, just pushed another update to fix the pycodestyle test.

[Iain Lane (laney)]

> - The thread is still there, sorry, though I'm not sure why it's a bad idea really.

Usual GLib style is to use async functions and make use of the main loop, then you can avoid caring about issues with thread safety. In this case that was more of a style thing / observation / comment, not a blocker for me though, so reviewing now. Thanks for the updates!

[Marcus Tomlinson (marcustomlinson)]

Bug updated to FFe and testing instructions added.

[lp:~marcustomlinson/update-manager/update-manager]

2865. By Marcus Tomlinson on 2020-04-06

UpdateManager/backend/__init__.py: once apt upgrade completes, use the deb2snap.json file from ubuntu-release-upgrader to perform snap updates.

[Iain Lane (laney)]

Thanks. I think this looks good to me now. I'm going to give it some more testing and will upload.

There are some review comments inline, nothing blocking.

One bit of feedback: I just got 'snap-store' installed thanks to this change. It took a couple of minutes to download though, and I was a bit worried that the process had stalled or broken somehow. I think it'd be reassuring if you could somehow output progress information, e.g. the amount that's been downloaded. I think this must be possible with snapd-glib, since gnome-software can do it.

review: Approve

=== modified file 'UpdateManager/Core/UpdateList.py'
--- UpdateManager/Core/UpdateList.py	2020-01-31 21:26:39 +0000
+++ UpdateManager/Core/UpdateList.py	2020-04-06 11:32:18 +0000
@@ -212,6 +212,7 @@
         self.update_groups = []
         self.security_groups = []
         self.kernel_autoremove_groups = []
+        self.duplicate_groups = []
         self.num_updates = 0
         self.random = random.Random()
         self.ignored_phased_updates = []
@@ -471,7 +472,7 @@
 
         return app_groups + pkg_groups
 
-    def update(self, cache, eventloop_callback=None):
+    def update(self, cache, eventloop_callback=None, duplicate_packages=[]):
         self.held_back = []
 
         # do the upgrade
@@ -480,6 +481,7 @@
         security_pkgs = []
         upgrade_pkgs = []
         kernel_autoremove_pkgs = []
+        duplicate_pkgs = []
 
         # Find all upgradable packages
         for pkg in cache:
@@ -516,18 +518,23 @@
                      and not cache.running_kernel_pkgs_regexp.match(
                          pkg.name))):
                 kernel_autoremove_pkgs.append(pkg)
+            if (pkg.name in duplicate_packages):
+                duplicate_pkgs.append(pkg)
 
         # perform operations after the loop to not skip packages which
         # changed state due to the resolver
         for pkg in kernel_autoremove_pkgs:
             pkg.mark_delete()
+        for pkg in duplicate_pkgs:
+            pkg.mark_delete()
         for pkg in self.ignored_phased_updates:
             pkg.mark_keep()
 
         if security_pkgs or upgrade_pkgs:
             # There's updates available. Initiate the desktop file cache.
             pkg_names = [p.name for p in
-                         security_pkgs + upgrade_pkgs + kernel_autoremove_pkgs]
+                         security_pkgs + upgrade_pkgs
+                         + kernel_autoremove_pkgs + duplicate_pkgs]
             self._populate_desktop_cache(pkg_names)
         self.update_groups = self._make_groups(cache, upgrade_pkgs,
                                                eventloop_callback)
@@ -535,3 +542,5 @@
                                                  eventloop_callback)
         self.kernel_autoremove_groups = self._make_groups(
             cache, kernel_autoremove_pkgs, eventloop_callback, True)
+        self.duplicate_groups = self._make_groups(
+            cache, duplicate_pkgs, eventloop_callback, True)

=== modified file 'UpdateManager/UpdateManager.py'
--- UpdateManager/UpdateManager.py	2019-04-08 17:00:30 +0000
+++ UpdateManager/UpdateManager.py	2020-04-06 11:32:18 +0000
@@ -86,6 +86,7 @@
         self.update_list = None
         self.meta_release = None
         self.hwe_replacement_packages = None
+        self.duplicate_packages = None
 
         # Basic GTK+ parameters
         self.set_title(_("Software Updater"))
@@ -232,7 +233,8 @@
         self._look_busy()
         self.refresh_cache()
 
-        pane = self._make_available_pane(self.cache.install_count,
+        pane = self._make_available_pane(self.cache.install_count
+                                         + self.cache.del_count,
                                          os.path.exists(REBOOT_REQUIRED_FILE),
                                          cancelled_update, error_occurred)
         self._start_pane(pane)
@@ -387,7 +389,8 @@
 
         self.update_list = UpdateList(self)
         try:
-            self.update_list.update(self.cache, eventloop_callback=iterate)
+            self.update_list.update(self.cache, eventloop_callback=iterate,
+                                    duplicate_packages=self.duplicate_packages)
         except SystemError as e:
             header = _("Could not calculate the upgrade")
             desc = _("An unresolvable problem occurred while "

=== modified file 'UpdateManager/UpdatesAvailable.py'
--- UpdateManager/UpdatesAvailable.py	2019-04-08 17:00:30 +0000
+++ UpdateManager/UpdatesAvailable.py	2020-04-06 11:32:18 +0000
@@ -1055,7 +1055,8 @@
             self._add_groups(self.list.security_groups)
         if self.list.security_groups and self.list.update_groups:
             self._add_header(_("Other updates"), self.list.update_groups)
-        elif self.list.update_groups and self.list.kernel_autoremove_groups:
+        elif self.list.update_groups and (self.list.kernel_autoremove_groups
+                                          or self.list.duplicate_groups):
             self._add_header(_("Updates"), self.list.update_groups)
         if self.list.update_groups:
             self._add_groups(self.list.update_groups)
@@ -1064,11 +1065,17 @@
                 _("Unused kernel updates to be removed"),
                 self.list.kernel_autoremove_groups)
             self._add_groups(self.list.kernel_autoremove_groups)
+        if self.list.duplicate_groups:
+            self._add_header(
+                _("Duplicate packages to be removed"),
+                self.list.duplicate_groups)
+            self._add_groups(self.list.duplicate_groups)
 
         self.treeview_update.set_model(self.store)
         self.pkg_cell_area.indent_toplevel = (
             bool(self.list.security_groups)
-            or bool(self.list.kernel_autoremove_groups))
+            or bool(self.list.kernel_autoremove_groups)
+            or bool(self.list.duplicate_groups))
         self.update_close_button()
         self.update_count()
         self.setBusy(False)

=== modified file 'UpdateManager/backend/__init__.py'
--- UpdateManager/backend/__init__.py	2019-04-08 17:00:30 +0000
+++ UpdateManager/backend/__init__.py	2020-04-06 11:32:18 +0000
@@ -6,11 +6,20 @@
 
 from __future__ import absolute_import
 
-from gi.repository import GLib
+import gi
+gi.require_version('Snapd', '1')
+from gi.repository import GLib, Gtk, Snapd
 
 from apt import Cache
+import json
+import logging
 import os
+import re
+import subprocess
+from subprocess import PIPE, Popen
+from threading import Thread
 
+from UpdateManager.Core.MyCache import MyCache
 from UpdateManager.Core.utils import inhibit_sleep
 from UpdateManager.Dialogs import Dialog
 
@@ -67,6 +76,170 @@
         """Commit the cache changes """
         raise NotImplementedError
 
+    def get_snap_seeds(self):
+        seeded_snaps = {}
+        unseeded_snaps = {}
+
+        release = subprocess.Popen(["lsb_release", "-r", "-s"],
+                                   universal_newlines=True,
+                                   stdout=subprocess.PIPE).communicate()
+        release = release[0].split()[0]
+        curr_channel = "stable/ubuntu-" + release
+
+        try:
+            d2s_file = open(
+                '/usr/share/ubuntu-release-upgrader/deb2snap.json', 'r')
+            d2s = json.load(d2s_file)
+            d2s_file.close()
+
+            for snap in d2s["seeded"]:
+                seed = d2s["seeded"][snap]
+                deb = seed.get("deb", None)
+                to_channel = seed.get("to_channel", curr_channel)
+                seeded_snaps[snap] = (deb, to_channel)
+
+            for snap in d2s["unseeded"]:
+                unseed = d2s["unseeded"][snap]
+                from_channel = unseed.get("from_channel", curr_channel)
+                unseeded_snaps[snap] = (from_channel)
+        except Exception as e:
+            logging.debug("error reading deb2snap.json file (%s)" % e)
+
+        return seeded_snaps, unseeded_snaps
+
+    def get_deb2snap_dups(self):
+        # update and grab the latest cache
+        try:
+            if self.window_main.cache is None:
+                self.window_main.cache = MyCache(None)
+            else:
+                self.window_main.cache.open(None)
+                self.window_main.cache._initDepCache()
+            cache = self.window_main.cache
+        except Exception as e:
+            logging.debug("error reading cache (%s)" % e)
+            cache = None
+
+        duplicates = []
+        seeded_snaps, _ = self.get_snap_seeds()
+
+        for snap, (deb, _) in seeded_snaps.items():
+            # if the deb is installed and was not manually installed,
+            # replace it
+            if (deb in cache and cache[deb].is_auto_installed):
+                duplicates.append(deb)
+
+        return duplicates
+
+    def get_snap_transitions(self):
+        # populate snap_list with deb2snap transitions
+        snap_list = {}
+        seeded_snaps, unseeded_snaps = self.get_snap_seeds()
+
+        for snap, (deb, to_channel) in seeded_snaps.items():
+            snap_object = {}
+            # check if the snap is already installed
+            snap_info = subprocess.Popen(["snap", "info", snap],
+                                         universal_newlines=True,
+                                         stdout=subprocess.PIPE).communicate()
+            if re.search("^installed: ", snap_info[0], re.MULTILINE):
+                logging.debug("Snap %s is installed" % snap)
+                continue
+            elif (deb in self.window_main.duplicate_packages):
+                # install the snap if the deb was just marked delete
+                snap_object['command'] = 'install'
+                snap_object['channel'] = to_channel
+                snap_list[snap] = snap_object
+
+        for snap, (_) in unseeded_snaps.items():
+            snap_object = {}
+            # check if the snap is already installed
+            snap_info = subprocess.Popen(["snap", "info", snap],
+                                         universal_newlines=True,
+                                         stdout=subprocess.PIPE).communicate()
+            if re.search("^installed: ", snap_info[0], re.MULTILINE):
+                logging.debug("Snap %s is installed" % snap)
+                # its not tracking the release channel so don't remove
+                from_channel = "stable/ubuntu-[0-9][0-9].[0-9][0-9]"
+                if not re.search(r"^tracking:.*%s" % from_channel,
+                                 snap_info[0], re.MULTILINE):
+                    logging.debug("Snap %s is not tracking the release channel"
+                                  % snap)
+                    continue
+
+                snap_object['command'] = 'remove'
+
+                # check if this snap is being used by any other snaps
+                conns = subprocess.Popen(["snap", "connections", snap],
+                                         universal_newlines=True,
+                                         stdout=subprocess.PIPE).communicate()
+
+                for conn in conns[0].split('\n'):
+                    conn_cols = conn.split()
+                    if len(conn_cols) != 4:
+                        continue
+                    plug = conn_cols[1]
+                    slot = conn_cols[2]
+
+                    if slot.startswith(snap + ':'):
+                        plug_snap = plug.split(':')[0]
+                        if plug_snap != '-' and \
+                           plug_snap not in unseeded_snaps:
+                            logging.debug("Snap %s is being used by %s. "
+                                          "Switching it to stable track"
+                                          % (snap, plug_snap))
+                            snap_object['command'] = 'refresh'
+                            snap_object['channel'] = 'stable'
+                            break
+
+                snap_list[snap] = snap_object
+
+        return snap_list
+
+    def update_snaps(self):
+        # update status and progress bar
+        def update_status(status):
+            GLib.idle_add(self.label_details.set_label, status)
+
+        def update_progress(progress_bar):
+            progress_bar.pulse()
+            return True
+
+        update_status(_("Updating snaps"))
+
+        progress_timer = None
+        progress_bars = self.progressbar_slot.get_children()
+        if progress_bars and isinstance(progress_bars[0], Gtk.ProgressBar):
+            progress_timer = GLib.timeout_add(100, update_progress,
+                                              progress_bars[0])
+
+        # populate snap_list with deb2snap transitions
+        snap_list = self.get_snap_transitions()
+
+        # (un)install (un)seeded snap(s)
+        try:
+            client = Snapd.Client()
+            client.connect_sync()
+            for snap, snap_object in snap_list.items():
+                command = snap_object['command']
+                if command == 'refresh':
+                    update_status(_("Refreshing %s snap" % snap))
+                    client.refresh_sync(snap, snap_object['channel'])
+                elif command == 'remove':
+                    update_status(_("Removing %s snap" % snap))
+                    client.remove_sync(snap)
+                else:
+                    update_status(_("Installing %s snap" % snap))
+                    client.install_sync(snap, snap_object['channel'])
+        except GLib.Error as e:
+            logging.debug("error updating snaps (%s)" % e)
+
+        # continue with the rest of the updates
+        if progress_timer:
+            GLib.source_remove(progress_timer)
+
+        GLib.idle_add(self.window_main.start_available)
+
     def _action_done(self, action, authorized, success, error_string,
                      error_desc, trans_failed=False):
 
@@ -78,7 +251,9 @@
         #close_after_install = False
 
         if action == self.ACTION_INSTALL:
-            if success:
+            if success and os.path.exists("/usr/bin/snap"):
+                Thread(target=self.update_snaps).start()
+            elif success:
                 self.window_main.start_available()
             elif error_string:
                 self.window_main.start_error(trans_failed, error_string,
@@ -90,6 +265,9 @@
         else:
             if error_string:
                 self.window_main.start_error(True, error_string, error_desc)
+            elif success and os.path.exists("/usr/bin/snap"):
+                self.window_main.duplicate_packages = self.get_deb2snap_dups()
+                self.window_main.start_available()
             else:
                 is_cancelled_update = not success
                 self.window_main.start_available(is_cancelled_update)

=== modified file 'debian/changelog'
--- debian/changelog	2020-01-31 21:44:36 +0000
+++ debian/changelog	2020-04-06 11:32:18 +0000
@@ -1,3 +1,10 @@
+update-manager (1:20.04.2) UNRELEASED; urgency=medium
+
+  * UpdateManager/backend/__init__.py: once apt upgrade completes, use the
+    deb2snap.json file from ubuntu-release-upgrader to perform snap updates.
+
+ -- Marcus Tomlinson <[e-mail]>  Mon, 02 Mar 2020 08:52:13 +0000
+
 update-manager (1:20.04.1) focal; urgency=medium
 
   * UpdateManager/Core/UpdateList.py: remove unused import of subprocess.