Merge lp:~tonyyarusso/update-notifier/bug-1167621 into lp:update-notifier/ubuntu

[Tony Yarusso]

Make apt_check.py more suitable for calling as a module from another script (LP: #1167621)

[Iain Lane (laney)]

Hey pitti, looks like this is up your street - fancy reviewing? :-)

(if not, let me know and I'll try to reason it out myself)

[Martin Pitt (pitti)]

8 -def write_package_names(outstream, cache, depcache):
9 +def write_package_names(cache, depcache, messages, standalone, delim="\n"):
10 " write out package names that change to outstream "

Can you please adjust the docstrings accordingly? Also, you should rename the methods, as they don't write anything any more. Perhaps get_package_names()? Also, it seems this function doesn't actually use the new "standalone" argument?

For write_human_readable_summary, please also document the two new arguments in the docstring. I think it's an unusual style to pass in "messages" and have the function return that plus some extra text; instead, concatenate the result of the two functions in the caller.

64 + "--delimeter",

It's "delimiter" (same typo again further down).

Thanks!

review: Needs Fixing

[Sebastien Bacher (seb128)]

(setting to "work in progress" so it drops from the sponsoring queue while being worked, please set it back to "needs review" again once you addressed the review comments)

Unmerged revisions

790. By Tony Yarusso <email address hidden> on 2013-04-11

Make apt_check.py more suitable for calling as a module from another script (LP: #1167621)

=== modified file 'data/apt_check.py'
--- data/apt_check.py	2012-10-16 13:45:38 +0000
+++ data/apt_check.py	2013-04-11 21:50:27 +0000
@@ -48,25 +48,57 @@
                 return True
     return False
 
-def write_package_names(outstream, cache, depcache):
+def write_package_names(cache, depcache, messages, standalone, delim="\n"):
     " write out package names that change to outstream "
     pkgs = filter(lambda pkg:
                   depcache.marked_install(pkg) or depcache.marked_upgrade(pkg),
                   cache.packages)
-    outstream.write("\n".join(map(lambda p: p.name, pkgs)))
+    messages += delim.join(map(lambda p: p.name, pkgs)) + "\n"
+    return messages
 
-def write_human_readable_summary(outstream, upgrades, security_updates):
+def write_human_readable_summary(upgrades, security_updates, messages, standalone):
     " 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")
+    messages += gettext.dngettext("update-notifier",
+                                  "%i package can be updated.",
+                                  "%i packages can be updated.",
+                                  upgrades) % upgrades
+    if standalone:
+        messages += "\n"
+    else:
+        messages += "  "
+    messages += gettext.dngettext("update-notifier",
+                                  "%i update is a security update.",
+                                  "%i updates are security updates.",
+                                  security_updates)  % security_updates
+    messages += "\n"
+    return messages
+
+def get_options(args):
+    parser = OptionParser()
+    parser.add_option("-p",
+                      "--package-names",
+                      action="store_true",
+                      dest="show_package_names",
+                      help=_("Show the packages that are going to be installed/upgraded"))
+    parser.add_option("",
+                      "--human-readable",
+                      action="store_true",
+                      dest="readable_output",
+                      help=_("Show human readable output on stdout"))
+    parser.add_option("",
+                      "--security-updates-unattended",
+                      action="store_true",
+                      help=_("Return the time in days when security updates "
+                             "are installed unattended (0 means disabled)"))
+    parser.add_option("-d",
+                      "--delimeter",
+                      action="store",
+                      type="string",
+                      dest="pkg_delim",
+                      help=_("Set delimeter to use between package names when used "
+                             "with the -p option.  Default is a newline."))
+    return parser.parse_args(args)
+
 def init():
     " init the system, be nice "
     # FIXME: do a ionice here too?
@@ -78,7 +110,9 @@
     apt_pkg.config.set("Dir::Cache::pkgcache","")
     apt_pkg.config.set("Dir::Cache::srcpkgcache","")
     
-def run(options=None):
+def run(options=None, standalone=False):
+
+    messages = ""
 
     # we are run in "are security updates installed automatically?"
     # question mode
@@ -146,18 +180,23 @@
                 break
 
     # print the number of upgrades
+
+    if options and options.readable_output:
+        messages = write_human_readable_summary(upgrades, security_updates, messages, standalone)
+
     if options and options.show_package_names:
-        write_package_names(sys.stderr, cache, depcache)
-    elif options and options.readable_output:
-        write_human_readable_summary(sys.stdout, upgrades, security_updates)
-    else:
+        if options.pkg_delim:
+            messages = write_package_names(cache, depcache, messages, standalone, options.pkg_delim)
+        else:
+            messages = write_package_names(cache, depcache, messages, standalone)
+
+    if (not options or (options and not (options.readable_output or options.show_package_names))) and standalone:
         # print the number of regular upgrades and the number of 
         # security upgrades
-        sys.stderr.write("%s;%s" % (upgrades,security_updates))
-
-    # return the number of upgrades (if its used as a module)
-    return(upgrades,security_updates)
-
+        messages = str(upgrades) + ";" + str(security_updates)
+
+    # return the number of upgrades
+    return (upgrades, security_updates, messages)
 
 if __name__ == "__main__":        
     # setup a exception handler to make sure that uncaught stuff goes
@@ -171,24 +210,9 @@
     gettext.textdomain(APP)
 
     # check arguments
-    parser = OptionParser()
-    parser.add_option("-p",
-                      "--package-names",
-                      action="store_true",
-                      dest="show_package_names",
-                      help=_("Show the packages that are going to be installed/upgraded"))
-    parser.add_option("",
-                      "--human-readable",
-                      action="store_true",
-                      dest="readable_output",
-                      help=_("Show human readable output on stdout"))
-    parser.add_option("",
-                      "--security-updates-unattended",
-                      action="store_true",
-                      help=_("Return the time in days when security updates "
-                             "are installed unattended (0 means disabled)"))
-    (options, args) = parser.parse_args()
+    (options, args) = get_options(sys.argv[1:])
 
     # run it
     init()
-    run(options)
+    result = run(options, True)[2]
+    sys.stdout.write(result)