Merge lp:~dylanmccall/update-manager/group-by-applications into lp:update-manager

[Dylan McCall]

This is a first step towards grouping updates according to the specification at https://wiki.ubuntu.com/SoftwareUpdates.

There are some details still to be worked out, but the feature so far is functional :)

Changes:
 - Changed the list of updates to a treeview, where updates are collapsed under application names.
 - UpdateList links updated packages to applications and groups them according to dependencies.
 - Restart required indicator is shown for packages with "XB-Restart-Required: system." There is a temporary override for linux-meta for testing purposes.
 - Update origins are not displayed.

Known issues, to be resolved with future work:
 - Indentation in the update tree view is presently hard coded and probably belongs (at least partly) in the light-themes package.
 - The 'Ubuntu base' update group should list specific core packages, not 'everything else.' The specification is unclear on what to do with everything else, so it's like this for the time being.
 - The Updates Required icon does not have its corresponding UI element below the download size text.
 - The Download column does not yet show a check mark for downloaded packages.
 - "Terminal" has an expander, with "gnome-terminal" as a child. Instead, group headers should simply be packages instead of their own things.

So, it isn't _finished_, but it does seem to be in a working state. The reason I'm interested in merging with trunk is because I want to keep our code in sync. Most of the crazy merge conflicts are probably done with now (especially since we seem to be standardized on 4 space indents). However, my diff spans _a lot_ of UpdateList.py and UpdatesAvailable.py, so any conflicts can be a real pain to deal with.

[lp:~dylanmccall/update-manager/group-by-applications]

2442. By Dylan McCall on 2012-06-29

Attempt to load a symbolic icon for 'restart required'.
Don't select second item in update list.

[Barry Warsaw (barry)]

See detailed review sent separately.

review: Needs Fixing

[Barry Warsaw (barry)]

Hi Dylan,

Thanks for your contribution to Update Manager, and to Ubuntu!

I've provided some detailed comments on the code below.

One thing I'd really like to see is some unittests, especially for the new
code in UpdateList.py. Would you like to take a crack at adding a few for
some of the new methods and classes you're adding? Have you run the existing
unittests to see if your changes have broken any of them? While Update
Manager doesn't have a fantastic test suite, I'm not crazy about making things
worse by adding more untested code.

Someone should also review the UI changes before this branch lands, probably
mpt or someone else on the design team. I'll request a review from mpt to
start with.

I did notice one UI problem when I ran your branch on Quantal. I see an
update for Ubuntu base (which I gather is a catch-all for anything that
doesn't fit into any other category). However, when I expand the arrow for
that group, I'm still left with a very short window, one row that contains the
"Ubuntu base" string, and a very long and mostly unusable scroll bar. Maybe
the window should expand vertically to fit the sub-package contents better, or
maybe the details window should be taller to start with.

Other than that, I think the groupings are a nice refactoring of the
information presented by Update Manager!

I'll mark the branch Needs Fixing for now, but I will be happy to re-review it
if you make the suggested changes.

=== modified file 'UpdateManager/Core/UpdateList.py'
--- UpdateManager/Core/UpdateList.py 2012-06-28 00:10:23 +0000
+++ UpdateManager/Core/UpdateList.py 2012-07-17 18:03:22 +0000
> @@ -1,25 +1,29 @@
> -# UpdateList.py
> +# UpdateList.py
> # -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*-
> -#
> +#

You need to be really careful about adding extra whitespace, as your patch
does here. There are several reason for this, but probably the most important
one is that such whitespace-only changes are just noise that distract from the
substantive changes, and reviews thereof. There are many options for ensuring
"whitespace normalization" in Python code, some of which can be accomplished
by your editor. E.g. I have nice functions for Emacs:

http://mail.python.org/pipermail/python-dev/2007-April/072773.html

> # Copyright (c) 2004-2008 Canonical
> -#
> +#
> # Author: Michael Vogt <email address hidden>
> -#
> -# This program is free software; you can redistribute it and/or
> -# modify it under the terms of the GNU General Public License as
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> # published by the Free Software Foundation; either version 2 of the
> # License, or (at your option) any later version.
> -#
> +#
> # This program is distributed in the hope that it will be useful,
> # but WITHOUT ANY WARRANTY; without even the implied warranty of
> # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> # GNU General Public License for more details.
> -#
> +#
> # You should have received a copy of the GNU General Public License
> # along with this program; if not, wri...

[lp:~dylanmccall/update-manager/group-by-applications]

2443. By Dylan McCall on 2012-08-18

Merged changes from trunk.
Removed unnecessary TODO notes in UpdateList.py

2444. By Dylan McCall on 2012-08-20

Fixed unintended formatting changes.

2445. By Dylan McCall on 2012-08-20

Merged changes from trunk.

2446. By Dylan McCall on 2012-08-20

Fixed some code style errors in UpdateList.

[Dylan McCall (dylanmccall)]

Okay, I'm finally working on this and I'll let you know when it's all done. (Couple days, I hope). A lot of the weirder issues are due to me manually merging changes, apparently to the wrong version of UpdateList.py. Unfortunate waste of time, so thank you for being so patient, Barry! :)

[lp:~dylanmccall/update-manager/group-by-applications]

2447. By Dylan McCall on 2012-08-21

Fixed lines longer than 79 characters in UpdateList.py.

2448. By Dylan McCall on 2012-11-10

Fixed style and commenting issues in UpdatesAvailable.py.

2449. By Dylan McCall on 2012-11-10

Merged changes from trunk

[Dylan McCall (dylanmccall)]

There, I think that covers the immediate problems. Those bizarre whitespace changes were because mterry had cleaned stuff up beautifully (along with the Python 3 update) between me starting and finishing this, and then I merged changes in the worst possible way. I'm sorry for taking so very long to get back to you.

Replacing pkg with package in UpdateList.py:
I'd like to leave "pkg" be in this branch, if that's okay, just because pkg seems to appear consistently across the application and in the previous version of UpdateList.py (even in the apt.pkg library we import). I agree it would be lovely to clean up, though. I honestly do cringe every time I abbreviate something :)

You were wondering about that weird function called _rate_application_for_package. Originally I had expected to need some fancier heuristics, but we seem to be getting along fine with what we have here. I agree with you, and I'll deal with that soon.

get_application_for_package:
GIO has a lovely function to parse a .desktop file, spitting out a class with a (localized) name, icon and such. The important bit is it's the same code we use in places like the Unity dash or the Applications menu in Gnome Panel, so it's all nice and familiar. Adding some comments to explain the purpose...

APP_INSTALL_PATH: Right now that is effectively the same thing that happens in Software Centre. I'm happy to turn it into a config parameter :)

I have some refactoring to do (and some new designs to implement!), so I think I will do that, and I will try to do tests and documentation alongside that work. I will resubmit then.

review: Needs Resubmitting

[Marco Biscaro (marcobiscaro2112)]

I tried to merge trunk and got conflicts. You should merge trunk again and fix them (in UpdateManager/Core/UpdateList.py).

Some details:

=== modified file 'UpdateManager/Core/UpdateList.py'
--- UpdateManager/Core/UpdateList.py 2012-08-20 15:32:45 +0000
+++ UpdateManager/Core/UpdateList.py 2012-11-10 06:29:19 +0000
@@ -1,7 +1,7 @@
 # UpdateList.py
 # -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*-
 #
-# Copyright (c) 2004-2012 Canonical
+# Copyright (c) 2004-2008 Canonical

I think this shouldn't be modified.

 #
 # Author: Michael Vogt <email address hidden>
 #

You modified this file a lot, I think your name should be here too. :)

@@ -22,53 +22,103 @@

 from __future__ import print_function

+import warnings
+warnings.filterwarnings("ignore", "Accessed deprecated property",
+ DeprecationWarning)
+
 from gettext import gettext as _
-
-import apt
-import logging
 import operator
-import random
+import itertools
 import subprocess
+import os
+import glob
 import sys

What about alphabetical order here?

[Michael Terry (mterry)]

Dylan, I wanted to help push this over the hump. I've got a merged-with-trunk branch here: lp:~mterry/update-manager/group-by-application

I'll look into adding some unittests next and fixing the single-package headers next. Is there a team we could put a shared branch under?

[Dylan McCall (dylanmccall)]

Cool! There is not one at the moment, but feel free to add me to one. (I'll
be able to do that in about 8 hours).

I have a few local changes that I'm aiming to finish up and push by Sunday,
so we might want to chat about those over email or IRC or something, too.
They could make things a lot easier (once they are not broken),
particularly with single packages and packages that aren't applications;)

On Mon, Dec 17, 2012 at 11:04 AM, Michael Terry <<email address hidden>
> wrote:

> Dylan, I wanted to help push this over the hump. I've got a
> merged-with-trunk branch here:
> lp:~mterry/update-manager/group-by-application
>
> I'll look into adding some unittests next and fixing the single-package
> headers next. Is there a team we could put a shared branch under?
> --
>
> https://code.launchpad.net/~dylanmccall/update-manager/group-by-applications/+merge/112678
> You are the owner of lp:~dylanmccall/update-manager/group-by-applications.
>

[Marco Biscaro (marcobiscaro2112)]

I can help too!

=== modified file 'UpdateManager/Core/UpdateList.py'
--- UpdateManager/Core/UpdateList.py	2012-08-20 15:32:45 +0000
+++ UpdateManager/Core/UpdateList.py	2012-11-10 06:29:19 +0000
@@ -1,7 +1,7 @@
 # UpdateList.py
 # -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*-
 #
-#  Copyright (c) 2004-2012 Canonical
+#  Copyright (c) 2004-2008 Canonical
 #
 #  Author: Michael Vogt <[e-mail]>
 #
@@ -22,53 +22,103 @@
 
 from __future__ import print_function
 
+import warnings
+warnings.filterwarnings("ignore", "Accessed deprecated property",
+                        DeprecationWarning)
+
 from gettext import gettext as _
-
-import apt
-import logging
 import operator
-import random
+import itertools
 import subprocess
+import os
+import glob
 import sys
 
-
-class UpdateOrigin(object):
+from gi.repository import Gio
+
+APP_INSTALL_PATH = "/usr/share/app-install/desktop"
+
+class UpdateGroup():
+    def __init__(self, core_pkgs, name, icon):
+        self._core_pkgs = set(core_pkgs)
+        self._extra_pkgs = set()
+        self.name = name
+        self.icon = icon
+
+    @property
+    def pkgs(self):
+        all_pkgs = []
+        all_pkgs.extend(self._core_pkgs)
+        all_pkgs.extend(self._extra_pkgs)
+        return sorted(all_pkgs, key=operator.attrgetter('name'))
+
+    def add_extra(self, extra_pkg):
+        if self._contains(extra_pkg):
+            return True
+        elif self._is_dependency(extra_pkg):
+            self._extra_pkgs.add(extra_pkg)
+            return True
+        else:
+            return False
+
+    def _contains(self, pkg):
+        return pkg in self._core_pkgs or pkg in self._extra_pkgs
+
+    def _is_dependency(self, pkg):
+        for core_pkg in self._core_pkgs:
+            candidate = core_pkg.candidate
+            dependencies = candidate.get_dependencies('Depends', 'Recommends')
+            for dependency_pkg in itertools.chain.from_iterable(dependencies):
+                if dependency_pkg.name == pkg.name:
+                    return True
+        return False
+
+    def packages_are_selected(self):
+        for pkg in self.pkgs:
+            if pkg.marked_install or pkg.marked_upgrade:
+                return True
+        return False
+
+    def selection_is_inconsistent(self):
+        pkgs_installing = [pkg for pkg in self.pkgs
+                           if pkg.marked_install or pkg.marked_upgrade]
+        return (len(pkgs_installing) > 0 and
+                len(pkgs_installing) < len(self.pkgs))
+
+    def get_total_size(self):
+        size = 0
+        for pkg in self.pkgs:
+            size += getattr(pkg.candidate, "size", 0)
+        return size
+
+
+class UpdateApplicationGroup(UpdateGroup):
+    def __init__(self, pkgs, application):
+        name = application.get_display_name()
+        icon = application.get_icon()
+        super(UpdateApplicationGroup, self).__init__(pkgs, name, icon)
+
+
+class UpdateSystemGroup(UpdateGroup):
+    def __init__(self, system_pkgs):
+        name = _("Ubuntu base")
+        icon = Gio.ThemedIcon.new("distributor-logo")
+        super(UpdateSystemGroup, self).__init__(system_pkgs, name, icon)
+
+
+class UpdateOrigin():
     def __init__(self, desc, importance):
         self.packages = []
         self.importance = importance
         self.description = desc
 
 
-class OriginsImportance:
-    # filed in by us
-    SECURITY = 10
-    UPDATES = 9
-    PROPOSED = 8
-    BACKPORTS = 7
-    ARCHIVE = 6
-    # this is filed in by MyCache
-    OTHER = 0
-    # this is used by us
-    OTHER_UNKNOWN = -1
-
-
-class UpdateList(object):
+class UpdateList():
     """
     class that contains the list of available updates in
     self.pkgs[origin] where origin is the user readable string
     """
 
-    # the key in the debian/control file used to add the phased
-    # updates percentage
-    PHASED_UPDATES_KEY = "Phased-Update-Percentage"
-
-    # the file that contains the uniq machine id
-    UNIQ_MACHINE_ID_FILE = "/var/lib/dbus/machine-id"
-
-    # the configuration key to turn phased-updates always on
-    ALWAYS_INCLUDE_PHASED_UPDATES = (
-        "Update-Manager::Always-Include-Phased-Updates")
-
     def __init__(self, parent):
         # a map of packages under their origin
         try:
@@ -81,59 +131,64 @@
                            "you are using.") % e)
             sys.exit(1)
         self.distUpgradeWouldDelete = 0
-        self.pkgs = {}
+        self.pkg_groups = []
         self.num_updates = 0
-        self.matcher = self.initMatcher(dist)
-        self.random = random.Random()
-        # a stable machine uniq id
-        with open(self.UNIQ_MACHINE_ID_FILE) as f:
-            self.machine_uniq_id = f.read()
-
-    def initMatcher(self, dist):
-        # (origin, archive, description, importance)
-        matcher_templates = [
-            ("%s-security" % dist, "Ubuntu", _("Important security updates"),
-             OriginsImportance.SECURITY),
-            ("%s-updates" % dist, "Ubuntu", _("Recommended updates"),
-             OriginsImportance.UPDATES),
-            ("%s-proposed" % dist, "Ubuntu", _("Proposed updates"),
-             OriginsImportance.PROPOSED),
-            ("%s-backports" % dist, "Ubuntu", _("Backports"),
-             OriginsImportance.BACKPORTS),
-            (dist, "Ubuntu", _("Distribution updates"),
-             OriginsImportance.ARCHIVE),
-            (None, None, _("Other updates"), OriginsImportance.OTHER_UNKNOWN)
-        ]
-        matcher = {}
-        for (origin, archive, desc, importance) in matcher_templates:
-            matcher[(origin, archive)] = UpdateOrigin(desc, importance)
-        return matcher
-
-    def is_ignored_phased_update(self, pkg):
-        """ This will test if the pkg is a phased updated and if
-            it needs to get installed or ignored.
-
-            :return: True if the updates should be ignored
-        """
-        # allow the admin to override this
-        if apt.apt_pkg.config.find_b(
-            self.ALWAYS_INCLUDE_PHASED_UPDATES, False):
-            return False
-
-        if self.PHASED_UPDATES_KEY in pkg.candidate.record:
-            # its important that we always get the same result on
-            # multiple runs of the update-manager, so we need to
-            # feed a seed that is a combination of the pkg/ver/machine
-            self.random.seed("%s-%s-%s" % (
-                    pkg.name, pkg.candidate.version,
-                    self.machine_uniq_id))
-            threshold = pkg.candidate.record[self.PHASED_UPDATES_KEY]
-            percentage = self.random.randint(0, 100)
-            if percentage > int(threshold):
-                logging.info("holding back phased update (%s < %s)" % (
-                        threshold, percentage))
-                return True
-        return False
+        data_dirs = os.environ['XDG_DATA_DIRS'].split(':')
+        self.application_dirs = [os.path.join(base, 'applications')
+                                 for base in data_dirs]
+        self.current_desktop = os.environ.get('XDG_CURRENT_DESKTOP')
+
+    def _file_is_application(self, file_path):
+        file_path = os.path.abspath(file_path)
+        is_application = False
+        for app_dir in self.application_dirs:
+            is_application = is_application or file_path.startswith(app_dir)
+        extension = os.path.splitext(file_path)[1]
+        is_application = is_application and (extension == '.desktop')
+        return is_application
+
+    def _rate_application_for_package(self, application, pkg):
+        score = 0
+        desktop_file = os.path.basename(application.get_filename())
+        application_id = os.path.splitext(desktop_file)[0]
+
+        if application.should_show():
+            score += 1
+
+            if application_id == pkg.name:
+                score += 5
+
+        return score
+
+    def _get_application_for_package(self, pkg):
+        desktop_files = []
+        rated_applications = []
+
+        for installed_file in pkg.installed_files:
+            if self._file_is_application(installed_file):
+                desktop_files.append(installed_file)
+
+        app_install_pattern = os.path.join(APP_INSTALL_PATH, '%s:*' % pkg.name)
+        for desktop_file in glob.glob(app_install_pattern):
+            desktop_files.append(desktop_file)
+
+        for desktop_file in desktop_files:
+            try:
+                application = Gio.DesktopAppInfo \
+                              .new_from_filename(desktop_file)
+                application.set_desktop_env(self.current_desktop)
+            except Exception as e:
+                print("Error loading .desktop file %s: %s" % installed_file, e)
+                continue
+            score = self._rate_application_for_package(application, pkg)
+            if score > 0:
+                rated_applications.append((score, application))
+
+        rated_applications.sort(key=lambda app: app[0], reverse=True)
+        if len(rated_applications) > 0:
+            return rated_applications[0][1]
+        else:
+            return None
 
     def update(self, cache):
         self.held_back = []
@@ -141,9 +196,11 @@
         # do the upgrade
         self.distUpgradeWouldDelete = cache.saveDistUpgrade()
 
-        #dselect_upgrade_origin = UpdateOrigin(_("Previous selected"), 1)
+        upgrade_pkgs = []
+        pkgs_by_source = {}
+        ungrouped_pkgs = []
 
-        # sort by origin
+        # Find all upgradable packages
         for pkg in cache:
             if pkg.is_upgradable or pkg.marked_install:
                 if getattr(pkg.candidate, "origins", None) is None:
@@ -152,22 +209,40 @@
                     print("WARNING: upgradable but no candidate.origins?!?: ",
                           pkg.name)
                     continue
-                # check where the package belongs
-                origin_node = cache.match_package_origin(pkg, self.matcher)
-
-                # see if its a phased update and *not* a security update
-                # or shadowing a security update
-                if (origin_node.importance != OriginsImportance.SECURITY and
-                    self.is_ignored_phased_update(pkg)):
-                    continue
-
-                if origin_node not in self.pkgs:
-                    self.pkgs[origin_node] = []
-                self.pkgs[origin_node].append(pkg)
+
+                upgrade_pkgs.append(pkg)
                 self.num_updates = self.num_updates + 1
+
             if pkg.is_upgradable and not (pkg.marked_upgrade or
                                           pkg.marked_install):
                 self.held_back.append(pkg.name)
-        for l in self.pkgs.keys():
-            self.pkgs[l].sort(key=operator.attrgetter("name"))
+
+        # Index packages by source package name
+        for pkg in upgrade_pkgs:
+            srcpkg = pkg.candidate.source_name
+            pkgs_by_source.setdefault(srcpkg, []).append(pkg)
+
+        for srcpkg, pkgs in pkgs_by_source.items():
+            for pkg in pkgs:
+                app = self._get_application_for_package(pkg)
+                if app is not None:
+                    app_group = UpdateApplicationGroup([pkg], app)
+                    self.pkg_groups.append(app_group)
+                else:
+                    ungrouped_pkgs.append(pkg)
+        self.pkg_groups.sort(key=operator.attrgetter('name'))
+
+        # Stick together packages and their immediate dependencies
+        for pkg in upgrade_pkgs:
+            for group in self.pkg_groups:
+                result = group.add_extra(pkg)
+                if result and pkg in ungrouped_pkgs:
+                    ungrouped_pkgs.remove(pkg)
+
+        # FIXME: system_group should only have packages that are dependencies
+        # of ubuntu-meta
+        system_group = UpdateSystemGroup(ungrouped_pkgs)
+        self.pkg_groups.append(system_group)
+
         self.keepcount = cache._depcache.keep_count
+
 
=== modified file 'UpdateManager/UpdatesAvailable.py'
--- UpdateManager/UpdatesAvailable.py	2012-10-10 08:13:13 +0000
+++ UpdateManager/UpdatesAvailable.py	2012-11-10 06:29:19 +0000
@@ -70,10 +70,12 @@
 
 # FIXME:
 # - kill "all_changes" and move the changes into the "Update" class
+# - screen reader does not read update toggle state
+# - screen reader does not say "Downloaded" for downloaded updates
 
 # list constants
-(LIST_CONTENTS, LIST_NAME, LIST_PKG,
- LIST_ORIGIN, LIST_TOGGLE_CHECKED) = range(5)
+(LIST_NAME, LIST_UPDATE_DATA, LIST_SIZE, LIST_TOGGLE_ACTIVE) = range(4)
+#LIST_UPDATE_DATA is a tuple of (is_toplevel, group, pkg)
 
 # NetworkManager enums
 from .Core.roam import NetworkManagerHelper
@@ -134,38 +136,71 @@
         self.update_close_button()
 
         # the treeview (move into it's own code!)
-        self.store = Gtk.ListStore(str, str, GObject.TYPE_PYOBJECT,
-                                   GObject.TYPE_PYOBJECT, bool)
+        self.store = Gtk.TreeStore(str, GObject.TYPE_PYOBJECT, str, bool)
         self.treeview_update.set_model(self.store)
-        self.treeview_update.set_headers_clickable(True)
+
+        restart_icon_renderer = Gtk.CellRendererPixbuf()
+        restart_icon_renderer.set_property("xpad", 4)
+        restart_icon_renderer.set_property("ypad", 2)
+        restart_icon_renderer.set_property("stock-size", Gtk.IconSize.MENU)
+        restart_icon_renderer.set_property("follow-state", True)
+        restart_column = Gtk.TreeViewColumn(None, restart_icon_renderer)
+        restart_column.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE)
+        self.treeview_update.append_column(restart_column)
+        restart_column.set_cell_data_func(restart_icon_renderer,
+                                          self.restart_icon_renderer_data_func)
+
+        pkg_column = Gtk.TreeViewColumn()
+        pkg_column.set_title(_("Install"))
+        pkg_column.set_property("spacing", 4)
+        pkg_column.set_sizing(Gtk.TreeViewColumnSizing.FIXED)
+        pkg_column.set_expand(True)
+        self.treeview_update.append_column(pkg_column)
+
+        pkg_toggle_renderer = Gtk.CellRendererToggle()
+        pkg_toggle_renderer.set_property("ypad", 2)
+        pkg_toggle_renderer.connect("toggled", self.on_update_toggled)
+        pkg_column.pack_start(pkg_toggle_renderer, False)
+        pkg_column.add_attribute(pkg_toggle_renderer,
+                                 'active', LIST_TOGGLE_ACTIVE)
+        pkg_column.set_cell_data_func(pkg_toggle_renderer,
+                                      self.pkg_toggle_renderer_data_func)
+
+        pkg_icon_renderer = Gtk.CellRendererPixbuf()
+        pkg_icon_renderer.set_property("ypad", 2)
+        pkg_icon_renderer.set_property("stock-size", Gtk.IconSize.MENU)
+        pkg_column.pack_start(pkg_icon_renderer, False)
+        pkg_column.set_cell_data_func(pkg_icon_renderer,
+                                      self.pkg_icon_renderer_data_func)
+
+        pkg_label_renderer = Gtk.CellRendererText()
+        pkg_label_renderer.set_property("ypad", 2)
+        pkg_column.pack_start(pkg_label_renderer, True)
+        pkg_column.set_cell_data_func(pkg_label_renderer,
+                                      self.pkg_label_renderer_data_func)
+
+        size_renderer = Gtk.CellRendererText()
+        size_renderer.set_property("xpad", 6)
+        size_renderer.set_property("ypad", 0)
+        size_renderer.set_property("xalign", 1)
+        # 1.0/1.2 == PANGO.Scale.SMALL. Constant is not (yet) introspected.
+        size_renderer.set_property("scale", 1.0/1.2)
+        size_column = Gtk.TreeViewColumn(_("Download"), size_renderer,
+                                         text=LIST_SIZE)
+        size_column.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE)
+        self.treeview_update.append_column(size_column)
+
+        self.treeview_update.set_headers_visible(True)
+        self.treeview_update.set_headers_clickable(False)
         self.treeview_update.set_direction(Gtk.TextDirection.LTR)
-
-        tr = Gtk.CellRendererText()
-        tr.set_property("xpad", 6)
-        tr.set_property("ypad", 6)
-        cr = Gtk.CellRendererToggle()
-        cr.set_property("activatable", True)
-        cr.set_property("xpad", 6)
-        cr.connect("toggled", self.toggled)
-
-        column_install = Gtk.TreeViewColumn(_("Install"), cr,
-                                            active=LIST_TOGGLE_CHECKED)
-        column_install.set_cell_data_func(cr, self.install_column_view_func)
-        column = Gtk.TreeViewColumn(_("Name"), tr, markup=LIST_CONTENTS)
-        column.set_resizable(True)
-
-        column_install.set_sizing(Gtk.TreeViewColumnSizing.FIXED)
-        column_install.set_fixed_width(30)
-        column.set_sizing(Gtk.TreeViewColumnSizing.FIXED)
-        column.set_fixed_width(100)
         self.treeview_update.set_fixed_height_mode(False)
-
-        self.treeview_update.append_column(column_install)
-        column_install.set_visible(True)
-        self.treeview_update.append_column(column)
+        self.treeview_update.set_expander_column(pkg_column)
         self.treeview_update.set_search_column(LIST_NAME)
         self.treeview_update.connect("button-press-event",
-                                     self.show_context_menu)
+                                     self.on_treeview_button_press)
+        # Line up row details under expanders.
+        # FIXME: This is arbitrary and should happen in the Gtk theme itself
+        self.treeview_update.set_level_indentation(12)
 
         # setup the help viewer and disable the help button if there
         # is no viewer available
@@ -210,26 +245,83 @@
             self.button_close.set_use_underline(False)
 
     def install_all_updates(self, menu, menuitem, data):
-        self.select_all_updgrades(None)
+        self.select_all_upgrades(None)
         self.on_button_install_clicked(None)
 
-    def install_column_view_func(self, cell_layout, renderer, model, iter,
-                                 data):
-        pkg = model.get_value(iter, LIST_PKG)
-        if pkg is None:
-            renderer.set_property("activatable", True)
-            return
-        current_state = renderer.get_property("active")
-        to_install = pkg.marked_install or pkg.marked_upgrade
-        renderer.set_property("active", to_install)
-        # we need to update the store as well to ensure orca knowns
-        # about state changes (it will not read view_func changes)
-        if to_install != current_state:
-            self.store[iter][LIST_TOGGLE_CHECKED] = to_install
-        if pkg.name in self.list.held_back:
-            renderer.set_property("activatable", False)
-        else:
-            renderer.set_property("activatable", True)
+    def restart_icon_renderer_data_func(self, cell_layout, renderer, model,
+                                        iter, data):
+        # List of packages we know require a restart.
+        # Hack to test the feature while packages lack the needed metadata.
+        # FIXME: Remove this soon.
+        restart_sourcepkgs = ['linux-meta']
+
+        def pkg_requires_restart(pkg):
+            restart_condition = pkg.candidate.record.get('XB-Restart-Required')
+            restart_override = pkg.candidate.source_name in restart_sourcepkgs
+            return restart_condition == 'system' or restart_override
+
+        is_toplevel, group, pkg = model.get_value(iter, LIST_UPDATE_DATA)
+        path = model.get_path(iter)
+
+        requires_restart = False
+        if is_toplevel:
+            if not self.treeview_update.row_expanded(path):
+                # A package in the group requires restart
+                for group_pkg in group.pkgs:
+                    if pkg_requires_restart(group_pkg):
+                        requires_restart = True
+                        break
+        else:
+            requires_restart = pkg_requires_restart(pkg)
+
+        # FIXME: Non-standard, incorrect icon name (from app category).
+        # Theme support for what we want seems to be lacking.
+        if requires_restart:
+            restart_icon_names = ['view-refresh-symbolic',
+                                  'system-restart',
+                                  'system-reboot']
+            gicon = Gio.ThemedIcon.new_from_names(restart_icon_names)
+        else:
+            gicon = None
+        renderer.set_property("gicon", gicon)
+
+    def pkg_toggle_renderer_data_func(self, cell_layout, renderer, model, iter, data):
+        is_toplevel, group, pkg = model.get_value(iter, LIST_UPDATE_DATA)
+
+        if is_toplevel:
+            activatable = True
+            inconsistent = group.selection_is_inconsistent()
+        elif pkg:
+            activatable = pkg.name not in self.list.held_back
+            inconsistent = False
+
+        # Assumes the "active" attribute is already set, tied directly to LIST_TOGGLE_ACTIVE
+        renderer.set_property("activatable", activatable)
+        renderer.set_property("inconsistent", inconsistent)
+
+    def pkg_icon_renderer_data_func(self, cell_layout, renderer, model, iter, data):
+        is_toplevel, group, pkg = model.get_value(iter, LIST_UPDATE_DATA)
+
+        if is_toplevel:
+            gicon = group.icon
+        else:
+            gicon = Gio.ThemedIcon.new("package")
+
+        renderer.set_property("gicon", gicon)
+
+    def pkg_label_renderer_data_func(self, cell_layout, renderer, model, iter, data):
+        is_toplevel, group, pkg = model.get_value(iter, LIST_UPDATE_DATA)
+
+        if is_toplevel:
+            markup = "<b>%s</b>" % group.name
+        else:
+            pkg_name = xml.sax.saxutils.escape(pkg.name)
+            if not pkg.is_installed:
+                markup = _("%s (New install)") % pkg_name
+            else:
+                markup = pkg_name
+
+        renderer.set_property("markup", markup)
 
     def set_changes_buffer(self, changes_buffer, text, name, srcpkg):
         changes_buffer.set_text("")
@@ -264,7 +356,7 @@
         iter = model.get_iter(path)
 
         # set descr
-        pkg = model.get_value(iter, LIST_PKG)
+        is_toplevel, group, pkg = model.get_value(iter, LIST_UPDATE_DATA)
         if (pkg is None or pkg.candidate is None or
                 pkg.candidate.description is None):
             changes_buffer = self.textview_changes.get_buffer()
@@ -350,7 +442,7 @@
         if changes:
             self.set_changes_buffer(changes_buffer, changes, name, srcpkg)
 
-    def show_context_menu(self, widget, event):
+    def on_treeview_button_press(self, widget, event):
         """
         Show a context menu if a right click was performed on an update entry
         """
@@ -361,10 +453,13 @@
             self.menu = menu = Gtk.Menu()
             item_select_none = \
                 Gtk.MenuItem.new_with_mnemonic(_("_Deselect All"))
-            item_select_none.connect("activate", self.select_none_updgrades)
+            item_select_none.connect("activate", self.select_none_upgrades)
             menu.append(item_select_none)
+            num_updates = self.cache.install_count
+            if num_updates == 0:
+                item_select_none.set_property("sensitive", False)
             item_select_all = Gtk.MenuItem.new_with_mnemonic(_("Select _All"))
-            item_select_all.connect("activate", self.select_all_updgrades)
+            item_select_all.connect("activate", self.select_all_upgrades)
             menu.append(item_select_all)
             menu.show_all()
             menu.popup_for_device(
@@ -373,35 +468,35 @@
             return True
 
     # we need this for select all/unselect all
-    def _toggle_origin_headers(self, new_selection_value):
-        """ small helper that will set/unset the origin headers
+    def _toggle_group_headers(self, new_selection_value):
+        """ small helper that will set/unset the group headers
         """
         model = self.treeview_update.get_model()
         for row in model:
-            if not model.get_value(row.iter, LIST_PKG):
-                model.set_value(row.iter, LIST_TOGGLE_CHECKED,
-                                new_selection_value)
+            is_toplevel, group, pkg = model.get_value(row.iter, LIST_UPDATE_DATA)
+            if is_toplevel:
+                model.set_value(row.iter, LIST_TOGGLE_ACTIVE, new_selection_value)
 
-    def select_all_updgrades(self, widget):
+    def select_all_upgrades(self, widget):
         """
         Select all updates
         """
         self.setBusy(True)
         self.cache.saveDistUpgrade()
-        self._toggle_origin_headers(True)
+        self._toggle_group_headers(True)
         self.treeview_update.queue_draw()
-        self.refresh_updates_count()
+        self.updates_changed()
         self.setBusy(False)
 
-    def select_none_updgrades(self, widget):
+    def select_none_upgrades(self, widget):
         """
         Select none updates
         """
         self.setBusy(True)
         self.cache.clear()
-        self._toggle_origin_headers(False)
+        self._toggle_group_headers(False)
         self.treeview_update.queue_draw()
-        self.refresh_updates_count()
+        self.updates_changed()
         self.setBusy(False)
 
     def setBusy(self, flag):
@@ -417,7 +512,17 @@
         while Gtk.events_pending():
             Gtk.main_iteration()
 
-    def refresh_updates_count(self):
+    def _mark_selected_updates(self):
+        def foreach_cb(model, path, iter, data):
+            is_toplevel, group, pkg = model.get_value(iter, LIST_UPDATE_DATA)
+            if is_toplevel:
+                active = group.packages_are_selected()
+            elif pkg:
+                active = pkg.marked_install or pkg.marked_upgrade
+            model.set_value(iter, LIST_TOGGLE_ACTIVE, active)
+        self.store.foreach(foreach_cb, None)
+
+    def _refresh_updates_count(self):
         self.button_install.set_sensitive(self.cache.install_count)
         try:
             inst_count = self.cache.install_count
@@ -458,10 +563,15 @@
             self.hbox_downsize.show()
             self.vbox_alerts.show()
 
+    def updates_changed(self):
+        self._mark_selected_updates()
+        self._refresh_updates_count()
+
     def update_count(self):
         """activate or disable widgets and show dialog texts correspoding to
            the number of available updates"""
-        self.refresh_updates_count()
+        self.updates_changed()
+        num_updates = self.cache.install_count
 
         text_header = None
         text_desc = None
@@ -571,14 +681,14 @@
         # can deal with dialup connections properly
         if state in NetworkManagerHelper.NM_STATE_CONNECTING_LIST:
             self.label_offline.set_text(_("Connecting..."))
-            self.refresh_updates_count()
+            self.updates_changed()
             self.hbox_offline.show()
             self.vbox_alerts.show()
             self.connected = False
         # in doubt (STATE_UNKNOWN), assume connected
         elif (state in NetworkManagerHelper.NM_STATE_CONNECTED_LIST or
               state == NetworkManagerHelper.NM_STATE_UNKNOWN):
-            self.refresh_updates_count()
+            self.updates_changed()
             self.hbox_offline.hide()
             self.connected = True
             # trigger re-showing the current app to get changelog info (if
@@ -588,7 +698,7 @@
             self.connected = False
             self.label_offline.set_text(_("You may not be able to check for "
                                           "updates or download new updates."))
-            self.refresh_updates_count()
+            self.updates_changed()
             self.hbox_offline.show()
             self.vbox_alerts.show()
 
@@ -611,45 +721,15 @@
             self.hbox_on_3g.hide()
             self.hbox_roaming.hide()
 
-    def row_activated(self, treeview, path, column):
-        iter = self.store.get_iter(path)
-
-        pkg = self.store.get_value(iter, LIST_PKG)
-        origin = self.store.get_value(iter, LIST_ORIGIN)
-        if pkg is not None:
-            return
-        self.toggle_from_origin(pkg, origin, True)
-
-    def toggle_from_origin(self, pkg, origin, select_all=True):
-        self.setBusy(True)
-        actiongroup = apt_pkg.ActionGroup(self.cache._depcache)
-        for pkg in self.list.pkgs[origin]:
-            if pkg.marked_install or pkg.marked_upgrade:
-                #print("marking keep: ", pkg.name)
-                pkg.mark_keep()
-            elif not (pkg.name in self.list.held_back):
-                #print("marking install: ", pkg.name)
-                pkg.mark_install(auto_fix=False, auto_inst=False)
-        # check if we left breakage
-        if self.cache._depcache.broken_count:
-            Fix = apt_pkg.ProblemResolver(self.cache._depcache)
-            Fix.resolve_by_keep()
-        self.refresh_updates_count()
-        self.treeview_update.queue_draw()
-        del actiongroup
-        self.setBusy(False)
-
-    def toggled(self, renderer, path):
+    def on_update_toggled(self, renderer, path):
         """ a toggle button in the listview was toggled """
         iter = self.store.get_iter(path)
-        pkg = self.store.get_value(iter, LIST_PKG)
-        origin = self.store.get_value(iter, LIST_ORIGIN)
+        is_toplevel, group, pkg = self.store.get_value(iter, LIST_UPDATE_DATA)
         # make sure that we don't allow to toggle deactivated updates
         # this is needed for the call by the row activation callback
-        if pkg is None:
-            toggled_value = not self.store.get_value(iter, LIST_TOGGLE_CHECKED)
-            self.toggle_from_origin(pkg, origin, toggled_value)
-            self.store.set_value(iter, LIST_TOGGLE_CHECKED, toggled_value)
+        if is_toplevel:
+            self.toggle_from_group(group)
+            self.updates_changed()
             self.treeview_update.queue_draw()
             return
         if pkg is None or pkg.name in self.list.held_back:
@@ -666,8 +746,8 @@
                 pkg.mark_install()
             except SystemError:
                 pass
+        self.updates_changed()
         self.treeview_update.queue_draw()
-        self.refresh_updates_count()
         self.setBusy(False)
 
     def on_treeview_update_row_activated(self, treeview, path, column, *args):
@@ -675,7 +755,28 @@
         If an update row was activated (by pressing space), toggle the
         install check box
         """
-        self.toggled(None, path)
+        self.on_update_toggled(None, path)
+
+    def toggle_from_group(self, group):
+        self.setBusy(True)
+        actiongroup = apt_pkg.ActionGroup(self.cache._depcache)
+
+        # Deselect all updates if any in group are selected
+        keep_packages = group.packages_are_selected()
+        for pkg in group.pkgs:
+            if keep_packages:
+                pkg.mark_keep()
+            elif not pkg.name in self.list.held_back:
+                pkg.mark_install(auto_fix=False, auto_inst=False)
+
+        # check if we left breakage
+        if self.cache._depcache.broken_count:
+            Fix = apt_pkg.ProblemResolver(self.cache._depcache)
+            Fix.resolve_by_keep()
+        self.updates_changed()
+        self.treeview_update.queue_draw()
+        del actiongroup
+        self.setBusy(False)
 
     def save_state(self):
         """ save the state  (window-size for now) """
@@ -704,46 +805,36 @@
         self.dl_size = 0
 
         self.scrolledwindow_update.show()
-        origin_list = sorted(
-            self.list.pkgs, key=operator.attrgetter("importance"),
-            reverse=True)
-        for origin in origin_list:
-            self.store.append(['<b><big>%s</big></b>' % origin.description,
-                               origin.description, None, origin, True])
-            for pkg in self.list.pkgs[origin]:
-                name = xml.sax.saxutils.escape(pkg.name)
-                if not pkg.is_installed:
-                    name += _(" (New install)")
-                summary = xml.sax.saxutils.escape(getattr(pkg.candidate,
-                                                          "summary", ""))
-                if self.summary_before_name:
-                    contents = "%s\n<small>%s</small>" % (summary, name)
-                else:
-                    contents = "<b>%s</b>\n<small>%s</small>" % (name, summary)
-                #TRANSLATORS: the b stands for Bytes
-                size = _("(Size: %s)") % humanize_size(getattr(pkg.candidate,
-                                                               "size", 0))
-                installed_version = getattr(pkg.installed, "version", None)
-                candidate_version = getattr(pkg.candidate, "version", None)
-                if installed_version is not None:
-                    version = _("From version %(old_version)s "
-                                "to %(new_version)s") % {
-                                    "old_version": installed_version,
-                                    "new_version": candidate_version}
-                else:
-                    version = _("Version %s") % candidate_version
-                if self.show_versions:
-                    contents = "%s\n<small>%s %s</small>" % (contents, version,
-                                                             size)
-                else:
-                    contents = "%s <small>%s</small>" % (contents, size)
-                self.store.append([contents, pkg.name, pkg, None, True])
+
+        # add update groups and packages to self.store. Each row contains:
+        # row label (for screen reader),
+        # update data tuple (is_toplevel, group object, package object),
+        # update size,
+        # update selection state
+        for group in self.list.pkg_groups:
+            group_row = [
+                group.name,
+                (True, group, None),
+                humanize_size(group.get_total_size()),
+                True
+                ]
+            tree_parent = self.store.append(None, group_row)
+
+            for pkg in group.pkgs:
+                pkg_row = [
+                    pkg.name,
+                    (False, group, pkg),
+                    humanize_size(getattr(pkg.candidate, "size", 0)),
+                    True
+                    ]
+                self.store.append(tree_parent, pkg_row)
+
         self.treeview_update.set_model(self.store)
         self.update_count()
         self.setBusy(False)
         while Gtk.events_pending():
             Gtk.main_iteration()
-        self.refresh_updates_count()
+        self.updates_changed()
         return False
 
     def main(self):