Merge ~paelzer/ubuntu/+source/psmisc:lp1806060-fix-killall-option-parsing-bionic into ubuntu/+source/psmisc:ubuntu/bionic-devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

Experimental PPA at https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3561/+packages

[Christian Ehrhardt  (paelzer)]

PPA verified with the testcase.
Bileto tests run on ticket: https://bileto.ubuntu.com/#/ticket/3561

This is open for review now ...

[Christian Ehrhardt  (paelzer)]

tests look shaky, but not due to the upload

Maria is already masked, but needs a bump to
vorlon:16:# requires its own version of mariadb-test, that binary package now owned by mariadb-10.2.
vorlon:17:force-badtest mariadb-10.1/1:10.1.29-6ubuntu2

pcs is already masked.

Remaining issues on booth, s3ql and gvfs
At least gvfs is a known trouble maker in that context (s390x), but it also fails on others now.
Retriggered the tests, none of them looks like it would be related to the proposed upload

[Christian Ehrhardt  (paelzer)]

Actually on Bionic the badtests are defined wider, so what is left is:
- gvfs @ i386 ppc64el
- booth @ s390x
Retrying theses for now ...

There but on older versions:
- s3ql
- mariadb
Those likely will need MPs for hints-ubuntu ...

already covered are:
- ocfs2-tools
- pcs

[Christian Ehrhardt  (paelzer)]

Ok all tests that I retried worked now.
Bumping the tests os s3ql and mariadb is a necessary evil that I'll open MPs once I uploaded and see the issues in -excuses
So it is ok for upload given the review is ack'ed

[Andreas Hasenack (ahasenack)]

Ok:

- compared cosmic fix with bionic fix, same
- ran the test (nice script), ok
- even though sometimes the test script doesn't find a process to kill, by then it parsed the command line already, and if an unknown option were to be used it would have shown the help output
- killall is definitely used by many scripts out there, let's keep an eye on possible regressions

+1

review: Approve

[Andreas Hasenack (ahasenack)]

Forgot to say, also verified that upstream didn't have more follow-up commits with One More Fix.

[Christian Ehrhardt  (paelzer)]

Thanks for the re-check Andreas.
Pushed Tag and uploaded for SRU

diff --git a/debian/changelog b/debian/changelog
index 2c10bd8..2ebddaa 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+psmisc (23.1-1ubuntu0.1) bionic; urgency=medium
+
+  * fix killall option parsing (LP: #1806060)
+    - d/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch
+    - d/patches/lp1806060-0002-killall-change-to-getopt_long.patch
+    - d/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch
+
+ -- Christian Ehrhardt <[e-mail]>  Tue, 11 Dec 2018 16:46:04 +0100
+
 psmisc (23.1-1) unstable; urgency=medium
 
   * New upstream source
diff --git a/debian/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch b/debian/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch
new file mode 100644
index 0000000..6bb7b59
--- /dev/null
+++ b/debian/patches/lp1806060-0001-killall-Fix-INT-option-parsing.patch
@@ -0,0 +1,49 @@
+From cf80d701397d2ff705ab1c1250e3906f32034539 Mon Sep 17 00:00:00 2001
+From: Craig Small <[e-mail]>
+Date: Sun, 20 May 2018 20:42:12 +1000
+Subject: [PATCH] killall: Fix -INT option parsing
+
+Sometimes killall gets confused between a signal name and a set of
+options. This small patch should un-confuse it.
+
+Author: Christian Ehrhardt <[e-mail]>
+Original-Author: Craig Small <[e-mail]>
+Origin: backport, https://gitlab.com/psmisc/psmisc/commit/cf80d701397d2ff705ab1c1250e3906f32034539
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1806060
+Last-Update: 2018-12-11
+
+---
+ ChangeLog     | 1 +
+ src/killall.c | 7 ++++++-
+ 2 files changed, 7 insertions(+), 1 deletion(-)
+
+--- a/src/killall.c
++++ b/src/killall.c
+@@ -803,6 +803,7 @@ main (int argc, char **argv)
+   int sig_num;
+   int optc;
+   int myoptind;
++  int skip_error=0;
+   struct passwd *pwent = NULL;
+   char yt[16];
+   char ot[16];
+@@ -912,6 +913,7 @@ main (int argc, char **argv)
+         ignore_case = 1;
+       } else {
+ 	      sig_num = get_signal (argv[optind]+1, "killall");
++          skip_error=1;
+       }
+       break;
+     case 'V':
+@@ -947,7 +949,10 @@ main (int argc, char **argv)
+         if (argv[optind-1][1] >= '0' && argv[optind-1][1] <= '9') {
+           sig_num = atoi(argv[optind-1]+1);
+         } else {
+-          usage(NULL);
++            if (skip_error)
++                skip_error=0;
++            else
++                usage(NULL);
+         }
+       }
+       break;
diff --git a/debian/patches/lp1806060-0002-killall-change-to-getopt_long.patch b/debian/patches/lp1806060-0002-killall-change-to-getopt_long.patch
new file mode 100644
index 0000000..6eb56c3
--- /dev/null
+++ b/debian/patches/lp1806060-0002-killall-change-to-getopt_long.patch
@@ -0,0 +1,37 @@
+From b769da1626044f97a689c1665270595f36f878e7 Mon Sep 17 00:00:00 2001
+From: Craig Small <[e-mail]>
+Date: Sun, 20 May 2018 20:46:55 +1000
+Subject: [PATCH] killall: change to getopt_long
+
+with getopt_long_only -ve fails because its not sure what the -v
+is for --version or --verbose. Changing to the non-_only version means
+it doesn't try to expand the options to long options.
+
+References:
+ psmisc/psmisc#12
+
+Author: Christian Ehrhardt <[e-mail]>
+Original-Author: Craig Small <[e-mail]>
+Origin: backport, https://gitlab.com/psmisc/psmisc/commit/b769da1626044f97a689c1665270595f36f878e7
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1806060
+Last-Update: 2018-12-11
+
+---
+ ChangeLog     | 1 +
+ src/killall.c | 4 ++--
+ 2 files changed, 3 insertions(+), 2 deletions(-)
+
+--- a/src/killall.c
++++ b/src/killall.c
+@@ -856,9 +856,9 @@ main (int argc, char **argv)
+ 
+   opterr = 0;
+ #ifdef WITH_SELINUX
+-  while ( (optc = getopt_long_only(argc,argv,"egy:o:ilqrs:u:vwZ:VIn:",options,NULL)) != -1) {
++  while ( (optc = getopt_long(argc,argv,"egy:o:ilqrs:u:vwZ:VIn:",options,NULL)) != -1) {
+ #else
+-  while ( (optc = getopt_long_only(argc,argv,"egy:o:ilqrs:u:vwVIn:",options,NULL)) != -1) {
++  while ( (optc = getopt_long(argc,argv,"egy:o:ilqrs:u:vwVIn:",options,NULL)) != -1) {
+ #endif
+     switch (optc) {
+     case 'e':
diff --git a/debian/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch b/debian/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch
new file mode 100644
index 0000000..026f280
--- /dev/null
+++ b/debian/patches/lp1806060-0003-killall-Another-go-at-option-parsing.patch
@@ -0,0 +1,87 @@
+From 258ee9166e585f87005d3a9686938a4fa26669f9 Mon Sep 17 00:00:00 2001
+From: Craig Small <[e-mail]>
+Date: Tue, 18 Sep 2018 21:17:00 +1000
+Subject: [PATCH] killall: Another go at option parsing
+
+This now seems to be working. There are some evil hacks, especially for
+the -ve option combination but it seems that we have a winner.
+
+Added a bunch more option parsing tests which picked up -ILL passed but
+-VTALRM did not. Not sure why, but length seems my guess
+
+References:
+ psmisc/psmisc#13
+ psmisc/psmisc#12
+
+Author: Christian Ehrhardt <[e-mail]>
+Original-Author: Craig Small <[e-mail]>
+Origin: backport, https://gitlab.com/psmisc/psmisc/commit/258ee9166e585f87005d3a9686938a4fa26669f9
+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1806060
+Last-Update: 2018-12-11
+
+---
+ ChangeLog                          |  2 +-
+ src/killall.c                      | 20 ++++++++++++++------
+ testsuite/killall.test/killall.exp | 13 +++++++++++++
+ 3 files changed, 28 insertions(+), 7 deletions(-)
+
+--- a/src/killall.c
++++ b/src/killall.c
+@@ -856,7 +856,7 @@ main (int argc, char **argv)
+ 
+   opterr = 0;
+ #ifdef WITH_SELINUX
+-  while ( (optc = getopt_long(argc,argv,"egy:o:ilqrs:u:vwZ:VIn:",options,NULL)) != -1) {
++  while ( (optc = getopt_long_only(argc,argv,"egy:o:ilqrs:u:vwZ:VIn:",options,NULL)) != -1) {
+ #else
+   while ( (optc = getopt_long(argc,argv,"egy:o:ilqrs:u:vwVIn:",options,NULL)) != -1) {
+ #endif
+@@ -913,7 +913,7 @@ main (int argc, char **argv)
+         ignore_case = 1;
+       } else {
+ 	      sig_num = get_signal (argv[optind]+1, "killall");
+-          skip_error=1;
++          skip_error=optind;
+       }
+       break;
+     case 'V':
+@@ -921,8 +921,10 @@ main (int argc, char **argv)
+       if (strcmp(argv[optind-1],"-V") == 0 || strncmp(argv[optind-1],"--",2) == 0) {
+         print_version();
+         return 0;
++      } else {
++        sig_num = get_signal (argv[optind]+1, "killall");
++        skip_error=optind;
+       }
+-	    sig_num = get_signal (argv[optind]+1, "killall");
+       break;
+     case 'n':
+       opt_ns_pid = atoi(optarg);
+@@ -940,6 +942,15 @@ main (int argc, char **argv)
+       break;
+ #endif /*WITH_SELINUX*/
+     case '?':
++      if (skip_error == optind)
++          break;
++      /* Sigh, this is a hack because -ve could be -version or
++       * -verbose */
++      if (strncmp(argv[optind-1], "-ve", 3) == 0) {
++          verbose=1;
++          exact=1;
++          break;
++      }
+       /* Signal names are in uppercase, so check to see if the argv
+        * is upper case */
+       if (argv[optind-1][1] >= 'A' && argv[optind-1][1] <= 'Z') {
+@@ -949,10 +960,7 @@ main (int argc, char **argv)
+         if (argv[optind-1][1] >= '0' && argv[optind-1][1] <= '9') {
+           sig_num = atoi(argv[optind-1]+1);
+         } else {
+-            if (skip_error)
+-                skip_error=0;
+-            else
+-                usage(NULL);
++            usage(NULL);
+         }
+       }
+       break;
diff --git a/debian/patches/series b/debian/patches/series
index 4bbbe65..ad87743 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,4 @@
 manpage-fixes
+lp1806060-0001-killall-Fix-INT-option-parsing.patch
+lp1806060-0002-killall-change-to-getopt_long.patch
+lp1806060-0003-killall-Another-go-at-option-parsing.patch