Merge ~paelzer/ubuntu/+source/chrony:lp-1898000-argparse-GROOVY into ubuntu/+source/chrony:ubuntu/groovy-devel

[Christian Ehrhardt ]

[Christian Ehrhardt  (paelzer)]

PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4301
Bug: https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/1898000

[Andreas Hasenack (ahasenack)]

Taking a look

[Bryce Harrington (bryce)]

A pattern I often use with bash scripts is to pre-process the args to handle long args and other oddities, and then run it through getopts for the full processing. So for example:

for arg in "${@}"; do
    shift
    case "${arg}" in
        "-1")
            set -- "${@}" "\"-1\""
            ;;
        "--long-opt-with-val")
            set -- "${@}" "-L"
            ;;
        *)
            set -- "${@}" "${arg}"
    esac
done

Once you've handled your corner cases, you can pass "${@}" to getopts normally. Below is a fully detailed example:

#!/bin/bash

for arg in "${@}"; do
    shift
    case "${arg}" in
        "-1")
            set -- "${@}" "\"-1\""
            ;;
        "--long-opt-with-val")
            set -- "${@}" "-L"
            ;;
        *)
            set -- "${@}" "${arg}"
    esac
done

# Now, if desired, can safely run the args through getopts:
OPTIND=1
while getopts ":xdF:L:" opt "${@}"; do
    case "${opt}" in
        x) echo "-x is set" ;;
        d) echo "-d is set" ;;
        F) echo "-F = ${OPTARG}" ;;
        L) echo "Long ${OPTARG}" ;;
    esac
done
shift $(( OPTIND - 1 ))

echo "Remaining args: ${@}"

Output:

$ /tmp/chronyd-starter-x.sh -F 1 -x
-F = 1
-x is set
Remaining args:
$ /tmp/chronyd-starter-x.sh -F -1 -x
-F = "-1"
-x is set
Remaining args:
$ /tmp/chronyd-starter-x.sh -dx -F -1 foo bar
-d is set
-x is set
-F = "-1"
Remaining args: foo bar
$ /tmp/chronyd-starter-x.sh -xF -1 foo bar
-x is set
-F = "-1"
Remaining args: foo bar
$ /tmp/chronyd-starter-x.sh --long-opt-with-val foo -dx -F 1 bar baz
Long foo
-d is set
-x is set
-F = 1
Remaining args: bar baz

[Christian Ehrhardt  (paelzer)]

Thanks Bryce, now we have three options that work :-)

We only want to know if -x is set and nothing else - and we don't want to have to know what potential options there are - in the case above I don't want to care if one day there is a valid "-V 8" and update the getopt string.
To be clear, for full parsing your suggestion is great, but that isn't what I want/need here.

Therefore I'd prefer the small version I have proposed which achieves exactly that and nothing more but therefore also saves some complexities and hopefully will need no updates (that we might forget on merges) down the road.

[Andreas Hasenack (ahasenack)]

abstain, Bryce was much more thorough :)

review: Abstain

[Bryce Harrington (bryce)]

@Andreas, nah he didn't like my suggestion, so review back to you.

[Simon Déziel (sdeziel)]

I too like the simplicity of grep and can confirm the proposed code works so thanks!

[Christian Ehrhardt  (paelzer)]

> > +for arg in $@; do
> > + echo ARG = $arg
>
> Looks like a debug statement that was left behind.
>

That is absolutely correct and fixed now (force push).

[Christian Ehrhardt  (paelzer)]

@Andreas - I think we are waiting for your ack?

[Andreas Hasenack (ahasenack)]

I can see how insisting on getopts will make this complicated, fast, so +1 on the grep approach. Simple to understand.

review: Approve

[Christian Ehrhardt  (paelzer)]

Thanks, tagged and uploaded

To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/chrony
 * [new tag] upload/3.5.1-1ubuntu2 -> upload/3.5.1-1ubuntu2

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading chrony_3.5.1-1ubuntu2.dsc: done.
  Uploading chrony_3.5.1-1ubuntu2.debian.tar.xz: done.
  Uploading chrony_3.5.1-1ubuntu2_source.buildinfo: done.
  Uploading chrony_3.5.1-1ubuntu2_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 72c5eb4..1bcdf43 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+chrony (3.5.1-1ubuntu2) groovy; urgency=medium
+
+  * d/chronyd-starter.sh: fix commandline argument parsing (LP: #1898000)
+
+ -- Christian Ehrhardt <[e-mail]>  Tue, 06 Oct 2020 12:20:40 +0200
+
 chrony (3.5.1-1ubuntu1) groovy; urgency=medium
 
   * Merge with Debian unstable. Remaining changes:
diff --git a/debian/chronyd-starter.sh b/debian/chronyd-starter.sh
index 55cc285..2539ffe 100755
--- a/debian/chronyd-starter.sh
+++ b/debian/chronyd-starter.sh
@@ -25,12 +25,10 @@ fi
 
 # Check if -x is already set manually, don't process further if that is the case
 X_SET=0
-while getopts ":x" opt; do
-    case $opt in
-        x)
-            X_SET=1
-            ;;
-    esac
+for arg in $@; do
+    if echo "$arg" | grep -q -e '^-[a-zA-Z0-9]*x'; then
+         X_SET=1
+    fi
 done
 
 if [ ${X_SET} -ne 1 ]; then