Merge lp:~software-updates-spec/update-manager/group-by-applications into lp:update-manager

[Michael Terry]

This is a continuation of https://code.launchpad.net/~dylanmccall/update-manager/group-by-applications/+merge/112678

This implements the 'Available Updates' description pane in the Software Updates spec:
https://wiki.ubuntu.com/SoftwareUpdates#Expanded_presentation_of_updates

Notably, it replaces the current list of packages with groups of packages and highlights the description of the package rather than the debian package name.

It also no longer categorizes the updates by source. The only categorization it does is "security or not".

It's a big diff, I know. Sorry. Mostly visual changes. And I added tests for the non-visual bits.

One big unfinished part of the spec is that when two packages have the same description (e.g. "Transitional package") we don't append the debian package name (e.g. "Transitional package (unity-2d)"). This is technically difficult. Talking to mpt, he indicated that just showing the label without the parenthetical would be an acceptable workaround until we can implement that bit. Ideally packages wouldn't have such duplicate descriptions.

Test with: ./update-manager --data-dir=./data

[lp:~software-updates-spec/update-manager/group-by-applications]

2486. By Dylan McCall on 2013-01-22

Fixed application crashing when running with XDG_DATA_DIRS or XDG_CURRENT_DESKTOP unset.

[Michael Vogt (mvo)]

On Tue, Jan 22, 2013 at 06:25:25PM -0000, Michael Terry wrote:
> Michael Terry has proposed merging lp:~software-updates-spec/update-manager/group-by-applications into lp:update-manager.
>
> Requested reviews:
> Ubuntu Core Development Team (ubuntu-core-dev)
>
> For more details, see:
> https://code.launchpad.net/~software-updates-spec/update-manager/group-by-applications/+merge/144362
>
> This is a continuation of https://code.launchpad.net/~dylanmccall/update-manager/group-by-applications/+merge/112678
>
> This implements the 'Available Updates' description pane in the Software Updates spec:
> https://wiki.ubuntu.com/SoftwareUpdates#Expanded_presentation_of_updates

Thanks for this branch! I haven't really had a chance to test-run it
yet, my bandwidth right now is not very good. From looking at the diff
it looks good, I like the new tests and the new
"aptroot-group-testing"!

Some things I noticed during reading the diff, nothing that really
needs fixing, just tiny nit-picking.

[..]
> + def __init__(self, parent, dist=None):
> + self.dist = dist
> + if self.dist is None:
> + try:
> + self.dist = subprocess.check_output(
> + ["lsb_release", "-c", "-s"],
> + universal_newlines=True).strip()
> + except subprocess.CalledProcessError as e:
> + print("Error in lsb_release: %s" % e)
[..]

As a tiny nit-pick, this above could be written also via:

import platform
self.dist = platform.dist()[2]

[..]
> +import xml.sax.saxutils
..
> +def get_package_label(pkg):
> + """ this takes a package synopsis and uppercases the first word's
> + first letter
> + """
> + import xml.sax.saxutils
> + name = xml.sax.saxutils.escape(getattr(pkg.candidate, "summary",
> ""))
> + return capitalize_first_word(name)

Looks like one of the imports can be skipped. Or (probably better) we use
GLib.markup_escape_text() instead.

> + if len(cells) != 3 or \
> + not isinstance(cells[0], Gtk.CellRendererToggle) or \
> + not isinstance(cells[1], Gtk.CellRendererPixbuf) or \
> + not isinstance(cells[2], Gtk.CellRendererText):
> + return

Most of the other python code seems to put "(" around ")" the multi
line if statements, either is fine with me, just wanted to mention
it.

Cheers,
 Michael

[Michael Vogt (mvo)]

I just got the chance to play with it too, looks very nice indeed.

review: Approve

[lp:~software-updates-spec/update-manager/group-by-applications]

2487. By Michael Terry on 2013-01-23

merge from trunk

2488. By Michael Terry on 2013-01-23

add myself to UpdateList authors and update copyright in UpdateList and UpdatesAvailable

2489. By Michael Terry on 2013-01-23

fix nits from review

2490. By Michael Terry on 2013-01-23

sort entries case insensitively

[Michael Terry (mterry)]

OK, fixed the nits and added one more fix -- sorting entries case insensitively. I wanted mpt to review changes too just to make sure I didn't screw his vision up. He said he'd get to it on Friday likely.

[Matthew Paul Thomas (mpt)]

This looks excellent to me. The behavior of the tri-state checkboxes isn't perfect, but that can certainly wait.

review: Approve (design)

=== modified file 'UpdateManager/Core/MyCache.py'
--- UpdateManager/Core/MyCache.py	2012-06-28 00:10:23 +0000
+++ UpdateManager/Core/MyCache.py	2013-01-23 15:48:20 +0000
@@ -43,7 +43,6 @@
 import re
 import DistUpgrade.DistUpgradeCache
 from gettext import gettext as _
-from .UpdateList import UpdateOrigin
 
 SYNAPTIC_PINFILE = "/var/lib/synaptic/preferences"
 CHANGELOGS_POOL = "http://changelogs.ubuntu.com/changelogs/pool/"
@@ -133,59 +132,6 @@
         self._depcache.upgrade()
         return wouldDelete
 
-    def match_package_origin(self, pkg, matcher):
-        """ match 'pkg' origin against 'matcher', take versions between
-            installed.version and candidate.version into account too
-            Useful if installed pkg A v1.0 is available in both
-            -updates (as v1.2) and -security (v1.1). we want to display
-            it as a security update then
-        """
-        inst_ver = pkg._pkg.current_ver
-        cand_ver = self._depcache.get_candidate_ver(pkg._pkg)
-        # init matcher with candidate.version
-        update_origin = matcher[(None, None)]
-        verFileIter = None
-        for (verFileIter, index) in cand_ver.file_list:
-            if (verFileIter.archive, verFileIter.origin) in matcher:
-                indexfile = pkg._pcache._list.find_index(verFileIter)
-                if indexfile:  # and indexfile.IsTrusted:
-                    match = matcher[verFileIter.archive, verFileIter.origin]
-                    update_origin = match
-                    break
-        else:
-            # add a node for each origin/archive combination
-            if verFileIter and verFileIter.origin and verFileIter.archive:
-                matcher[verFileIter.archive, verFileIter.origin] = \
-                    UpdateOrigin(_("Other updates (%s)") % verFileIter.origin,
-                                 0)
-                update_origin = matcher[verFileIter.archive,
-                                        verFileIter.origin]
-        # if the candidate comes from a unknown source (e.g. a PPA) skip
-        # skip the shadow logic below as it would put e.g. a PPA package
-        # in "Recommended updates" when the version in the PPA
-        # is higher than the one in %s-updates
-        if update_origin.importance <= 0:
-            return update_origin
-        # for known packages, check if we have higher versions that
-        # "shadow" this one
-        for ver in pkg._pkg.version_list:
-            # discard is < than installed ver
-            if (inst_ver and
-                    apt_pkg.version_compare(ver.ver_str,
-                                            inst_ver.ver_str) <= 0):
-                #print("skipping '%s' " % ver.ver_str)
-                continue
-            # check if we have a match
-            for (verFileIter, index) in ver.file_list:
-                if (verFileIter.archive, verFileIter.origin) in matcher:
-                    indexfile = pkg._pcache._list.find_index(verFileIter)
-                    if indexfile:  # and indexfile.IsTrusted:
-                        match = matcher[verFileIter.archive,
-                                        verFileIter.origin]
-                        if match.importance > update_origin.importance:
-                            update_origin = match
-        return update_origin
-
     def _strip_epoch(self, verstr):
         " strip of the epoch "
         l = verstr.split(":")
@@ -356,9 +302,10 @@
     def get_changelog(self, name):
         " get the changelog file from the changelog location "
         origins = self[name].candidate.origins
-        self.all_changes[name] = _("Changes for the versions:\n"
+        self.all_changes[name] = _("Changes for %s versions:\n"
                                    "Installed version: %s\n"
                                    "Available version: %s\n\n") % (
+                                       name,
                                        getattr(self[name].installed, "version",
                                                None),
                                        self[name].candidate.version)
 
=== modified file 'UpdateManager/Core/UpdateList.py'
--- UpdateManager/Core/UpdateList.py	2013-01-22 17:59:35 +0000
+++ UpdateManager/Core/UpdateList.py	2013-01-23 15:48:20 +0000
@@ -1,9 +1,11 @@
 # UpdateList.py
 # -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*-
 #
-#  Copyright (c) 2004-2012 Canonical
+#  Copyright (c) 2004-2013 Canonical
 #
 #  Author: Michael Vogt <[e-mail]>
+#          Dylan McCall <[e-mail]>
+#          Michael Terry <[e-mail]>
 #
 #  This program is free software; you can redistribute it and/or
 #  modify it under the terms of the GNU General Public License as
@@ -22,36 +24,131 @@
 
 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 itertools
+import platform
+import subprocess
+import os
 import random
-import subprocess
-
-
-class UpdateOrigin(object):
+import glob
+
+from gi.repository import Gio
+
+from UpdateManager.Core import utils
+
+
+class UpdateItem():
+    def __init__(self, pkg, name, icon):
+        self.icon = icon
+        self.name = name
+        self.pkg = pkg
+
+
+class UpdateGroup(UpdateItem):
+    def __init__(self, pkg, name, icon):
+        UpdateItem.__init__(self, pkg, name, icon)
+        self._items = set()
+        self.core_item = None
+        if pkg is not None:
+            self.core_item = UpdateItem(pkg, name, icon)
+            self._items.add(self.core_item)
+
+    @property
+    def items(self):
+        all_items = []
+        all_items.extend(self._items)
+        return sorted(all_items, key=lambda a: a.name.lower())
+
+    def add(self, pkg):
+        name = utils.get_package_label(pkg)
+        icon = Gio.ThemedIcon.new("package")
+        self._items.add(UpdateItem(pkg, name, icon))
+
+    def contains(self, item):
+        return item in self._items
+
+    def _is_dependency_helper(self, cache, pkg, dep, seen=set()):
+        if pkg is None or pkg.candidate is None or pkg in seen:
+            return False
+        elif pkg.name == dep.name:
+            return True
+        seen.add(pkg)
+        candidate = pkg.candidate
+        dependencies = candidate.get_dependencies('Depends', 'Recommends')
+        for dependency_pkg in itertools.chain.from_iterable(dependencies):
+            if dependency_pkg.name in cache and \
+               self._is_dependency_helper(cache, cache[dependency_pkg.name],
+                                          dep, seen):
+                return True
+        return False
+
+    def is_dependency(self, cache, maybe_dep):
+        # This is a recursive dependency check.  TODO: We do this many times
+        # when grouping packages, and it could be made more efficient.
+        seen = set()
+        for item in self._items:
+            if self._is_dependency_helper(cache, item.pkg, maybe_dep,
+                                          seen=seen):
+                return True
+        return False
+
+    def packages_are_selected(self):
+        for item in self.items:
+            if item.pkg.marked_install or item.pkg.marked_upgrade:
+                return True
+        return False
+
+    def selection_is_inconsistent(self):
+        pkgs_installing = [item for item in self.items
+            if item.pkg.marked_install or item.pkg.marked_upgrade]
+        return (len(pkgs_installing) > 0 and
+                len(pkgs_installing) < len(self.items))
+
+    def get_total_size(self):
+        size = 0
+        for item in self.items:
+            size += getattr(item.pkg.candidate, "size", 0)
+        return size
+
+
+class UpdateApplicationGroup(UpdateGroup):
+    def __init__(self, pkg, application):
+        name = application.get_display_name()
+        icon = application.get_icon()
+        super(UpdateApplicationGroup, self).__init__(pkg, name, icon)
+
+
+class UpdatePackageGroup(UpdateGroup):
+    def __init__(self, pkg):
+        name = utils.get_package_label(pkg)
+        icon = Gio.ThemedIcon.new("package")
+        super(UpdatePackageGroup, self).__init__(pkg, name, icon)
+
+
+class UpdateSystemGroup(UpdateGroup):
+    def __init__(self, cache):
+        # Translators: the %s is a distro name, like 'Ubuntu' and 'base' as in
+        # the core components and packages.
+        name = _("%s base") % utils.get_ubuntu_flavor_name(cache=cache)
+        icon = Gio.ThemedIcon.new("distributor-logo")
+        super(UpdateSystemGroup, self).__init__(None, name, icon)
+
+
+class UpdateOrigin():
     def __init__(self, desc, importance):
         self.packages = []
         self.importance = importance
         self.description = desc
 
 
-class OriginsImportance:
-    # filled in by us
-    SECURITY = 10
-    UPDATES = 9
-    PROPOSED = 8
-    BACKPORTS = 7
-    ARCHIVE = 6
-    # this is filled 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
@@ -64,6 +161,8 @@
     # the file that contains the uniq machine id
     UNIQ_MACHINE_ID_FILE = "/var/lib/dbus/machine-id"
 
+    APP_INSTALL_PATH = "/usr/share/app-install/desktop"
+
     # the configuration key to turn phased-updates always on
     ALWAYS_INCLUDE_PHASED_UPDATES = (
         "Update-Manager::Always-Include-Phased-Updates")
@@ -71,47 +170,111 @@
     NEVER_INCLUDE_PHASED_UPDATES = (
         "Update-Manager::Never-Include-Phased-Updates")
 
-    def __init__(self, parent):
-        # a map of packages under their origin
-        try:
-            dist = subprocess.check_output(
-                ["lsb_release", "-c", "-s"], universal_newlines=True).strip()
-        except subprocess.CalledProcessError as e:
-            print("Error in lsb_release: %s" % e)
-            dist = None
+    def __init__(self, parent, dist=None):
+        self.dist = dist if dist else platform.dist()[2]
         self.distUpgradeWouldDelete = 0
-        self.pkgs = {}
+        self.update_groups = []
+        self.security_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 = [
-            (None, None, _("Other updates"), OriginsImportance.OTHER_UNKNOWN)
-        ]
-        if dist:
-            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),
-            ]
-        matcher = {}
-        for (origin, archive, desc, importance) in matcher_templates:
-            matcher[(origin, archive)] = UpdateOrigin(desc, importance)
-        return matcher
-
-    def is_ignored_phased_update(self, pkg):
+        if 'XDG_DATA_DIRS' in os.environ and os.environ['XDG_DATA_DIRS']:
+            data_dirs = os.environ['XDG_DATA_DIRS']
+        else:
+            data_dirs= '/usr/local/share/:/usr/share/'
+        self.application_dirs = [os.path.join(base, 'applications')
+                                 for base in data_dirs.split(':')]
+
+        if 'XDG_CURRENT_DESKTOP' in os.environ:
+            self.current_desktop = os.environ.get('XDG_CURRENT_DESKTOP')
+        else:
+            self.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(self.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 _is_security_update(self, pkg):
+        """ This will test if the pkg is a security update.
+            This includes if there is a newer version in -updates, but also
+            an older update available in -security.  For example, if
+            installed pkg A v1.0 is available in both -updates (as v1.2) and
+            -security (v1.1). we want to display it as a security update.
+
+            :return: True if the update comes from the security pocket
+        """
+        if not self.dist:
+            return False
+        inst_ver = pkg._pkg.current_ver
+        for ver in pkg._pkg.version_list:
+            # discard is < than installed ver
+            if (inst_ver and
+                    apt.apt_pkg.version_compare(ver.ver_str,
+                                                inst_ver.ver_str) <= 0):
+                continue
+            # check if we have a match
+            for (verFileIter, index) in ver.file_list:
+                if verFileIter.archive == "%s-security" % self.dist and \
+                        verFileIter.origin == "Ubuntu":
+                    indexfile = pkg._pcache._list.find_index(verFileIter)
+                    if indexfile:  # and indexfile.IsTrusted:
+                        return True
+        return False
+
+    def _is_ignored_phased_update(self, pkg):
         """ This will test if the pkg is a phased update and if
             it needs to get installed or ignored.
 
@@ -142,15 +305,88 @@
                 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-image', 'linux-headers-generic',
+                'linux-image-generic', 'linux-generic',
+                'linux-headers-generic-pae', 'linux-image-generic-pae',
+                'linux-generic-pae', 'linux-headers-omap', 'linux-image-omap',
+                'linux-omap', 'linux-headers-server', 'linux-image-server',
+                'linux-server', 'linux-signed-image-generic',
+                'linux-signed-generic', 'linux-headers-virtual',
+                'linux-image-virtual', 'linux-virtual',
+                'linux-image-extra-virtual']
+
+    def _make_groups(self, cache, pkgs):
+        pkgs_by_source = {}
+        ungrouped_pkgs = []
+        app_groups = []
+        pkg_groups = []
+
+        # Index packages by source package name
+        for pkg in 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)
+                    app_groups.append(app_group)
+                else:
+                    ungrouped_pkgs.append(pkg)
+
+        # Stick together applications and their immediate dependencies
+        for pkg in list(ungrouped_pkgs):
+            dep_groups = []
+            for group in app_groups:
+                if group.is_dependency(cache, pkg):
+                    dep_groups.append(group)
+                    if len(dep_groups) > 1:
+                        break
+            if len(dep_groups) == 1:
+                dep_groups[0].add(pkg)
+                ungrouped_pkgs.remove(pkg)
+
+        # Separate out system base packages
+        system_group = None
+        meta_group = UpdateGroup(None, None, None)
+        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])
+        for pkg in ungrouped_pkgs:
+            if meta_group.contains(pkg) or meta_group.is_dependency(cache, pkg):
+                if system_group is None:
+                    system_group = UpdateSystemGroup(cache)
+                system_group.add(pkg)
+            else:
+                pkg_groups.append(UpdatePackageGroup(pkg))
+
+        app_groups.sort(key=lambda a: a.name.lower())
+        pkg_groups.sort(key=lambda a: a.name.lower())
+        if system_group:
+            pkg_groups.append(system_group)
+
+        return app_groups + pkg_groups
+
     def update(self, cache):
         self.held_back = []
 
         # do the upgrade
         self.distUpgradeWouldDelete = cache.saveDistUpgrade()
 
-        #dselect_upgrade_origin = UpdateOrigin(_("Previous selected"), 1)
+        security_pkgs = []
+        upgrade_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:
@@ -159,22 +395,22 @@
                     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)):
+                is_security_update = self._is_security_update(pkg)
+                if (not is_security_update 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)
+                if is_security_update:
+                    security_pkgs.append(pkg)
+                else:
+                    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"))
-        self.keepcount = cache._depcache.keep_count
+
+        self.update_groups = self._make_groups(cache, upgrade_pkgs)
+        self.security_groups = self._make_groups(cache, security_pkgs)
 
=== modified file 'UpdateManager/Core/utils.py'
--- UpdateManager/Core/utils.py	2012-12-14 23:03:19 +0000
+++ UpdateManager/Core/utils.py	2013-01-23 15:48:20 +0000
@@ -1,9 +1,10 @@
 # utils.py
 # -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*-
 #
-#  Copyright (c) 2004-2008 Canonical
+#  Copyright (c) 2004-2013 Canonical
 #
-#  Author: Michael Vogt <[e-mail]>
+#  Authors: Michael Vogt <[e-mail]>
+#           Michael Terry <[e-mail]>
 #
 #  This program is free software; you can redistribute it and/or
 #  modify it under the terms of the GNU General Public License as
@@ -27,6 +28,7 @@
 from stat import (S_IMODE, ST_MODE, S_IXUSR)
 from math import ceil
 
+import apt
 import apt_pkg
 apt_pkg.init_config()
 
@@ -58,6 +60,7 @@
     from urlparse import urlsplit
 
 from copy import copy
+from gi.repository import GLib
 
 
 class ExecutionTime(object):
@@ -402,36 +405,56 @@
         return None
 
 
-def get_ubuntu_flavor():
+def get_ubuntu_flavor(cache=None):
     """ try to guess the flavor based on the running desktop """
     # this will (of course) not work in a server environment,
     # but the main use case for this is to show the right
-    # release notes
-    # TODO: actually examine which meta packages are installed, like
-    # DistUpgrade/DistUpgradeCache.py does and use that to choose a flavor.
-    denv = os.environ.get("DESKTOP_SESSION", "")
-    if "gnome" in denv:
-        return "ubuntu"
-    elif "kde" in denv:
-        return "kubuntu"
-    elif "xfce" in denv or "xubuntu" in denv:
-        return "xubuntu"
-    elif "LXDE" in denv or "Lubuntu" in denv:
-        return "lubuntu"
-    # default to ubuntu if nothing more specific is found
-    return "ubuntu"
-
-
-def get_ubuntu_flavor_name():
-    flavor = get_ubuntu_flavor()
-    if flavor == "kubuntu":
-        return "Kubuntu"
-    elif flavor == "xubuntu":
-        return "Xubuntu"
-    elif flavor == "lubuntu":
-        return "Lubuntu"
+    # release notes.
+    pkg = get_ubuntu_flavor_package(cache=cache)
+    return pkg.split('-', 1)[0]
+
+
+def _load_meta_pkg_list():
+    # This could potentially introduce a circular dependency, but the config
+    # parser logic is simple, and doesn't rely on any UpdateManager code.
+    from DistUpgrade.DistUpgradeConfigParser import DistUpgradeConfig
+    parser = DistUpgradeConfig('/usr/share/ubuntu-release-upgrader')
+    return parser.getlist('Distro', 'MetaPkgs')
+
+
+def get_ubuntu_flavor_package(cache=None):
+    """ try to guess the flavor metapackage based on the running desktop """
+    # From spec, first if ubuntu-desktop is installed, use that.
+    # Second, grab first installed one from DistUpgrade.cfg.
+    # Lastly, fallback to ubuntu-desktop again.
+    meta_pkgs = ['ubuntu-desktop']
+
+    try:
+        meta_pkgs.extend(sorted(_load_meta_pkg_list()))
+    except Exception as e:
+        print('Could not load list of meta packages:', e)
+
+    if cache is None:
+        cache = apt.Cache()
+    for meta_pkg in meta_pkgs:
+        cache_pkg = cache[meta_pkg] if meta_pkg in cache else None
+        if cache_pkg and cache_pkg.is_installed:
+            return meta_pkg
+    return 'ubuntu-desktop'
+
+
+def get_ubuntu_flavor_name(cache=None):
+    """ try to guess the flavor name based on the running desktop """
+    pkg = get_ubuntu_flavor_package(cache=cache)
+    lookup = {'ubuntustudio-desktop': 'Ubuntu Studio'}
+    if pkg in lookup:
+        return lookup[pkg]
+    elif pkg.endswith('-desktop'):
+        return capitalize_first_word(pkg.rsplit('-desktop', 1)[0])
+    elif pkg.endswith('-netbook'):
+        return capitalize_first_word(pkg.rsplit('-netbook', 1)[0])
     else:
-        return "Ubuntu"
+        return 'Ubuntu'
 
 
 # Unused by update-manager, but still used by ubuntu-release-upgrader
@@ -523,6 +546,22 @@
     return True
 
 
+def capitalize_first_word(string):
+    """ this uppercases the first word's first letter
+    """
+    if len(string) > 1 and string[0].isalpha() and not string[0].isupper():
+        return string[0].capitalize() + string[1:]
+    return string
+
+
+def get_package_label(pkg):
+    """ this takes a package synopsis and uppercases the first word's
+        first letter
+    """
+    name = GLib.markup_escape_text(getattr(pkg.candidate, "summary", ""))
+    return capitalize_first_word(name)
+
+
 if __name__ == "__main__":
     #print(mirror_from_sources_list())
     #print(on_battery())
 
=== modified file 'UpdateManager/UpdatesAvailable.py'
--- UpdateManager/UpdatesAvailable.py	2012-12-13 20:39:37 +0000
+++ UpdateManager/UpdatesAvailable.py	2013-01-23 15:48:20 +0000
@@ -1,7 +1,7 @@
 # UpdatesAvailable.py
 # -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*-
 #
-#  Copyright (c) 2004-2012 Canonical
+#  Copyright (c) 2004-2013 Canonical
 #                2004 Michiel Sikkes
 #                2005 Martin Willemoes Hansen
 #                2010 Mohamed Amine IL Idrissi
@@ -12,6 +12,7 @@
 #          Mohamed Amine IL Idrissi <[e-mail]>
 #          Alex Launi <[e-mail]>
 #          Michael Terry <[e-mail]>
+#          Dylan McCall <[e-mail]>
 #
 #  This program is free software; you can redistribute it and/or
 #  modify it under the terms of the GNU General Public License as
@@ -30,6 +31,7 @@
 
 from __future__ import absolute_import, print_function
 
+from gi.repository import GLib
 from gi.repository import Gtk
 from gi.repository import Gdk
 from gi.repository import GObject
@@ -46,18 +48,17 @@
 import os
 import re
 import logging
-import operator
 import subprocess
 import time
 import threading
-import xml.sax.saxutils
 
 from gettext import gettext as _
 from gettext import ngettext
 
 
-from .Core.utils import humanize_size
+from .Core.utils import (get_package_label, humanize_size)
 from .Core.AlertWatcher import AlertWatcher
+from .Core.UpdateList import UpdateSystemGroup
 
 from DistUpgrade.DistUpgradeCache import NotEnoughFreeSpaceError
 
@@ -70,15 +71,126 @@
 
 # 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)
 
 # NetworkManager enums
 from .Core.roam import NetworkManagerHelper
 
 
+class UpdateData():
+    def __init__(self, groups, group, item):
+        self.groups = groups if groups else []
+        self.group = group
+        self.item = item
+
+
+class CellAreaPackage(Gtk.CellAreaBox):
+    """This CellArea lays our package cells side by side, without allocating
+       width for a cell if it isn't present (like icons for header labels).
+    """
+
+    def __init__(self, indent_toplevel=False):
+        Gtk.CellAreaBox.__init__(self)
+        self.indent_toplevel = indent_toplevel
+        self.column = None
+        self.toggle_size = None
+        self.pixbuf_size = None
+        self.cached_cell_start = {}
+
+    def do_foreach_alloc(self, context, widget, cell_area_in, bg_area_in,
+                         callback):
+        # First, gather cell renderers and make sure they are what we expect
+        cells = []
+        def gather(cell, data):
+            cells.append(cell)
+        self.foreach(gather, None)
+        if (len(cells) != 3 or
+                not isinstance(cells[0], Gtk.CellRendererToggle) or
+                not isinstance(cells[1], Gtk.CellRendererPixbuf) or
+                not isinstance(cells[2], Gtk.CellRendererText)):
+            return
+        toggle = cells[0]
+        pixbuf = cells[1]
+        text = cells[2]
+
+        # Now just grab some size info
+        cell_area = cell_area_in.copy()
+        bg_area = bg_area_in.copy()
+        spacing = self.get_property("spacing")
+        gicon = pixbuf.get_property("gicon")
+        cell_start = self.get_cell_start(widget)
+        orig_end = cell_area.width + cell_area.x
+        if self.toggle_size is None:
+            toggle_min, self.toggle_size = toggle.get_preferred_width(widget)
+        if gicon and self.pixbuf_size is None:
+            pixbuf_min, self.pixbuf_size = pixbuf.get_preferred_width(widget)
+
+        # And finally, start handling each cell        
+
+        cur_path = self.get_current_path_string()
+        depth = Gtk.TreePath.new_from_string(cur_path).get_depth()
+        if gicon is not None and self.indent_toplevel:
+            # if not a header, align with header rows
+            depth = depth + 1
+        if depth == 1:
+            cell_area.x = cell_start
+        elif depth == 2:
+            cell_area.x = cell_start + self.toggle_size + spacing
+        elif depth == 3:
+            # Oddly, cells don't line up if we simply use spacing * 2
+            cell_area.x = cell_start + self.toggle_size * 2 + spacing + 1
+        cell_area.width = self.toggle_size
+        if callback(cells[0], cell_area.copy(), bg_area.copy()):
+            return
+
+        cell_area.x = cell_area.x + cell_area.width + spacing
+        if gicon is None:
+            cell_area.width = 0
+        else:
+            cell_area.width = self.pixbuf_size
+        if callback(cells[1], cell_area.copy(), bg_area.copy()):
+            return
+
+        if gicon is not None:
+            cell_area.x = cell_area.x + cell_area.width + spacing
+        cell_area.width = orig_end - cell_area.x
+        if callback(cells[2], cell_area.copy(), bg_area.copy()):
+            return
+
+    def do_event(self, context, widget, event, cell_area, flags):
+        # This override is just to trick our parent implementation into
+        # allowing clicks on toggle cells when they are where the expanders
+        # usually are.  It doesn't expect that, so we expand the cell_area
+        # here to be equivalent to bg_area.
+        cell_start = self.get_cell_start(widget)
+        cell_area.width = cell_area.width + cell_area.x - cell_start
+        cell_area.x = cell_start
+        return Gtk.CellAreaBox.do_event(self, context, widget, event,
+                                        cell_area, flags)
+
+    def get_cell_start(self, widget):
+        if not self.column:
+            return 0
+        else:
+            val = GObject.Value()
+            val.init(int)
+            widget.style_get_property("horizontal-separator", val)
+            h_sep = val.get_int()
+            widget.style_get_property("grid-line-width", val)
+            line_width = val.get_int()
+            cell_start = self.column.get_x_offset() - h_sep - line_width
+            if not self.indent_toplevel:  # i.e. if no headers
+                widget.style_get_property("expander-size", val)
+                spacing = self.get_property("spacing")
+                # Hardcode 4 because GTK+ hardcodes 4 internally
+                cell_start = cell_start + val.get_int() + 4 + spacing
+            return cell_start
+
+
 class UpdatesAvailable(SimpleGtkbuilderApp):
 
     def __init__(self, app, header=None, desc=None):
@@ -134,38 +246,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.FIXED)
+        restart_column.set_fixed_width(20)
+        self.treeview_update.append_column(restart_column)
+        restart_column.set_cell_data_func(restart_icon_renderer,
+                                          self.restart_icon_renderer_data_func)
+
+        pkg_area = CellAreaPackage(bool(self.list.security_groups))
+        pkg_column = Gtk.TreeViewColumn.new_with_area(pkg_area)
+        pkg_area.column = pkg_column
+        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)
 
         # setup the help viewer and disable the help button if there
         # is no viewer available
@@ -210,26 +355,97 @@
             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)
+    def restart_icon_renderer_data_func(self, cell_layout, renderer, model,
+                                        iter, data):
+        def pkg_requires_restart(pkg):
+            restart_condition = pkg.candidate.record.get('XB-Restart-Required')
+            return restart_condition == 'system'
+
+        data = model.get_value(iter, LIST_UPDATE_DATA)
+        path = model.get_path(iter)
+
+        requires_restart = False
+        if data.item:
+            requires_restart = pkg_requires_restart(data.item.pkg)
+        elif data.group:
+            if not self.treeview_update.row_expanded(path):
+                # A package in the group requires restart
+                for group_item in data.group.items:
+                    if pkg_requires_restart(group_item.pkg):
+                        requires_restart = True
+                        break
+
+        # 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:
-            renderer.set_property("activatable", True)
+            gicon = None
+        renderer.set_property("gicon", gicon)
+
+    def pkg_toggle_renderer_data_func(self, cell_layout, renderer, model, iter,
+                                      data):
+        data = model.get_value(iter, LIST_UPDATE_DATA)
+
+        activatable = False
+        inconsistent = False
+        if data.item:
+            activatable = data.item.name not in self.list.held_back
+            inconsistent = False
+        elif data.group:
+            activatable = True
+            inconsistent = data.group.selection_is_inconsistent()
+        elif data.groups:
+            activatable = True
+            inconsistent = False
+            saw_install = None
+            for group in data.groups:
+                for item in group.items:
+                    pkg = item.pkg
+                    this_install = pkg.marked_install or pkg.marked_upgrade
+                    if saw_install is not None and saw_install != this_install:
+                        inconsistent = True
+                        break
+                    saw_install = this_install
+                if inconsistent:
+                    break
+
+        # The "active" attribute is already set via LIST_TOGGLE_ACTIVE in the
+        # tree model, so we don't set it here.
+        renderer.set_property("activatable", activatable)
+        renderer.set_property("inconsistent", inconsistent)
+
+    def pkg_icon_renderer_data_func(self, cell_layout, renderer, model, iter,
+                                    data):
+        data = model.get_value(iter, LIST_UPDATE_DATA)
+
+        gicon = None
+        if data.group:
+            gicon = data.group.icon
+        elif data.item:
+            gicon = data.item.icon
+
+        renderer.set_property("gicon", gicon)
+
+    def pkg_label_renderer_data_func(self, cell_layout, renderer, model, iter,
+                                     data):
+        data = model.get_value(iter, LIST_UPDATE_DATA)
+        name = model.get_value(iter, LIST_NAME)
+
+        if data.group:
+            markup = name
+        elif data.item:
+            markup = name
+        else:  # header
+            markup = "<b>%s</b>" % name
+
+        renderer.set_property("markup", markup)
 
     def set_changes_buffer(self, changes_buffer, text, name, srcpkg):
         changes_buffer.set_text("")
@@ -264,16 +480,21 @@
         iter = model.get_iter(path)
 
         # set descr
-        pkg = model.get_value(iter, LIST_PKG)
-        if (pkg is None or pkg.candidate is None or
-                pkg.candidate.description is None):
+        data = model.get_value(iter, LIST_UPDATE_DATA)
+        item = data.item
+        if (item is None and data.group is not None and
+                data.group.core_item is not None):
+            item = data.group.core_item
+        if (item is None or item.pkg is None or 
+                item.pkg.candidate is None or
+                item.pkg.candidate.description is None):
             changes_buffer = self.textview_changes.get_buffer()
             changes_buffer.set_text("")
             desc_buffer = self.textview_descr.get_buffer()
             desc_buffer.set_text("")
             self.notebook_details.set_sensitive(False)
             return
-        long_desc = pkg.candidate.description
+        long_desc = item.pkg.candidate.description
         self.notebook_details.set_sensitive(True)
         # do some regular expression magic on the description
         # Add a newline before each bullet
@@ -290,7 +511,7 @@
         desc_buffer.set_text(long_desc)
 
         # now do the changelog
-        name = model.get_value(iter, LIST_NAME)
+        name = item.pkg.name
         if name is None:
             return
 
@@ -334,11 +555,10 @@
                 button.disconnect(id)
         # check if we still are in the right pkg (the download may have taken
         # some time and the user may have clicked on a new pkg)
-        path = widget.get_cursor()[0]
-        if path is None:
+        now_path = widget.get_cursor()[0]
+        if now_path is None:
             return
-        now_name = widget.get_model()[path][LIST_NAME]
-        if name != now_name:
+        if path != now_path:
             return
         # display NEWS.Debian first, then the changelog
         changes = ""
@@ -350,7 +570,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 +581,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 +596,36 @@
             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,
+            data = model.get_value(row.iter, LIST_UPDATE_DATA)
+            if data.groups is not None or data.group is not None:
+                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 +641,21 @@
         while Gtk.events_pending():
             Gtk.main_iteration()
 
-    def refresh_updates_count(self):
+    def _mark_selected_updates(self):
+        def foreach_cb(model, path, iter, data):
+            data = model.get_value(iter, LIST_UPDATE_DATA)
+            active = False
+            if data.item:
+                active = data.item.pkg.marked_install or \
+                         data.item.pkg.marked_upgrade
+            elif data.group:
+                active = data.group.packages_are_selected()
+            elif data.groups:
+                active = any([g.packages_are_selected() for g in data.groups])
+            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 +696,14 @@
             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()
 
         text_header = None
         text_desc = None
@@ -486,8 +728,6 @@
         self.notebook_details.set_sensitive(True)
         self.treeview_update.set_sensitive(True)
         self.button_install.grab_default()
-        self.treeview_update.set_cursor(Gtk.TreePath.new_from_string("1"),
-                                        None, False)
         self.label_header.set_label(text_header)
 
         if text_desc is not None:
@@ -573,14 +813,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
@@ -590,7 +830,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()
 
@@ -613,63 +853,37 @@
             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)
+        data = 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 data.groups or data.group:
+            if data.groups:
+                self.toggle_from_groups(data.groups)
+            elif data.group:
+                self.toggle_from_groups([data.group])
+            self.updates_changed()
             self.treeview_update.queue_draw()
             return
-        if pkg is None or pkg.name in self.list.held_back:
+        if (data.item is None or data.item.pkg is None or
+                data.item.pkg.name in self.list.held_back):
             return False
         self.setBusy(True)
         # update the cache
-        if pkg.marked_install or pkg.marked_upgrade:
-            pkg.mark_keep()
+        if data.item.pkg.marked_install or data.item.pkg.marked_upgrade:
+            data.item.pkg.mark_keep()
             if self.cache._depcache.broken_count:
                 Fix = apt_pkg.ProblemResolver(self.cache._depcache)
                 Fix.resolve_by_keep()
         else:
             try:
-                pkg.mark_install()
+                data.item.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):
@@ -677,7 +891,29 @@
         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_groups(self, groups):
+        self.setBusy(True)
+        actiongroup = apt_pkg.ActionGroup(self.cache._depcache)
+
+        # Deselect all updates if any in group are selected
+        keep_packages = any([g.packages_are_selected() for g in groups])
+        for group in groups:
+            for item in group.items:
+                if keep_packages:
+                    item.pkg.mark_keep()
+                elif not item.pkg.name in self.list.held_back:
+                    item.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) """
@@ -693,9 +929,58 @@
         expanded = self.expander_details.get_expanded()
         self.window_main.set_resizable(expanded)
         if w > 0 and h > 0 and expanded:
-            self.window_main.resize(w, h)
+            # There is a race here.  If we immediately resize, it often doesn't
+            # take.  Using idle_add helps, but we *still* occasionally don't
+            # restore the size correctly.  Help needed to track this down!
+            GLib.idle_add(lambda: self.window_main.resize(w, h))
         return False
 
+    def _add_header(self, name, groups):
+        total_size = 0
+        for group in groups:
+            total_size = total_size + group.get_total_size()
+        header_row = [
+            name,
+            UpdateData(groups, None, None),
+            humanize_size(total_size),
+            True
+        ]
+        return self.store.append(None, header_row)
+
+    def _add_groups(self, groups):
+        # 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 groups:
+            if not group.items:
+                continue
+
+            group_is_item = None
+            if not isinstance(group, UpdateSystemGroup) and \
+                    len(group.items) == 1:
+                group_is_item = group.items[0]
+
+            group_row = [
+                group.name,
+                UpdateData(None, group, group_is_item),
+                humanize_size(group.get_total_size()),
+                True
+            ]
+            group_iter = self.store.append(None, group_row)
+
+            if group_is_item:
+                continue
+            for item in group.items:
+                item_row = [
+                    item.name,
+                    UpdateData(None, None, item),
+                    humanize_size(getattr(item.pkg.candidate, "size", 0)),
+                    True
+                ]
+                self.store.append(group_iter, item_row)
+
     def fillstore(self):
         # use the watch cursor
         self.setBusy(True)
@@ -706,46 +991,22 @@
         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 security and update groups to self.store
+        if self.list.security_groups:
+            self._add_header(_("Security updates"), self.list.security_groups)
+            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)
+        if self.list.update_groups:
+            self._add_groups(self.list.update_groups)
+
         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):
 
=== modified file 'data/com.ubuntu.update-manager.gschema.xml.in'
--- data/com.ubuntu.update-manager.gschema.xml.in	2012-05-31 22:28:41 +0000
+++ data/com.ubuntu.update-manager.gschema.xml.in	2013-01-23 15:48:20 +0000
@@ -6,12 +6,12 @@
       <description>Stores the state of the expander that contains the list of changes and the description</description>
     </key>
     <key name="window-width" type="i">
-      <default>0</default>
+      <default>1</default>
       <summary>The window width</summary>
       <description>Stores the width of the update-manager dialog</description>
     </key>
     <key name="window-height" type="i">
-      <default>0</default>
+      <default>400</default>
       <summary>The window height</summary>
       <description>Stores the height of the update-manager dialog</description>
     </key>
 
=== modified file 'data/gtkbuilder/UpdateManager.ui'
--- data/gtkbuilder/UpdateManager.ui	2012-12-14 22:11:25 +0000
+++ data/gtkbuilder/UpdateManager.ui	2013-01-23 15:48:20 +0000
@@ -112,11 +112,13 @@
                         <property name="visible">True</property>
                         <property name="can_focus">True</property>
                         <property name="shadow_type">in</property>
+                        <property name="min_content_height">100</property>
                         <child>
                           <object class="GtkTreeView" id="treeview_update">
                             <property name="visible">True</property>
                             <property name="can_focus">True</property>
                             <property name="headers_visible">False</property>
+                            <property name="headers_clickable">False</property>
                             <property name="rules_hint">True</property>
                             <child internal-child="accessible">
                               <object class="AtkObject" id="treeview_update-atkobject">
@@ -470,11 +472,9 @@
             <child>
               <object class="GtkButton" id="button_settings">
                 <property name="label" translatable="yes">_Settings...</property>
-                <property name="use_action_appearance">False</property>
                 <property name="visible">True</property>
                 <property name="can_focus">True</property>
                 <property name="receives_default">True</property>
-                <property name="use_action_appearance">False</property>
                 <property name="use_underline">True</property>
                 <signal name="clicked" handler="on_button_settings_clicked" swapped="no"/>
               </object>
@@ -488,15 +488,13 @@
             <child>
               <object class="GtkButton" id="button_close">
                 <property name="label">gtk-cancel</property>
-                <property name="use_action_appearance">False</property>
                 <property name="visible">True</property>
                 <property name="can_focus">True</property>
                 <property name="can_default">True</property>
                 <property name="receives_default">True</property>
-                <property name="use_action_appearance">False</property>
                 <property name="use_stock">True</property>
+                <accelerator key="W" signal="clicked" modifiers="GDK_CONTROL_MASK"/>
                 <accelerator key="Q" signal="clicked" modifiers="GDK_CONTROL_MASK"/>
-                <accelerator key="W" signal="clicked" modifiers="GDK_CONTROL_MASK"/>
               </object>
               <packing>
                 <property name="expand">False</property>
@@ -507,13 +505,11 @@
             <child>
               <object class="GtkButton" id="button_install">
                 <property name="label" translatable="yes">_Install Now</property>
-                <property name="use_action_appearance">False</property>
                 <property name="visible">True</property>
                 <property name="can_focus">True</property>
                 <property name="can_default">True</property>
                 <property name="has_default">True</property>
                 <property name="receives_default">True</property>
-                <property name="use_action_appearance">False</property>
                 <property name="image">image-apply</property>
                 <property name="use_underline">True</property>
                 <signal name="clicked" handler="on_button_install_clicked" swapped="no"/>
 
=== added directory 'tests/aptroot-grouping-test'
=== added directory 'tests/aptroot-grouping-test/etc'
=== added directory 'tests/aptroot-grouping-test/etc/apt'
=== added file 'tests/aptroot-grouping-test/etc/apt/sources.list'
--- tests/aptroot-grouping-test/etc/apt/sources.list	1970-01-01 00:00:00 +0000
+++ tests/aptroot-grouping-test/etc/apt/sources.list	2013-01-23 15:48:20 +0000
@@ -0,0 +1,2 @@
+deb http://archive.ubuntu.com/ubuntu lucid main
+deb http://archive.ubuntu.com/ubuntu lucid-security main
 
=== added symlink 'tests/aptroot-grouping-test/etc/apt/trusted.gpg'
=== target is u'/etc/apt/trusted.gpg'
=== added directory 'tests/aptroot-grouping-test/var'
=== added directory 'tests/aptroot-grouping-test/var/cache'
=== added directory 'tests/aptroot-grouping-test/var/lib'
=== added directory 'tests/aptroot-grouping-test/var/lib/apt'
=== added directory 'tests/aptroot-grouping-test/var/lib/apt/lists'
=== added file 'tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_Release'
--- tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_Release	1970-01-01 00:00:00 +0000
+++ tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_Release	2013-01-23 15:48:20 +0000
@@ -0,0 +1,9 @@
+Origin: Ubuntu
+Label: Ubuntu
+Suite: lucid-security
+Version: 10.04
+Codename: lucid
+Date: Thu, 29 Apr 2010 17:24:55 UTC
+Architectures: amd64 armel i386 ia64 powerpc sparc
+Components: main restricted universe multiverse
+Description: Ubuntu Lucid 10.04
 
=== added file 'tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_Release.gpg'
--- tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_Release.gpg	1970-01-01 00:00:00 +0000
+++ tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_Release.gpg	2013-01-23 15:48:20 +0000
@@ -0,0 +1,7 @@
+-----BEGIN PGP SIGNATURE-----
+Version: GnuPG v1.4.6 (GNU/Linux)
+
+iD8DBQBL2cDzQJdur0N9BbURAmk2AJ9ungOjKn0ektAH87KhRIHht+1cDQCfck7P
+ZoIb2P0v2PEqa4Az8KnIIW4=
+=b/mY
+-----END PGP SIGNATURE-----
 
=== added file 'tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_main_binary-amd64_Packages'
--- tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_main_binary-amd64_Packages	1970-01-01 00:00:00 +0000
+++ tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid-security_main_binary-amd64_Packages	2013-01-23 15:48:20 +0000
@@ -0,0 +1,10 @@
+Package: base-pkg
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 2
+Size: 1
+Description: a base package
+Origin: Ubuntu
 
=== added file 'tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_Release'
--- tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_Release	1970-01-01 00:00:00 +0000
+++ tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_Release	2013-01-23 15:48:20 +0000
@@ -0,0 +1,9 @@
+Origin: Ubuntu
+Label: Ubuntu
+Suite: lucid
+Version: 10.04
+Codename: lucid
+Date: Thu, 29 Apr 2010 17:24:55 UTC
+Architectures: amd64 armel i386 ia64 powerpc sparc
+Components: main restricted universe multiverse
+Description: Ubuntu Lucid 10.04
 
=== added file 'tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_Release.gpg'
--- tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_Release.gpg	1970-01-01 00:00:00 +0000
+++ tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_Release.gpg	2013-01-23 15:48:20 +0000
@@ -0,0 +1,7 @@
+-----BEGIN PGP SIGNATURE-----
+Version: GnuPG v1.4.6 (GNU/Linux)
+
+iD8DBQBL2cDzQJdur0N9BbURAmk2AJ9ungOjKn0ektAH87KhRIHht+1cDQCfck7P
+ZoIb2P0v2PEqa4Az8KnIIW4=
+=b/mY
+-----END PGP SIGNATURE-----
 
=== added 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	1970-01-01 00:00:00 +0000
+++ tests/aptroot-grouping-test/var/lib/apt/lists/archive.ubuntu.com_ubuntu_dists_lucid_main_binary-amd64_Packages	2013-01-23 15:48:20 +0000
@@ -0,0 +1,80 @@
+Package: installed-app
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 2
+Size: 1
+Depends: installed-pkg-multiple-deps
+Suggests: installed-pkg-single-dep
+Description: just an app
+Origin: Ubuntu
+
+Package: installed-app-with-subitems
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 2
+Size: 1
+Recommends: intermediate, installed-pkg-multiple-deps
+Description: app with subitems
+Origin: Ubuntu
+
+Package: intermediate
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 0
+Size: 1
+Description: intermediate pkg
+Origin: Ubuntu
+
+Package: installed-pkg
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 2
+Size: 1
+Description: just a pkg
+Origin: Ubuntu
+
+Package: installed-pkg-multiple-deps
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 2
+Size: 1
+Description: pkg with multiple deps
+Origin: Ubuntu
+
+Package: installed-pkg-single-dep
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 2
+Size: 1
+Description: pkg with single dep
+Origin: Ubuntu
+
+Package: ubuntu-minimal
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Foo <[e-mail]>
+Architecture: all
+Version: 2
+Size: 1
+Recommends: base-pkg
+Description: meta package
+Origin: Ubuntu
 
=== added file 'tests/aptroot-grouping-test/var/lib/apt/lists/lock'
=== added directory 'tests/aptroot-grouping-test/var/lib/apt/lists/partial'
=== added directory 'tests/aptroot-grouping-test/var/lib/dpkg'
=== added file 'tests/aptroot-grouping-test/var/lib/dpkg/status'
--- tests/aptroot-grouping-test/var/lib/dpkg/status	1970-01-01 00:00:00 +0000
+++ tests/aptroot-grouping-test/var/lib/dpkg/status	2013-01-23 15:48:20 +0000
@@ -0,0 +1,82 @@
+Package: base-pkg
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: a base package
+
+Package: installed-app
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: an installed app
+
+Package: installed-app-with-subitems
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: an installed app with subitems
+
+Package: intermediate
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Depends: installed-pkg-single-dep
+Description: an intermediate pkg
+
+Package: installed-pkg
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: an installed package
+
+Package: not-updatable
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Depends: installed-pkg-single-dep
+Description: extra pkg
+ to confirm we only look at updatable pkgs when calculating groupings
+
+Package: installed-pkg-multiple-deps
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: an installed package that multiple apps depend on
+
+Package: installed-pkg-single-dep
+Status: install ok installed
+Priority: optional
+Section: admin
+Installed-Size: 1
+Maintainer: Ubuntu Developers <[e-mail]>
+Architecture: all
+Version: 0.1
+Description: an installed package that only one app depends on
 
=== added directory 'tests/aptroot-grouping-test/var/lib/dpkg/updates'
=== modified file 'tests/test_update_list.py'
--- tests/test_update_list.py	2013-01-16 12:45:38 +0000
+++ tests/test_update_list.py	2013-01-23 15:48:20 +0000
@@ -6,15 +6,16 @@
 import apt
 import unittest
 
-from UpdateManager.Core.UpdateList import UpdateList
+from UpdateManager.Core import UpdateList
 from UpdateManager.Core.MyCache import MyCache
 
-from mock import patch
+from gi.repository import Gio
+from mock import patch, PropertyMock, MagicMock
 
 CURDIR = os.path.dirname(os.path.abspath(__file__))
 
 
-class UpdateListTestCase(unittest.TestCase):
+class PhasedTestCase(unittest.TestCase):
 
     def setUp(self):
         # mangle the arch
@@ -30,11 +31,10 @@
         self.cache._listsLock = 0
         self.cache.update()
         self.cache.open()
-        self.updates_list = UpdateList(parent=None)
+        self.updates_list = UpdateList.UpdateList(parent=None)
 
     def assertUpdatesListLen(self, nr):
-        origin = list(self.updates_list.pkgs.keys())[0]
-        self.assertEqual(len(self.updates_list.pkgs[origin]), nr)
+        self.assertEqual(self.updates_list.num_updates, nr)
 
     def test_phased_percentage_not_included(self):
         """ Test that updates above the threshold are not included"""
@@ -76,16 +76,96 @@
             self.updates_list.update(self.cache)
             self.assertUpdatesListLen(1)
 
-    @patch('UpdateManager.Core.UpdateList.OriginsImportance')
-    def test_phased_percentage_from_security(self, mock_origin_importance):
+    @patch('UpdateManager.Core.UpdateList.UpdateList._is_security_update')
+    def test_phased_percentage_from_security(self, mock_security):
         """ Test that updates from the security node go in"""
         # pretend all updates come from security for the sake of this test
-        mock_origin_importance.SECURITY = 0
+        mock_security.return_value = True
         with patch.object(self.updates_list.random, "randint") as mock_randint:
             mock_randint.return_value = 100
             self.updates_list.update(self.cache)
             self.assertUpdatesListLen(2)
 
+class GroupingTestCase(unittest.TestCase):
+    # installed_files does not respect aptroot, so we have to patch it
+    @patch('apt.package.Package.installed_files', new_callable=PropertyMock)
+    @patch('gi.repository.Gio.DesktopAppInfo.new_from_filename')
+    def setUp(self, mock_desktop, mock_installed):
+        # mangle the arch
+        real_arch = apt.apt_pkg.config.find("APT::Architecture")
+        apt.apt_pkg.config.set("APT::Architecture", "amd64")
+        self.addCleanup(
+            lambda: apt.apt_pkg.config.set("APT::Architecture", real_arch))
+        self.aptroot = os.path.join(CURDIR,
+                                    "aptroot-grouping-test")
+        self.cache = MyCache(apt.progress.base.OpProgress(),
+                             rootdir=self.aptroot)
+        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)
+
+    def fake_installed_files(self, mock_prop, pkg, pkg_class):
+        if pkg.name == 'installed-app':
+            return ['/usr/share/applications/installed-app.desktop']
+        elif pkg.name == 'installed-app-with-subitems':
+            return ['/usr/share/applications/installed-app2.desktop']
+        else:
+            return []
+
+    def fake_desktop(self, path):
+        # These can all be the same for our purposes
+        app = MagicMock()
+        app.get_filename.return_value = path
+        app.get_display_name.return_value = 'App ' + os.path.basename(path)
+        app.get_icon.return_value = Gio.ThemedIcon.new("package")
+        return app
+
+    def test_app(self):
+        self.assertGreater(len(self.updates_list.update_groups), 0)
+        group = self.updates_list.update_groups[0]
+        self.assertIsInstance(group, UpdateList.UpdateApplicationGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertEqual(group.core_item.pkg.name, 'installed-app')
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['installed-app'])
+
+    def test_app_with_subitems(self):
+        self.assertGreater(len(self.updates_list.update_groups), 1)
+        group = self.updates_list.update_groups[1]
+        self.assertIsInstance(group, UpdateList.UpdateApplicationGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertEqual(group.core_item.pkg.name,
+                         'installed-app-with-subitems')
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['installed-app-with-subitems',
+                              'installed-pkg-single-dep'])
+
+    def test_pkg(self):
+        self.assertGreater(len(self.updates_list.update_groups), 2)
+        group = self.updates_list.update_groups[2]
+        self.assertIsInstance(group, UpdateList.UpdatePackageGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertEqual(group.core_item.pkg.name, 'installed-pkg')
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['installed-pkg'])
+
+    def test_pkg_multiple_deps(self):
+        self.assertEqual(len(self.updates_list.update_groups), 4)
+        group = self.updates_list.update_groups[3]
+        self.assertIsInstance(group, UpdateList.UpdatePackageGroup)
+        self.assertIsNotNone(group.core_item)
+        self.assertEqual(group.core_item.pkg.name,
+                         'installed-pkg-multiple-deps')
+        self.assertListEqual([x.pkg.name for x in group.items],
+                             ['installed-pkg-multiple-deps'])
+
+    def test_security(self):
+        self.assertEqual(len(self.updates_list.security_groups), 1)
+        group = self.updates_list.security_groups[0]
+        self.assertIsInstance(group, UpdateList.UpdateSystemGroup)
+        self.assertIsNone(group.core_item)
+        self.assertListEqual([x.pkg.name for x in group.items], ['base-pkg'])
 
 if __name__ == "__main__":
     unittest.main()
 
=== modified file 'tests/test_update_origin.py'
--- tests/test_update_origin.py	2012-11-19 16:20:30 +0000
+++ tests/test_update_origin.py	2013-01-23 15:48:20 +0000
@@ -48,15 +48,11 @@
                         if l.archive == "lucid-security"]:
                     test_pkgs.add(pkg.name)
         self.assertTrue(len(test_pkgs) > 0)
-        ul = UpdateList(None)
-        matcher = ul.initMatcher("lucid")
+        ul = UpdateList(None, dist="lucid")
         for pkgname in test_pkgs:
             pkg = self.cache[pkgname]
-            origin = self.cache.match_package_origin(pkg, matcher)
-            self.assertEqual(self.cache.match_package_origin(pkg, matcher),
-                             matcher[("lucid-security", "Ubuntu")],
-                             "pkg '%s' is not in lucid-security but in '%s' "
-                             "instead" % (pkg.name, origin.description))
+            self.assertTrue(ul._is_security_update(pkg),
+                            "pkg '%s' is not in lucid-security" % pkg.name)
 
     def testOriginMatcherWithVersionInUpdatesAndSecurity(self):
         # empty dpkg status
@@ -90,15 +86,13 @@
                         "newer")
 
         # now test if versions in -security are detected
-        ul = UpdateList(None)
-        matcher = ul.initMatcher("lucid")
+        ul = UpdateList(None, dist="lucid")
         for pkgname in test_pkgs:
             pkg = self.cache[pkgname]
-            self.assertEqual(self.cache.match_package_origin(pkg, matcher),
-                             matcher[("lucid-security", "Ubuntu")],
-                             "package '%s' from lucid-updates contains also a "
-                             "(not yet installed) security updates, but it is "
-                             "not labeled as such" % pkg.name)
+            self.assertTrue(ul._is_security_update(pkg),
+                            "package '%s' from lucid-updates contains also a "
+                            "(not yet installed) security updates, but it is "
+                            "not labeled as such" % pkg.name)
 
         # now check if it marks the version with -update if the -security
         # version is installed
@@ -118,17 +112,14 @@
         self.cache.open()
         for pkgname in test_pkgs:
             pkg = self.cache[pkgname]
-            self.assertNotEqual(None, pkg._pkg.current_ver,
-                                "no package '%s' installed" % pkg.name)
+            self.assertIsNotNone(pkg._pkg.current_ver,
+                                 "no package '%s' installed" % pkg.name)
             candidate_version = getattr(pkg.candidate, "version", None)
-            origin = self.cache.match_package_origin(pkg, matcher)
-            self.assertEqual(self.cache.match_package_origin(pkg, matcher),
-                             matcher[("lucid-updates", "Ubuntu")],
+            self.assertFalse(ul._is_security_update(pkg),
                              "package '%s' (%s) from lucid-updates is "
-                             "labelled '%s' even though we have marked this "
-                             "version as installed already" %
-                             (pkg.name, candidate_version,
-                              origin.description))
+                             "labelled as a security update even though we "
+                             "have marked this version as installed already" %
+                             (pkg.name, candidate_version))
 
 
 if __name__ == "__main__":
 
=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2012-08-17 21:11:57 +0000
+++ tests/test_utils.py	2013-01-23 15:48:20 +0000
@@ -3,14 +3,11 @@
 
 import logging
 import glob
+import mock
 import sys
 import unittest
 
-from UpdateManager.Core.utils import (
-    is_child_of_process_name,
-    get_string_with_no_auth_from_source_entry,
-    humanize_size,
-    estimate_kernel_size_in_boot)
+from UpdateManager.Core import utils
 
 
 class TestUtils(unittest.TestCase):
@@ -18,27 +15,27 @@
     def test_humanize_size(self):
         # humanize size is a bit funny, it rounds up to kB as the meaningful
         # unit for users
-        self.assertEqual(humanize_size(1000), "1 kB")
-        self.assertEqual(humanize_size(10), "1 kB")
-        self.assertEqual(humanize_size(1200), "2 kB")
+        self.assertEqual(utils.humanize_size(1000), "1 kB")
+        self.assertEqual(utils.humanize_size(10), "1 kB")
+        self.assertEqual(utils.humanize_size(1200), "2 kB")
         # but not for MB as well
-        self.assertEqual(humanize_size(1200 * 1000), "1.2 MB")
-        self.assertEqual(humanize_size(1478 * 1000), "1.5 MB")
+        self.assertEqual(utils.humanize_size(1200 * 1000), "1.2 MB")
+        self.assertEqual(utils.humanize_size(1478 * 1000), "1.5 MB")
         # and we don't go to Gb  just yet (as its not really needed
         # in a upgrade context most of the time
-        self.assertEqual(humanize_size(1000 * 1000 * 1000), "1000.0 MB")
+        self.assertEqual(utils.humanize_size(1000 * 1000 * 1000), "1000.0 MB")
 
     @unittest.skipIf(not glob.glob("/boot/*"), "inside chroot")
     def test_estimate_kernel_size(self):
-        estimate = estimate_kernel_size_in_boot()
+        estimate = utils.estimate_kernel_size_in_boot()
         self.assertTrue(estimate > 0)
 
     def test_is_child_of_process_name(self):
-        self.assertTrue(is_child_of_process_name("init"))
-        self.assertFalse(is_child_of_process_name("mvo"))
+        self.assertTrue(utils.is_child_of_process_name("init"))
+        self.assertFalse(utils.is_child_of_process_name("mvo"))
         for e in glob.glob("/proc/[0-9]*"):
             pid = int(e[6:])
-            is_child_of_process_name("gdm", pid)
+            utils.is_child_of_process_name("gdm", pid)
 
     def test_is_port_listening(self):
         from UpdateManager.Core.utils import is_port_already_listening
@@ -49,16 +46,70 @@
         # entry with PW
         s = SourceEntry("deb http://user:pass@some-ppa/ ubuntu main")
         self.assertTrue(
-            not "user" in get_string_with_no_auth_from_source_entry(s))
+            not "user" in utils.get_string_with_no_auth_from_source_entry(s))
         self.assertTrue(
-            not "pass" in get_string_with_no_auth_from_source_entry(s))
-        self.assertEqual(get_string_with_no_auth_from_source_entry(s),
+            not "pass" in utils.get_string_with_no_auth_from_source_entry(s))
+        self.assertEqual(utils.get_string_with_no_auth_from_source_entry(s),
                          "deb http://hidden-u:hidden-p@some-ppa/ ubuntu main")
         # no pw
         s = SourceEntry("deb http://some-ppa/ ubuntu main")
-        self.assertEqual(get_string_with_no_auth_from_source_entry(s),
+        self.assertEqual(utils.get_string_with_no_auth_from_source_entry(s),
                          "deb http://some-ppa/ ubuntu main")
 
+    @mock.patch('UpdateManager.Core.utils._load_meta_pkg_list')
+    def test_flavor_package_ubuntu_first(self, mock_load):
+        cache = {'ubuntu-desktop': mock.MagicMock(),
+                 'other-desktop': mock.MagicMock()}
+        cache['ubuntu-desktop'].is_installed = True
+        cache['other-desktop'].is_installed = True
+        mock_load.return_value = ['other-desktop']
+        self.assertEqual(utils.get_ubuntu_flavor_package(cache=cache),
+                         'ubuntu-desktop')
+
+    @mock.patch('UpdateManager.Core.utils._load_meta_pkg_list')
+    def test_flavor_package_match(self, mock_load):
+        cache = {'a': mock.MagicMock(),
+                 'b': mock.MagicMock(),
+                 'c': mock.MagicMock()}
+        cache['a'].is_installed = True
+        cache['b'].is_installed = True
+        cache['c'].is_installed = True
+        mock_load.return_value = ['c', 'a', 'b']
+        # Must pick alphabetically first
+        self.assertEqual(utils.get_ubuntu_flavor_package(cache=cache), 'a')
+
+    def test_flavor_package_default(self):
+        self.assertEqual(utils.get_ubuntu_flavor_package(cache={}),
+                         'ubuntu-desktop')
+
+    def test_flavor_default(self):
+        self.assertEqual(utils.get_ubuntu_flavor(cache={}), 'ubuntu')
+
+    @mock.patch('UpdateManager.Core.utils.get_ubuntu_flavor_package')
+    def test_flavor_simple(self, mock_package):
+        mock_package.return_value = 'd'
+        self.assertEqual(utils.get_ubuntu_flavor(), 'd')
+
+    @mock.patch('UpdateManager.Core.utils.get_ubuntu_flavor_package')
+    def test_flavor_chop(self, mock_package):
+        mock_package.return_value = 'd-pkg'
+        self.assertEqual(utils.get_ubuntu_flavor(), 'd')
+
+    @mock.patch('UpdateManager.Core.utils.get_ubuntu_flavor_package')
+    def test_flavor_name_desktop(self, mock_package):
+        mock_package.return_value = 'something-desktop'
+        self.assertEqual(utils.get_ubuntu_flavor_name(), 'Something')
+
+    @mock.patch('UpdateManager.Core.utils.get_ubuntu_flavor_package')
+    def test_flavor_name_netbook(self, mock_package):
+        mock_package.return_value = 'something-netbook'
+        self.assertEqual(utils.get_ubuntu_flavor_name(), 'Something')
+
+    @mock.patch('UpdateManager.Core.utils.get_ubuntu_flavor_package')
+    def test_flavor_name_studio(self, mock_package):
+        mock_package.return_value = 'ubuntustudio-desktop'
+        self.assertEqual(utils.get_ubuntu_flavor_name(), 'Ubuntu Studio')
+
 
 if __name__ == '__main__':
     if len(sys.argv) > 1 and sys.argv[1] == "-v":