Merge lp:~mvo/apport/apport-checkreports-addition into lp:~apport-hackers/apport/trunk

[Michael Vogt]

This branch adds a new exit state for apport-checkreports (that is the helper that update-notifier is using). If apport is globally disabled but there are reports it now exits with status 2 and prints a message. This will make update-notifier not call the apport GUI is apport is disabled but there are reports comming in. It also moves the enabled() test into apport.fileutils

In addition to that I would like to add a new script apport-is-enabled that just returns 0/1. That I will then call in apt/aptdaemon to check that reports are actually enabled. Alternatively we could add a additional apport-checkreports --is-apport-enabled call if the additional binary is not wanted.

[lp:~mvo/apport/apport-checkreports-addition]

1897. By Michael Vogt on 2011-06-08

apport_python_hook.py: remove the no longer needed CONFIG

[Martin Pitt (pitti)]

There is already apport.packaging.enabled(), so no need to redefine it again in fileutils.

The new exit status is fine for me, I'll merge that.

A new script apport-is-enabled also sounds ok to me, but I'd write it in dash, not python; calling a full python interpreter from dpkg/apt not only sounds very expensive to just check a single file, but also is prone to failures if the very thing we are reporting are failed packages.

review: Approve

[Martin Pitt (pitti)]

Also, apport_python_hook directly checks the file as importing any other apport module is too brittle. Please see bug 528355 for details.

[Martin Pitt (pitti)]

Final merge: http://bazaar.launchpad.net/~apport-hackers/apport/trunk/revision/1896

Thanks!

=== modified file 'apport/fileutils.py'
--- apport/fileutils.py	2011-02-03 13:52:02 +0000
+++ apport/fileutils.py	2011-06-08 08:48:00 +0000
@@ -25,6 +25,17 @@
 
 _config_file = '~/.config/apport/settings'
 
+def apport_enabled():
+    '''Return whether Apport should generate crash reports.'''
+    CONFIG = '/etc/default/apport'
+    import re
+    try:
+        conf = open(CONFIG).read()
+        return re.search('^\s*enabled\s*=\s*0\s*$', conf, re.M) is None
+    except IOError:
+        # if the file does not exist, assume it's enabled
+        return True
+
 def find_package_desktopfile(package):
     '''Return a package's .desktop file.
 

=== modified file 'apport_python_hook.py'
--- apport_python_hook.py	2011-02-28 10:58:32 +0000
+++ apport_python_hook.py	2011-06-08 08:48:00 +0000
@@ -13,18 +13,7 @@
 import os
 import sys
 
-CONFIG = '/etc/default/apport'
-
-def enabled():
-    '''Return whether Apport should generate crash reports.'''
-
-    import re
-    try:
-        conf = open(CONFIG).read()
-        return re.search('^\s*enabled\s*=\s*0\s*$', conf, re.M) is None
-    except IOError:
-        # if the file does not exist, assume it's enabled
-        return True
+from apport.fileutils import apport_enabled
 
 def apport_excepthook(exc_type, exc_obj, exc_tb):
     '''Catch an uncaught exception and make a traceback.'''
@@ -45,7 +34,7 @@
             return
 
         # do not do anything if apport was disabled
-        if not enabled():
+        if not apport_enabled():
             return
 
         try:

=== modified file 'data/apport-checkreports'
--- data/apport-checkreports	2009-09-08 10:30:40 +0000
+++ data/apport-checkreports	2011-06-08 08:48:00 +0000
@@ -14,7 +14,7 @@
 
 import sys, os.path, optparse
 
-from apport.fileutils import get_new_reports, get_new_system_reports
+from apport.fileutils import get_new_reports, get_new_system_reports, apport_enabled
 
 # parse command line options
 optparser = optparse.OptionParser('%prog [options]')
@@ -29,8 +29,12 @@
     reports = get_new_reports()
 
 if len(reports) > 0:
-    for r in reports:
-        print r.split('.')[0].split('_')[-1]
-    sys.exit(0)
+    if apport_enabled():
+        for r in reports:
+            print r.split('.')[0].split('_')[-1]
+        sys.exit(0)
+    else:
+        print "new reports but apport disabled"
+        sys.exit(2)
 else:
     sys.exit(1)