[Merge] lp:~juliank/update-notifier/esm into lp:update-notifier
Brian Murray
brian at ubuntu.com
Fri Mar 29 16:22:46 UTC 2019
Nothing critical but a few comments appear in-line.
Diff comments:
> === modified file 'data/apt_check.py'
> --- data/apt_check.py 2018-11-13 20:53:28 +0000
> +++ data/apt_check.py 2019-03-29 16:07:11 +0000
> @@ -61,18 +70,85 @@
> outstream.write("\n".join([p.name for p in pkgs]))
>
>
> -def write_human_readable_summary(outstream, upgrades, security_updates):
> +def write_human_readable_summary(outstream, upgrades, security_updates,
> + esm_updates, have_esm, disabled_esm_updates):
> " write out human summary summary to outstream "
> - outstream.write(gettext.dngettext("update-notifier",
> - "%i package can be updated.",
> - "%i packages can be updated.",
> - upgrades) % upgrades)
> - outstream.write("\n")
> - outstream.write(gettext.dngettext("update-notifier",
> - "%i update is a security update.",
> - "%i updates are security updates.",
> - security_updates) % security_updates)
> - outstream.write("\n")
> + if have_esm is not None:
> + if have_esm:
> + outstream.write(gettext.dgettext("update-notifier",
> + "Extended Security Maintenance "
> + "(ESM) is enabled."))
> + else:
> + outstream.write(gettext.dgettext("update-notifier",
> + "Extended Security Maintenance "
> + "(ESM) is not enabled."))
> + outstream.write("\n\n")
> +
> + if upgrades != 0 or disabled_esm_updates > 0:
> + outstream.write(gettext.dngettext("update-notifier",
> + "%i update can be installed "
> + "immediately.",
> + "%i updates can be installed "
> + "immediately.",
> + upgrades) % upgrades)
> + outstream.write("\n")
> + if esm_updates > 0:
> + outstream.write(gettext.dngettext("update-notifier",
> + "%i of these updates is "
> + "provided through ESM.",
> + "%i of these updates are "
> + "provided through ESM.",
> + esm_updates) %
> + esm_updates)
> + outstream.write("\n")
> + if upgrades > 0:
> + # only show this line if we have (any) upgrades. The point here
> + # is that we want to hide it if all updates are applied, but there
> + # are some ESM updates available.
> + outstream.write(gettext.dngettext("update-notifier",
> + "%i of these updates is a "
> + "security update.",
> + "%i of these updates are "
> + "security updates.",
> + security_updates) %
> + security_updates)
> + outstream.write("\n")
> + else:
> + outstream.write(gettext.dgettext("update-notifier",
> + "All updates are applied."))
> + outstream.write("\n")
> +
> + if disabled_esm_updates > 0:
> + outstream.write("\n")
> + outstream.write(gettext.dngettext("update-notifier",
> + "Enable ESM to get %i additional "
> + "security update.",
> + "Enable ESM to get %i additional "
> + "security updates.",
> + disabled_esm_updates) %
> + disabled_esm_updates)
> + outstream.write("\n")
> + outstream.write(gettext.dgettext("update-notifier",
> + "See 'ua enable esm' or "
> + "https://ubuntu.com/esm"))
> + outstream.write("\n")
> +
> +
> +def has_disabled_esm_security_update(depcache, pkg):
> + " check if we have a disabled ESM security update "
> + inst_ver = pkg.current_ver
> + if not inst_ver:
> + return False
> +
> + for version in pkg.version_list:
> + if version == inst_ver:
> + break
> +
> + for file, index in version.file_list:
In isESMupgrade you use the following which is different:
for (file, index) in ver.file_list:
> + if (file.origin == "UbuntuESM" and file.archive.startswith(DISTRO)
> + and depcache.policy.get_priority(file) == -32768):
> + return True
> + return False
>
>
> def init():
> @@ -115,13 +191,28 @@
> sys.stderr.write("E: " + _("Error: Marking the upgrade (%s)") % e)
> sys.exit(-1)
>
> + # Check if we have ESM enabled or disabled; and if it exists in the
> + # first place.
> + have_esm = None # None == does not exist
> + for file in cache.file_list:
> + if file.origin == "UbuntuESM" and file.archive.startswith(DISTRO):
> + if depcache.policy.get_priority(file) == -32768:
> + have_esm = False
> + else:
> + have_esm = True
Do you need to keep iterating through the loop once have_esm is True?
> +
> # analyze the ugprade
> upgrades = 0
> security_updates = 0
> + esm_updates = 0
> + disabled_esm_updates = 0
>
> # we need another cache that has more pkg details
> with apt.Cache() as aptcache:
> for pkg in cache.packages:
> + if has_disabled_esm_security_update(depcache, pkg):
> + disabled_esm_updates += 1
> +
> # skip packages that are not marked upgraded/installed
> if not (depcache.marked_install(pkg)
> or depcache.marked_upgrade(pkg)):
>
> === modified file 'debian/changelog'
> --- debian/changelog 2019-03-19 10:27:07 +0000
> +++ debian/changelog 2019-03-29 16:07:11 +0000
> @@ -1,3 +1,10 @@
> +update-notifier (3.192.16) UNRELEASED; urgency=medium
> +
> + * Rewritten and extended motd messaging (LP: #1822340)
Rewrote and extended reads better.
> + * Count ESM security updates as security updates
> +
> + -- Julian Andres Klode <juliank at ubuntu.com> Fri, 29 Mar 2019 16:26:45 +0100
> +
> update-notifier (3.192.15) disco; urgency=medium
>
> * debian/control:
>
> === added symlink 'tests/apt_check.py'
> === target is u'../data/apt_check.py'
> === added file 'tests/test_motd.py'
> --- tests/test_motd.py 1970-01-01 00:00:00 +0000
> +++ tests/test_motd.py 2019-03-29 16:07:11 +0000
> @@ -0,0 +1,134 @@
> +#!/usr/bin/python3
> +# -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*-
> +
> +import apt_check
> +import io
> +import os
> +import subprocess
> +import unittest
> +import textwrap
> +
> +
> +def get_message(*args, **kwds):
> + with io.StringIO() as stream:
> + apt_check.write_human_readable_summary(stream, *args, **kwds)
> + return stream.getvalue()
> +
> +
> +class TestMotd(unittest.TestCase):
> + """ ensure that the tree is pep8 clean """
> +
> + def test_esm_disabled_upto_date_esm_avail(self):
> + self.assertEqual(
> + get_message(upgrades=0, security_updates=0,
> + esm_updates=0, have_esm=False,
> + disabled_esm_updates=23),
> + textwrap.dedent(
> + """
> + Extended Security Maintenance (ESM) is not enabled.
> +
> + 0 updates can be installed immediately.
> +
> + Enable ESM to get 23 additional security updates.
> + See 'ua enable esm' or https://ubuntu.com/esm
> + """).lstrip())
> +
> + def test_esm_disabled_security_esm_avail(self):
> + self.assertEqual(
> + get_message(upgrades=15, security_updates=7,
> + esm_updates=0, have_esm=False,
> + disabled_esm_updates=23),
> + textwrap.dedent(
> + """
> + Extended Security Maintenance (ESM) is not enabled.
> +
> + 15 updates can be installed immediately.
> + 7 of these updates are security updates.
> +
> + Enable ESM to get 23 additional security updates.
> + See 'ua enable esm' or https://ubuntu.com/esm
> + """).lstrip())
> +
> + def test_esm_disabled_security_no_esm_avail(self):
> + self.assertEqual(
> + get_message(upgrades=15, security_updates=7,
> + esm_updates=0, have_esm=False,
> + disabled_esm_updates=0),
> + textwrap.dedent(
> + """
> + Extended Security Maintenance (ESM) is not enabled.
> +
> + 15 updates can be installed immediately.
> + 7 of these updates are security updates.
> + """).lstrip())
> +
> + def test_esm_disabled_nosecurity(self):
> + self.assertEqual(
> + get_message(upgrades=15, security_updates=0,
> + esm_updates=0, have_esm=False,
> + disabled_esm_updates=0),
> + textwrap.dedent(
> + """
> + Extended Security Maintenance (ESM) is not enabled.
> +
> + 15 updates can be installed immediately.
> + 0 of these updates are security updates.
> + """).lstrip())
> +
> + def test_esm_disabled_noupdates(self):
> + self.assertEqual(
> + get_message(upgrades=0, security_updates=0,
> + esm_updates=0, have_esm=False,
> + disabled_esm_updates=0),
> + textwrap.dedent(
> + """
> + Extended Security Maintenance (ESM) is not enabled.
> +
> + All updates are applied.
> + """).lstrip())
> +
> + def test_esm_enabled_nosecurity(self):
> + self.assertEqual(
> + get_message(upgrades=35, security_updates=0,
> + esm_updates=13, have_esm=True,
> + disabled_esm_updates=0),
> + textwrap.dedent(
> + """
> + Extended Security Maintenance (ESM) is enabled.
> +
> + 35 updates can be installed immediately.
> + 13 of these updates are provided through ESM.
> + 0 of these updates are security updates.
> + """).lstrip())
> +
> + def test_esm_enabled_somesecurity(self):
> + self.assertEqual(
> + get_message(upgrades=47, security_updates=7,
> + esm_updates=13, have_esm=True,
> + disabled_esm_updates=0),
> + textwrap.dedent(
> + """
> + Extended Security Maintenance (ESM) is enabled.
> +
> + 47 updates can be installed immediately.
> + 13 of these updates are provided through ESM.
> + 7 of these updates are security updates.
> + """).lstrip())
> +
> + def test_esm_enabled_noupdates(self):
> + self.assertEqual(
> + get_message(upgrades=0, security_updates=0,
> + esm_updates=0, have_esm=True,
> + disabled_esm_updates=0),
> + textwrap.dedent(
> + """
> + Extended Security Maintenance (ESM) is enabled.
> +
> + All updates are applied.
> + """).lstrip())
Seeing this in a test might make it more obvious than when someone is using this code, but I find "updates can be installed" vs "updates are applied" inconsistent.
> +
> +
> +if __name__ == "__main__":
> + import logging
> + logging.basicConfig(level=logging.DEBUG)
> + unittest.main()
--
https://code.launchpad.net/~juliank/update-notifier/esm/+merge/365288
Your team Ubuntu Core Development Team is requested to review the proposed merge of lp:~juliank/update-notifier/esm into lp:update-notifier.
More information about the Ubuntu-reviews
mailing list