Merge lp:~brian-murray/apport/whoopsie-upload-more into lp:~apport-hackers/apport/trunk

[Brian Murray]

For reasons unknown to me whoopsie-upload-all as it is currently written will quit if the add_gdb_info() call fails. This is different than the behavior in apport/ui.py which will pass on a failure because "we'll get stack traces on retracing". So this change modifies how add_gdb_info exits so that whoopsie-upload-all will proceed with gathering crash information and creating the .upload file. This will in turn end up fixing bug 1820132.

I wonder if there is a better way to do this though so all feedback is welcome!

[Julian Andres Klode (juliank)]

Looks OK, although I'd go with something less string magic.

review: Approve

[lp:~brian-murray/apport/whoopsie-upload-more]

3231. By Brian Murray on 2019-03-29

Switch to FileNotFoundError per reviewer suggestion, add in a test for a missing executable.

[Brian Murray (brian-murray)]

While switching to FileNotFound makes sense it is causing issues with test/run's calling of pyflakes e.g. ./build/lib/apport/report.py:723: undefined name 'FileNotFoundError'. I'm not easily finding a way to work around that. Do you have any ideas?

[Julian Andres Klode (juliank)]

Oh, we should be running pyflakes3 not pyflakes, that would fix it.

[Brian Murray (brian-murray)]

I made the switch from pyflakes to pyflakes3 directly in trunk.

=== modified file 'apport/report.py'
--- apport/report.py	2019-03-15 21:53:40 +0000
+++ apport/report.py	2019-03-29 20:51:03 +0000
@@ -705,7 +705,8 @@
         files there.
 
         Raises a IOError if the core dump is invalid/truncated, or OSError if
-        calling gdb fails.
+        calling gdb fails, or FileNotFoundError if gdb or the crashing
+        executable cannot be found.
         '''
         if 'CoreDump' not in self or 'ExecutablePath' not in self:
             return
@@ -718,6 +719,9 @@
                        'GLibAssertionMessage': 'print __glib_assert_msg',
                        'NihAssertionMessage': 'print (char*) __nih_abort_msg'}
         gdb_cmd, environ = self.gdb_command(rootdir, gdb_sandbox)
+        if not gdb_cmd:
+            raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT),
+                                    'gdb not found in retracing env')
 
         # limit maximum backtrace depth (to avoid looped stacks)
         gdb_cmd += ['--batch', '--ex', 'set backtrace limit 2000']
@@ -742,6 +746,9 @@
             reason = 'Invalid core dump: ' + warnings.strip()
             self['UnreportableReason'] = reason
             raise IOError(reason)
+        elif out.split('\n')[0].endswith('No such file or directory.'):
+            raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT),
+                                    'executable file for coredump not found')
 
         # split the output into the various fields
         part_re = re.compile(r'^\$\d+\s*=\s*-99$', re.MULTILINE)
@@ -1544,8 +1551,7 @@
                            if gdb_sandbox else None)
         gdb_path = _which_extrapath('gdb', gdb_sandbox_bin)
         if not gdb_path:
-            apport.fatal('gdb does not exist in the %ssandbox nor on the host'
-                         % ('gdb ' if not same_arch else ''))
+            return '', ''
         command = [gdb_path]
         environ = None
 
@@ -1587,7 +1593,6 @@
                             gdb_sandbox]
             executable = sandbox + '/' + executable
 
-        assert os.path.exists(executable)
         command += ['--ex', 'file "%s"' % executable]
 
         if 'CoreDump' in self:

=== modified file 'data/whoopsie-upload-all'
--- data/whoopsie-upload-all	2016-12-10 11:28:27 +0000
+++ data/whoopsie-upload-all	2019-03-29 20:51:03 +0000
@@ -19,6 +19,7 @@
 import subprocess
 import argparse
 import fcntl
+import errno
 
 import apport.fileutils
 import apport
@@ -78,10 +79,13 @@
         try:
             r.add_gdb_info()
         except (IOError, EOFError, OSError) as e:
-            sys.stderr.write('ERROR: processing %s: %s\n' % (report, str(e)))
-            if os.path.exists(report):
-                os.unlink(report)
-            return None
+            # the crash file won't have a Stacktrace etc but apport-retrace
+            # could still process it
+            if e.errno != errno.ENOENT:
+                sys.stderr.write('ERROR: processing %s: %s\n' % (report, str(e)))
+                if os.path.exists(report):
+                    os.unlink(report)
+                return None
 
         # write updated report
         with open(report, 'ab') as f:

=== modified file 'test/test_report.py'
--- test/test_report.py	2017-07-21 19:39:25 +0000
+++ test/test_report.py	2019-03-29 20:51:03 +0000
@@ -644,6 +644,15 @@
         self.assertNotIn('StacktraceTop', pr)
         self.assertIn('core is truncated', pr['UnreportableReason'])
 
+    def test_add_gdb_info_exe_missing(self):
+        '''add_gdb_info() with missing executable'''
+
+        pr = self._generate_sigsegv_report()
+        # change it to something that doesn't exist
+        pr['ExecutablePath'] = pr['ExecutablePath'].replace('crash', 'gone')
+
+        self.assertRaises(FileNotFoundError, pr.add_gdb_info)
+
     def test_add_zz_parse_segv_details(self):
         '''parse-segv produces sensible results'''
         rep = tempfile.NamedTemporaryFile()