Merge lp:~juliank/apport/named-arguments into lp:~apport-hackers/apport/trunk

[Julian Andres Klode]

New command line parsing for bug 1732962

[Julian Andres Klode (juliank)]

This could be somewhat nicer after some refactoring to make it support int arguments, but I wanted to keep the diff tiny rather than doing big refactoring.

Not tested yet, BTW.

[Julian Andres Klode (juliank)]

Please don't merge without considering the comments in 1732962

[lp:~juliank/apport/named-arguments]

3195. By Launchpad Translations on behalf of apport-hackers on 2018-03-31

Launchpad automatic translations update.

3196. By Brian Murray on 2018-04-30

apport/ui.py: Include ProblemType in reports which are updated as package hooks may expect the report to have a ProblemType. (LP: #1766794)

3197. By Launchpad Translations on behalf of apport-hackers on 2018-05-08

Launchpad automatic translations update.

3198. By Brian Murray on 2018-05-09

backends/packaging-apt-dpkg.py: fix a pep8 error

3199. By Brian Murray on 2018-05-09

test/test_ui.py: modify run_crash_kernel test to account for the fact that linux-image-- is now built from the linux-signed source package on amd64 and ppc64el.

3200. By Brian Murray on 2018-05-09

release 2.20.10

3201. By Launchpad Translations on behalf of apport-hackers on 2018-05-17

Launchpad automatic translations update.

3202. By Brian Murray on 2018-05-30

SECURITY UPDATE: Ensure that we propely handle crashes that originate from a PID namespace. Thanks to Sander Bos for discovering this issue.

3203. By Launchpad Translations on behalf of apport-hackers on 2018-06-07

Launchpad automatic translations update.

3204. By Brian Murray on 2018-07-19

merge didier's changes to utilize a remember option which stores the send or don't send information.

3205. By Launchpad Translations on behalf of apport-hackers on 2018-07-27

Launchpad automatic translations update.

3206. By Launchpad Translations on behalf of apport-hackers on 2018-08-23

Launchpad automatic translations update.

[Brian Murray (brian-murray)]

This looks good to me.

review: Approve

[lp:~juliank/apport/named-arguments]

3207. By Brian Murray on 2018-09-28

backends/packaging-apt-dpkg.py: install only the matching -dbg from the source package so we fall back to installing the -dbgsym package

3208. By Brian Murray on 2018-10-05

backends/packaging-apt-dpkg.py: switch to using python3-launchpadlib to communicate with Launchpad.

3209. By Launchpad Translations on behalf of apport-hackers on 2018-10-06

Launchpad automatic translations update.

3210. By Brian Murray on 2018-10-10

backends/packaging-apt-dpkg.py: The Contents file can contain files with spaces in the name, e.g. liblcms2-dev package in bionic, so use the part after the last space as the package.

3211. By Brian Murray on 2018-10-11

merge my branch to use a glocal mapping of files to packages

3212. By Brian Murray on 2018-10-15

assign variables default values

3213. By Launchpad Translations on behalf of apport-hackers on 2018-10-18

Launchpad automatic translations update.

3214. By Brian Murray on 2018-10-25

merge my change to failover to using -dbgsym packages if the versioned -dbg packages are not available.

3215. By Brian Murray on 2019-02-12

data/iwlwifi_error_dump: resolve W605 re invalid escape sequence

3216. By Brian Murray on 2019-02-12

test/run: Ignore PEP8 W503, W504 warnings

3217. By Brian Murray on 2019-02-12

apport/ui.py: guard against report.get_timestamp() returning None

3218. By Brian Murray on 2019-02-12

backends/packaging-apt-dpkg.py: switch back to installing all -dbg from the source package as not doing so causes a failure to find the -dbg package for libkrb5-3

3219. By Brian Murray on 2019-02-12

merge Launchpad automatic translations updates

3220. By Brian Murray on 2019-02-12

merge didier's branch handling older reports w/o a remember option

3221. By Launchpad Translations on behalf of apport-hackers on 2019-02-13

Launchpad automatic translations update.

3222. By Brian Murray on 2019-02-15

Merge slangasek's changes to accomodate new exit codes from gdb 8.2.50.

3223. By Launchpad Translations on behalf of apport-hackers on 2019-02-23

Launchpad automatic translations update.

3224. By Julian Andres Klode on 2019-03-04

data/apport: Refactor argument parsing into function

3225. By Julian Andres Klode on 2019-03-04

Introduce support for non-positional arguments

This makes it much easier to support new kernel features. For
compatibility with old containers, we need to have the global
PID as the 5th argument: The forwarding code parses that argument
and passes the rest of the arguments to the host via a socket.

It also checks that there are precisely 6 arguments, though, so we
cannot directly use the new arguments, but need to hack around
a bit to fit them into that space, by introducing --more.

(LP: #1732962)

=== modified file 'NEWS'
--- NEWS	2019-02-15 14:42:39 +0000
+++ NEWS	2019-03-04 16:41:51 +0000
@@ -17,6 +17,8 @@
     option. If the option isn't there, consider as false. (LP: #1791324)
   * apport/report.py: End our gdb batch script with a separator, to accomodate
     new exit codes from gdb 8.2.50. Thanks Steve Langasek!
+  * data/apport: Introduce support for non-positional arguments so we
+    can easily extend core_pattern in the future (LP: #1732962)
 
 2.20.10 (2018-05-09)
 --------------------

=== modified file 'data/apport'
--- data/apport	2018-05-30 20:49:07 +0000
+++ data/apport	2019-03-04 16:41:51 +0000
@@ -15,7 +15,7 @@
 
 import sys, os, os.path, subprocess, time, traceback, pwd, io
 import signal, inspect, grp, fcntl, socket, atexit, array, struct
-import errno
+import errno, argparse
 
 import apport, apport.fileutils
 
@@ -325,6 +325,42 @@
     return False
 
 
+def parse_arguments():
+    parser = argparse.ArgumentParser(epilog="""
+    Alternatively, the following command line is understood for legacy hosts:
+        <pid> <signal number> <core file ulimit> <dump mode> [global pid]
+    """)
+
+    # TODO: Use type=int
+    parser.add_argument("-p", "--pid", help="process id (%%p)")
+    parser.add_argument("-s", "--signal-number", help="signal number (%%s)")
+    parser.add_argument("-c", "--core-ulimit", help="core ulimit (%%c)")
+    parser.add_argument("-d", "--dump-mode", help="dump mode (%%d)")
+    parser.add_argument("-P", "--global-pid", nargs='?', help="pid in root namespace (%%P)")
+
+    options, rest = parser.parse_known_args()
+
+    if options.pid is not None:
+        for arg in rest:
+            error_log("Unknown argument: %s", arg)
+
+    elif len(rest) in (4,5):
+        # Translate legacy command line
+        options.pid = rest[0]
+        options.signal_number = rest[1]
+        options.core_ulimit = rest[2]
+        options.dump_mode = rest[3]
+        try:
+            options.global_pid = rest[4]
+        except IndexError:
+            options.global_pid = None
+    else:
+        parser.print_usage()
+        sys.exit(1)
+
+    return options
+
+
 #################################################################
 #
 # main
@@ -373,16 +409,8 @@
         error_log('Received a bad number of arguments from forwarder, received %d, expected 5, aborting.' % len(sys.argv))
         sys.exit(1)
 
-# Normal startup
-if len(sys.argv) not in (5, 6):
-    try:
-        print('Usage: %s <pid> <signal number> <core file ulimit> <dump mode> [global pid]' % sys.argv[0])
-        print('The core dump is read from stdin.')
-    except IOError:
-        # sys.stderr might not actually exist, expecially not when being called
-        # from the kernel
-        pass
-    sys.exit(1)
+
+options = parse_arguments()
 
 init_error_log()
 
@@ -390,8 +418,8 @@
 # then compare it with the local PID. If they don't match, it's an
 # indication that the crash originated from another PID namespace.
 # Simply log an entry in the host error log and exit 0.
-if len(sys.argv) == 6:
-    host_pid = int(sys.argv[5])
+if options.global_pid is not None:
+    host_pid = int(options.global_pid)
 
     if not is_same_ns(host_pid, "pid") and not is_same_ns(host_pid, "mnt"):
         # If the crash came from a container, don't attempt to handle
@@ -401,7 +429,7 @@
         # forward the process information there.
         if not os.path.exists('/proc/%d/root/run/apport.socket' % host_pid):
             error_log('host pid %s crashed in a container without apport support' %
-                      sys.argv[5])
+                      options.global_pid)
             sys.exit(0)
 
         sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
@@ -409,7 +437,7 @@
             sock.connect('/proc/%d/root/run/apport.socket' % host_pid)
         except Exception as e:
             error_log('host pid %s crashed in a container with a broken apport' %
-                      sys.argv[5])
+                      options.global_pid)
             sys.exit(0)
 
         # Send all arguments except for the first (exec path) and last (global pid)
@@ -440,14 +468,17 @@
         # sandbox will use container namespaces as a security measure but are
         # still otherwise host processes. When that's the case, we need to keep
         # handling those crashes locally using the global pid.
-        sys.argv[1] = str(host_pid)
+        options.pid = str(host_pid)
 
 check_lock()
 
 try:
     setup_signals()
 
-    (pid, signum, core_ulimit, dump_mode) = sys.argv[1:5]
+    pid = options.pid
+    signum = options.signal_number
+    core_ulimit = options.core_ulimit
+    dump_mode = options.dump_mode
 
     get_pid_info(pid)
 

=== modified file 'etc/init.d/apport'
--- etc/init.d/apport	2017-11-15 20:07:23 +0000
+++ etc/init.d/apport	2019-03-04 16:41:51 +0000
@@ -50,7 +50,12 @@
 		rm -f /var/lib/pm-utils/resume-hang.log
 	fi
 
-	echo "|$AGENT %p %s %c %d %P" > /proc/sys/kernel/core_pattern
+	# Old compatibility mode, switch later to second one
+	if true; then
+		echo "|$AGENT %p %s %c %d %P" > /proc/sys/kernel/core_pattern
+	else
+		echo "|$AGENT -p%p -s%s -c%c -d%d -P%P" > /proc/sys/kernel/core_pattern
+	fi
 	echo 2 > /proc/sys/fs/suid_dumpable
 }
 

=== modified file 'use-local'
--- use-local	2017-11-15 20:07:23 +0000
+++ use-local	2019-03-04 16:41:51 +0000
@@ -4,4 +4,4 @@
 # making changes to bin/apport and want to test them without copying them to
 # /usr/share/apport/ every time.
 
-echo "|$(readlink -f $(dirname $0)/data/apport) %p %s %c %d %P" > /proc/sys/kernel/core_pattern
+echo "|$(readlink -f $(dirname $0)/data/apport) -p%p -s%s -c%c -d%d -P%P" | tee /proc/sys/kernel/core_pattern