[pacman-dev] [PATCH 1/8] scripts/library: introduce parseopts
This will replace our current options parser used in pacman-key and makepkg. It follows heuristics closer to that of GNU getopt long (and thus pacman itself), with the exception that it does not allow for options with optional arguments. Due to the way this parser will be used, this sort of functionality will not be needed. Instead of relying on eval+set, options are normalized into an array, OPTRET, which callers should expect to be populated after returning from parseopts. This avoids problems with quotes and spaces in arguments, assuming that the user quotes properly when passing into the application. A new test harness for parseopts is added in test/scripts. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- Makefile.am | 6 +- configure.ac | 1 + scripts/Makefile.am | 1 + scripts/library/README | 12 ++++ scripts/library/parseopts.sh | 113 ++++++++++++++++++++++++++++++++++++ scripts/po/POTFILES.in | 1 + test/scripts/Makefile.am | 9 +++ test/scripts/parseopts_test.sh | 123 ++++++++++++++++++++++++++++++++++++++++ 8 files changed, 264 insertions(+), 2 deletions(-) create mode 100644 scripts/library/parseopts.sh create mode 100644 test/scripts/Makefile.am create mode 100755 test/scripts/parseopts_test.sh diff --git a/Makefile.am b/Makefile.am index a024a2e..e08b809 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = lib/libalpm src/util src/pacman scripts etc test/pacman test/util +SUBDIRS = lib/libalpm src/util src/pacman scripts etc test/pacman test/util test/scripts if WANT_DOC SUBDIRS += doc endif @@ -23,7 +23,7 @@ dist_pkgdata_DATA = \ proto/ChangeLog.proto # run the pactest test suite and vercmp tests -check-local: test/pacman test/util src/pacman src/util +check-local: test/pacman test/scripts test/util src/pacman src/util LC_ALL=C $(PYTHON) $(top_srcdir)/test/pacman/pactest.py --debug=1 \ --test $(top_srcdir)/test/pacman/tests/*.py \ -p $(top_builddir)/src/pacman/pacman @@ -31,6 +31,8 @@ check-local: test/pacman test/util src/pacman src/util $(top_builddir)/src/util/pacsort $(SH) $(top_srcdir)/test/util/vercmptest.sh \ $(top_builddir)/src/util/vercmp + $(BASH_SHELL) $(top_srcdir)/test/scripts/parseopts_test.sh \ + $(top_srcdir)/scripts/library/parseopts.sh # create the pacman DB and cache directories upon install install-data-local: diff --git a/configure.ac b/configure.ac index eccf425..a6114f8 100644 --- a/configure.ac +++ b/configure.ac @@ -442,6 +442,7 @@ doc/Makefile etc/Makefile test/pacman/Makefile test/pacman/tests/Makefile +test/scripts/Makefile test/util/Makefile contrib/Makefile Makefile diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 328fbff..7662fba 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -27,6 +27,7 @@ EXTRA_DIST = \ LIBRARY = \ library/output_format.sh \ + library/parseopts.sh \ library/parse_options.sh # Files that should be removed, but which Automake does not know. diff --git a/scripts/library/README b/scripts/library/README index 1e9c962..33f6321 100644 --- a/scripts/library/README +++ b/scripts/library/README @@ -13,3 +13,15 @@ A getopt replacement to avoids portability issues, in particular the lack of long option name support in the default getopt provided by some platforms. Usage: parse_option $SHORT_OPTS $LONG_OPTS "$@" + +parseopts.sh: +A getopt_long-like parser which portably supports longopts and shortopts +with some GNU extensions. It does not allow for options with optional +arguments. For both short and long opts, options requiring an argument +should be suffixed with a colon. Long opts must be passed comma +separated. +Usage: + parseopts "fb:z" "foo,bar:,baz" "$@"; set -- "${OPTRET[@]}" +Returns: + 0: parse success + 1: parse failure (error message supplied) diff --git a/scripts/library/parseopts.sh b/scripts/library/parseopts.sh new file mode 100644 index 0000000..912bb49 --- /dev/null +++ b/scripts/library/parseopts.sh @@ -0,0 +1,113 @@ +# getopt-like parser +parseopts() { + local opt= optarg= i= shortopts=$1 + local -a longopts unused_argv + + # split the longopt array on commas for easier parsing + IFS=, read -ra longopts <<< "$2" + shift 2 + + get_argreq() { + local o re="^($1)(:?:?)$" + for o in "${longopts[@]}"; do + # found option, return number of colons (0 = none, 1 = required) + [[ $o =~ $re ]] && return ${#BASH_REMATCH[2]} + done + # failure + return 255 + } + + while (( $# )); do + case $1 in + --) # explicit end of options + shift + break + ;; + -[!-]*) # short option + for (( i = 1; i < ${#1}; i++ )); do + opt=${1:i:1} + + # option doesn't exist + if [[ $shortopts != *$opt* ]]; then + echo "@SCRIPTNAME@: $(gettext "unrecognized option") '-$opt'" >&2 + OPTRET=(--) + return 1 + fi + + OPTRET+=("-$opt") + # option requires optarg + if [[ $shortopts = *$opt:* ]]; then + # if we're not at the end of the option chunk, the rest is the optarg + if (( i < ${#1} - 1 )); then + OPTRET+=("${1:i+1}") + break + # if we're at the end, grab the the next positional, if it exists + elif (( i == ${#1} - 1 )) && [[ $2 ]]; then + OPTRET+=("$2") + shift + break + # parse failure + else + printf "@SCRIPTNAME@: %s '-%s'\n" "$(gettext "option %s requires an argument")" "$opt" >&2 + OPTRET=(--) + return 1 + fi + fi + done + ;; + --?*=*|--?*) # long option + IFS='=' read -r opt optarg <<< "${1#--}" + get_argreq "$opt" + case $? in + *) + opt=${opt%%:} + ;;& + # intentional fallthrough + 0) + if [[ $optarg ]]; then + printf "@SCRIPTNAME@: %s '--%s'\n" "$(gettext "option %s does not allow an argument")" "$opt" >&2 + OPTRET=(--) + return 1 + else + OPTRET+=("--$opt") + shift + continue 2 + fi + ;; + 1) + # --longopt=optarg + if [[ $optarg ]]; then + OPTRET+=("--$opt" "$optarg") + shift + # --longopt optarg + elif [[ $2 ]]; then + OPTRET+=("--$opt" "$2" ) + shift 2 + else + printf "@SCRIPTNAME@: %s '--%s'\n" "$(gettext "option %s requires an argument")" "$opt" >&2 + OPTRET=(--) + return 1 + fi + continue 2 + ;; + 255) + printf "@SCRIPTNAME@: %s '--%s'\n" "$(gettext "unrecognized option")" "$opt" >&2 + OPTRET=(--) + return 1 + ;; + *) + printf "@SCRIPTNAME@: $(gettext "internal error: invalid option in longopt array") '--$opt'" >&2 + esac + ;; + *) # non-option arg encountered, add it as a parameter + unused_argv+=("$1") + ;; + esac + shift + done + + # add end-of-opt terminator and any leftover positional parameters + OPTRET+=('--' "${unused_argv[@]}" "$@") + + return 0 +} diff --git a/scripts/po/POTFILES.in b/scripts/po/POTFILES.in index 007e535..01cc235 100644 --- a/scripts/po/POTFILES.in +++ b/scripts/po/POTFILES.in @@ -9,3 +9,4 @@ scripts/pkgdelta.sh.in scripts/repo-add.sh.in scripts/library/output_format.sh scripts/library/parse_options.sh +scripts/library/parseopts.sh diff --git a/test/scripts/Makefile.am b/test/scripts/Makefile.am new file mode 100644 index 0000000..b949e88 --- /dev/null +++ b/test/scripts/Makefile.am @@ -0,0 +1,9 @@ +check_SCRIPTS = \ + parseopts_test.sh + +noinst_SCRIPTS = $(check_SCRIPTS) + +EXTRA_DIST = \ + $(check_SCRIPTS) + +# vim:set ts=2 sw=2 noet: diff --git a/test/scripts/parseopts_test.sh b/test/scripts/parseopts_test.sh new file mode 100755 index 0000000..58f2a8e --- /dev/null +++ b/test/scripts/parseopts_test.sh @@ -0,0 +1,123 @@ +#!/bin/bash + +declare -i testcount=0 pass=0 fail=0 + +# source the library function +if [[ -z $1 || ! -f $1 ]]; then + printf "error: path to parse_option library not provided or does not exist\n" + exit 1 +fi +. "$1" + +if ! type -t parseopts >/dev/null; then + printf 'parse_options function not found\n' + exit 1 +fi + +# borrow opts from makepkg +OPT_SHORT="AcdefFghiLmop:rRsV" +OPT_LONG="allsource,asroot,ignorearch,check,clean,nodeps" +OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver" +OPT_LONG+=",install,key:,log,nocolor,nobuild,nocheck,nosign,pkg:,rmdeps" +OPT_LONG+=",repackage,skipinteg,sign,source,syncdeps,version,config:" +OPT_LONG+=",noconfirm,noprogressbar" + +parse() { + local result=$1 tokencount=$2; shift 2 + + (( ++testcount )) + parseopts "$OPT_SHORT" "$OPT_LONG" "$@" 2>/dev/null + test_result "$result" "$tokencount" "$*" "${OPTRET[@]}" + unset OPTRET +} + +test_result() { + local result=$1 tokencount=$2 input=$3; shift 3 + + if { [[ $result = "$*" ]] || [[ $2 = NULL && -z $1 ]]; } && (( tokencount == $# )); then + (( ++pass )) + else + printf '[TEST %3s]: FAIL\n' "$testcount" + printf ' input: %s\n' "$input" + printf ' output: %s (%s tokens)\n' "$*" "$#" + printf ' expected: %s (%s tokens)\n' "$result" "$tokencount" + echo + (( ++fail )) + fi +} + +summarize() { + if (( !fail )); then + printf 'All %s tests successful\n' "$testcount" + exit 0 + else + printf '%s of %s tests failed\n' "$fail" "$testcount" + exit 1 + fi +} +trap 'summarize' EXIT + +printf 'Beginning parse_options tests\n' + +# usage: parse <expected result> <token count> test-params... +# a failed parse will match only the end of options marker '--' + +# no options +parse '--' 1 + +# short options +parse '-s -r --' 3 -s -r + +# short options, no spaces +parse '-s -r --' 3 -sr + +# short opt missing an opt arg +parse '--' 1 -s -p + +# short opt with an opt arg +parse '-p PKGBUILD -L --' 4 -p PKGBUILD -L + +# short opt with an opt arg, no space +parse '-p PKGBUILD --' 3 -pPKGBUILD + +# valid shortopts as a long opt +parse '--' 1 --sir + +# long opt wiht no optarg +parse '--log --' 2 --log + +# long opt with missing optarg +parse '--' 1 -sr --pkg + +# long opt with optarg +parse '--pkg foo --' 3 --pkg foo + +# long opt with optarg with whitespace +parse '--pkg foo bar -- baz' 4 --pkg "foo bar" baz + +# long opt with optarg with = +parse '--pkg foo=bar -- baz' 4 --pkg foo=bar baz + +# long opt with explicit optarg +parse '--pkg bar -- foo baz' 5 foo --pkg=bar baz + +# long opt with explicit optarg, with whitespace +parse '--pkg foo bar -- baz' 4 baz --pkg="foo bar" + +# long opt with explicit optarg that doesn't take optarg +parse '--' 1 --force=always -s + +# long opt with explicit optarg with = +parse '--pkg foo=bar --' 3 --pkg=foo=bar + +# explicit end of options with options after +parse '-s -r -- foo bar baz' 6 -s -r -- foo bar baz + +# non-option parameters mixed in with options +parse '-s -r -- foo baz' 5 -s foo baz -r + +# optarg with whitespace +parse '-p foo bar -s --' 4 -p'foo bar' -s + +# non-option parameter with whitespace +parse '-i -- foo bar' 3 -i 'foo bar' -- 1.7.10
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- doc/makepkg.8.txt | 3 ++- scripts/Makefile.am | 2 +- scripts/makepkg.sh.in | 10 +++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index df41e18..f80d7f2 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -153,7 +153,8 @@ Options the GPL when distributing binary packages. *\--pkg <list>*:: - Only build listed packages from a split package. + Only build listed packages from a split package. Multiple packages should + be comma separated in the list. *\--check*:: Run the check() function in the PKGBUILD, overriding the setting in diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 7662fba..0df90e1 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -68,7 +68,7 @@ $(OURSCRIPTS): Makefile makepkg: \ $(srcdir)/makepkg.sh.in \ - $(srcdir)/library/parse_options.sh + $(srcdir)/library/parseopts.sh pacman-db-upgrade: \ $(srcdir)/pacman-db-upgrade.sh.in \ diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 46191ee..4e3b0e3 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1885,7 +1885,7 @@ canonicalize_path() { fi } -m4_include(library/parse_options.sh) +m4_include(library/parseopts.sh) usage() { printf "makepkg (pacman) %s\n" "$myver" @@ -1962,11 +1962,11 @@ OPT_LONG+=",version,config:" # Pacman Options OPT_LONG+=",noconfirm,noprogressbar" -if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then +if ! parseopts "$OPT_SHORT" "$OPT_LONG" "$@"; then echo; usage; exit 1 # E_INVALID_OPTION; fi -eval set -- "$OPT_TEMP" -unset OPT_SHORT OPT_LONG OPT_TEMP +set -- "${OPTRET[@]}" +unset OPT_SHORT OPT_LONG OPTRET while true; do case "$1" in @@ -1997,7 +1997,7 @@ while true; do --nosign) SIGNPKG='n' ;; -o|--nobuild) NOBUILD=1 ;; -p) shift; BUILDFILE=$1 ;; - --pkg) shift; PKGLIST=($1) ;; + --pkg) shift; IFS=, read -ra PKGLIST <<<"$1" ;; -r|--rmdeps) RMDEPS=1 ;; -R|--repackage) REPKG=1 ;; --skipchecksums) SKIPCHECKSUMS=1 ;; -- 1.7.10
On 13/04/12 00:54, Dave Reisner wrote:
Signed-off-by: Dave Reisner <dreisner@archlinux.org>
Ack once a patch is added to prevent pkgname containing a comma. As no package in existence (being the Arch repos and the AUR...) has a comma in its name, that will not break anything so I decided it is best to be proactive here. Allan
Make this option additive, so that the following two operations are equivalent: makepkg --pkg foo --pkg bar makepkg --pkg foo,bar --- doc/makepkg.8.txt | 2 +- scripts/makepkg.sh.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index f80d7f2..57af98f 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -154,7 +154,7 @@ Options *\--pkg <list>*:: Only build listed packages from a split package. Multiple packages should - be comma separated in the list. + be comma separated in the list. This option can be specified multiple times. *\--check*:: Run the check() function in the PKGBUILD, overriding the setting in diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 4e3b0e3..d2510c1 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1997,7 +1997,7 @@ while true; do --nosign) SIGNPKG='n' ;; -o|--nobuild) NOBUILD=1 ;; -p) shift; BUILDFILE=$1 ;; - --pkg) shift; IFS=, read -ra PKGLIST <<<"$1" ;; + --pkg) shift; IFS=, read -ra p <<<"$1"; PKGLIST+=("${p[@]}"); unset p ;; -r|--rmdeps) RMDEPS=1 ;; -R|--repackage) REPKG=1 ;; --skipchecksums) SKIPCHECKSUMS=1 ;; -- 1.7.10
This requires an ugly amount of reworking of how pacman-key handles options. The change simply to avoid passing keys, files, and directories as arguments to options, but to leave them as arguments to the overall program. This is reasonable since pacman-key limits the user to essentially one operation per invocation (like pacman). Since we now pass around the positional parameters to the various operations, we can add some better sanity checking. Each operation is responsible for testing input and making sure it can operate properly, otherwise it throws an error and exits. The doc is updated to reflect this, and uses similar verbiage as pacman, describing the non-option arguments now passed to pacman-key as targets. Similar to the doc, --help is reorganized to separate operations and options and remove argument tokens from operations. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- I really hate this patch, but I don't think it makes sense to split it. doc/pacman-key.8.txt | 74 +++++++++--------- scripts/Makefile.am | 2 +- scripts/pacman-key.sh.in | 187 +++++++++++++++++++++++++--------------------- 3 files changed, 144 insertions(+), 119 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 3631ec8..96ac31c 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -12,7 +12,7 @@ pacman-key - manage pacman's list of trusted keys Synopsis -------- -'pacman-key' [options] +'pacman-key' [options] operation [targets] Description @@ -26,45 +26,40 @@ More complex keyring management can be achieved using GnuPG directly combined wi the '\--homedir' option pointing at the pacman keyring (located in +{sysconfdir}/pacman.d/gnupg+ by default). +Invoking pacman-key consists of supplying an operation with any potential +options and targets to operate on. Depending on the operation, a 'target' may +be a valid key identifier, filename, or directory. -Options -------- -*-a, \--add* [file(s)]:: +Operations +---------- +*-a, \--add*:: Add the key(s) contained in the specified file or files to pacman's keyring. If a key already exists, update it. -*\--config* <file>:: - Use an alternate config file instead of the +{sysconfdir}/pacman.conf+ - default. - -*-d, \--delete* <keyid(s)>:: +*-d, \--delete*:: Remove the key(s) identified by the specified keyid(s) from pacman's keyring. -*-e, \--export* [keyid(s)]:: +*-e, \--export*:: Export key(s) identified by the specified keyid(s) to 'stdout'. If no keyid is specified, all keys will be exported. -*\--edit-key* <keyid(s)>:: +*\--edit-key*:: Present a menu for key management task on the specified keyid(s). Useful for adjusting a keys trust level. -*-f, \--finger* [keyid(s)]:: +*-f, \--finger*:: List a fingerprint for each specified keyid, or for all known keys if no keyids are specified. -*\--gpgdir* <dir>:: - Set an alternate home directory for GnuPG. If unspecified, the value is - read from +{sysconfdir}/pacman.conf+. - *-h, \--help*:: Output syntax and command line options. -*\--import* <dir(s)>:: +*\--import*:: Imports keys from `pubring.gpg` into the public keyring from the specified directories. -*\--import-trustdb* <dir(s)> :: +*\--import-trustdb*:: Imports ownertrust values from `trustdb.gpg` into the shared trust database from the specified directories. @@ -72,42 +67,53 @@ Options Ensure the keyring is properly initialized and has the required access permissions. -*\--keyserver* <keyserver>:: - Use the specified keyserver if the operation requires one. This will take - precedence over any keyserver option specified in a `gpg.conf` - configuration file. Running '\--init' with this option will set the default - keyserver if one was not already configured. - -*-l, \--list-keys* [keyid(s)]:: +*-l, \--list-keys*:: Lists all or specified keys from the public keyring. -*\--list-sigs* [keyid(s)]:: +*\--list-sigs*:: Same as '\--list-keys', but the signatures are listed too. -*\--lsign-key* <keyid>:: +*\--lsign-key*:: Locally sign the given key. This is primarily used to root the web of trust in the local private key generated by '\--init'. -*-r, \--recv-keys* <keyid(s)>:: +*-r, \--recv-keys*:: Equivalent to '\--recv-keys' in GnuPG. -*\--refresh-keys* [keyid(s)]:: +*\--refresh-keys*:: Equivalent to '\--refresh-keys' in GnuPG. -*\--populate* [keyring(s)]:: +*\--populate*:: Reload the default keys from the (optionally provided) keyrings in +{pkgdatadir}/keyrings+. For more information, see <<SC,Providing a Keyring for Import>> below. *-u, \--updatedb*:: - Equivalent to '\--check-trustdb' in GnuPG. - -*-v, \--verify* <signature>:: - Verify the given signature file. + Equivalent to '\--check-trustdb' in GnuPG. This operation can be specified with + other operations. *-V, \--version*:: Displays the program version. +*-v, \--verify*:: + Verify the given signature file. + +Options +------- +*\--config* <file>:: + Use an alternate config file instead of the +{sysconfdir}/pacman.conf+ + default. + +*\--gpgdir* <dir>:: + Set an alternate home directory for GnuPG. If unspecified, the value is + read from +{sysconfdir}/pacman.conf+. + +*\--keyserver* <keyserver>:: + Use the specified keyserver if the operation requires one. This will take + precedence over any keyserver option specified in a `gpg.conf` + configuration file. Running '\--init' with this option will set the default + keyserver if one was not already configured. + Providing a Keyring for Import ------------------------------ diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 0df90e1..fc70732 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -77,7 +77,7 @@ pacman-db-upgrade: \ pacman-key: \ $(srcdir)/pacman-key.sh.in \ $(srcdir)/library/output_format.sh \ - $(srcdir)/library/parse_options.sh + $(srcdir)/library/parseopts.sh pacman-optimize: \ $(srcdir)/pacman-optimize.sh.in \ diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 288b76e..b2c3da9 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -49,40 +49,43 @@ DEFAULT_KEYSERVER='hkp://pool.sks-keyservers.net' m4_include(library/output_format.sh) -m4_include(library/parse_options.sh) +m4_include(library/parseopts.sh) usage() { printf "pacman-key (pacman) %s\n" ${myver} echo - printf -- "$(gettext "Usage: %s [options]")\n" $(basename $0) + printf -- "$(gettext "Usage: %s [options] operation [targets]")\n" $(basename $0) echo printf -- "$(gettext "Manage pacman's list of trusted keys")\n" echo - printf -- "$(gettext "Options:")\n" - printf -- "$(gettext " -a, --add [file(s)] Add the specified keys (empty for stdin)")\n" - printf -- "$(gettext " -d, --delete <keyid(s)> Remove the specified keyids")\n" - printf -- "$(gettext " -e, --export [keyid(s)] Export the specified or all keyids")\n" - printf -- "$(gettext " -f, --finger [keyid(s)] List fingerprint for specified or all keyids")\n" - printf -- "$(gettext " -h, --help Show this help message and exit")\n" - printf -- "$(gettext " -l, --list-keys [keyid(s)] List the specified or all keys")\n" - printf -- "$(gettext " -r, --recv-keys <keyid(s)> Fetch the specified keyids")\n" + printf -- "$(gettext "Operations:")\n" + printf -- "$(gettext " -a, --add Add the specified keys (empty for stdin)")\n" + printf -- "$(gettext " -d, --delete Remove the specified keyids")\n" + printf -- "$(gettext " -e, --export Export the specified or all keyids")\n" + printf -- "$(gettext " -f, --finger List fingerprint for specified or all keyids")\n" + printf -- "$(gettext " -l, --list-keys List the specified or all keys")\n" + printf -- "$(gettext " -r, --recv-keys Fetch the specified keyids")\n" printf -- "$(gettext " -u, --updatedb Update the trustdb of pacman")\n" - printf -- "$(gettext " -v, --verify <signature> Verify the file specified by the signature")\n" - printf -- "$(gettext " -V, --version Show program version")\n" + printf -- "$(gettext " -v, --verify Verify the file specified by the signature")\n" + printf -- "$(gettext " --edit-key Present a menu for key management task on keyids")\n" + printf -- "$(gettext " --import Imports pubring.gpg from dir(s)")\n" + printf -- "$(gettext " --import-trustdb Imports ownertrust values from trustdb.gpg in dir(s)")\n" + printf -- "$(gettext " --init Ensure the keyring is properly initialized")\n" + printf -- "$(gettext " --list-sigs List keys and their signatures")\n" + printf -- "$(gettext " --lsign-key Locally sign the specified keyid")\n" + printf -- "$(gettext " --populate Reload the default keys from the (given) keyrings\n\ + in '%s'")\n" "@pkgdatadir@/keyrings" + printf -- "$(gettext " --refresh-keys Update specified or all keys from a keyserver")\n" + echo + printf -- "$(gettext "Options:")\n" printf -- "$(gettext " --config <file> Use an alternate config file (instead of\n\ '%s')")\n" "@sysconfdir@/pacman.conf" - printf -- "$(gettext " --edit-key <keyid(s)> Present a menu for key management task on keyids")\n" printf -- "$(gettext " --gpgdir <dir> Set an alternate directory for GnuPG (instead\n\ of '%s')")\n" "@sysconfdir@/pacman.d/gnupg" - printf -- "$(gettext " --import <dir(s)> Imports pubring.gpg from dir(s)")\n" - printf -- "$(gettext " --import-trustdb <dir(s)> Imports ownertrust values from trustdb.gpg in dir(s)")\n" - printf -- "$(gettext " --init Ensure the keyring is properly initialized")\n" - printf -- "$(gettext " --keyserver Specify a keyserver to use if necessary")\n" - printf -- "$(gettext " --list-sigs [keyid(s)] List keys and their signatures")\n" - printf -- "$(gettext " --lsign-key <keyid> Locally sign the specified keyid")\n" - printf -- "$(gettext " --populate [keyring(s)] Reload the default keys from the (given) keyrings\n\ - in '%s'")\n" "@pkgdatadir@/keyrings" - printf -- "$(gettext " --refresh-keys [keyid(s)] Update specified or all keys from a keyserver")\n" + printf -- "$(gettext " --keyserver <server-url> Specify a keyserver to use if necessary")\n" + echo + printf -- "$(gettext " -h, --help Show this help message and exit")\n" + printf -- "$(gettext " -V, --version Show program version")\n" } version() { @@ -146,7 +149,7 @@ add_gpg_conf_option() { check_keyids_exist() { local ret=0 - for key in "${KEYIDS[@]}"; do + for key in "$@"; do # Verify if the key exists in pacman's keyring if ! "${GPG_PACMAN[@]}" --list-keys "$key" &>/dev/null ; then error "$(gettext "The key identified by %s could not be found locally.")" "$key" @@ -217,16 +220,16 @@ check_keyring() { populate_keyring() { local KEYRING_IMPORT_DIR='@pkgdatadir@/keyrings' - local keyring + local keyring KEYRINGIDS=("$@") local ret=0 - if [[ -z ${KEYRINGIDS[@]} ]]; then + if (( ${#KEYRINGIDS[*]} == 0 )); then # get list of all available keyrings shopt -s nullglob KEYRINGIDS=("$KEYRING_IMPORT_DIR"/*.gpg) shopt -u nullglob KEYRINGIDS=("${KEYRINGIDS[@]##*/}") KEYRINGIDS=("${KEYRINGIDS[@]%.gpg}") - if [[ -z ${KEYRINGIDS[@]} ]]; then + if (( ${#KEYRINGIDS[*]} == 0 )); then error "$(gettext "No keyring files exist in %s.")" "$KEYRING_IMPORT_DIR" ret=1 fi @@ -311,24 +314,36 @@ populate_keyring() { } add_keys() { - if ! "${GPG_PACMAN[@]}" --quiet --batch --import "${KEYFILES[@]}" ; then + if (( $# == 0 )); then + error "$(gettext "No keys specified")" + exit 1 + fi + if ! "${GPG_PACMAN[@]}" --quiet --batch --import "$@" ; then error "$(gettext "A specified keyfile could not be added to the gpg keychain.")" exit 1 fi } delete_keys() { - check_keyids_exist - if ! "${GPG_PACMAN[@]}" --quiet --batch --delete-key --yes "${KEYIDS[@]}" ; then + if (( $# == 0 )); then + error "$(gettext "No keys specified")" + exit 1 + fi + check_keyids_exist "$@" + if ! "${GPG_PACMAN[@]}" --quiet --batch --delete-key --yes "$@" ; then error "$(gettext "A specified key could not be removed from the gpg keychain.")" exit 1 fi } edit_keys() { - check_keyids_exist + if (( $# == 0 )); then + error "$(gettext "No keys specified")" + exit 1 + fi + check_keyids_exist "$@" local ret=0 - for key in "${KEYIDS[@]}"; do + for key in "$@"; do if ! "${GPG_PACMAN[@]}" --edit-key "$key" ; then error "$(gettext "The key identified by %s could not be edited.")" "$key" ret=1 @@ -340,8 +355,8 @@ edit_keys() { } export_keys() { - check_keyids_exist - if ! "${GPG_PACMAN[@]}" --armor --export "${KEYIDS[@]}" ; then + check_keyids_exist "$@" + if ! "${GPG_PACMAN[@]}" --armor --export "$@" ; then error "$(gettext "A specified key could not be exported from the gpg keychain.")" exit 1 fi @@ -349,7 +364,7 @@ export_keys() { finger_keys() { check_keyids_exist - if ! "${GPG_PACMAN[@]}" --batch --fingerprint "${KEYIDS[@]}" ; then + if ! "${GPG_PACMAN[@]}" --batch --fingerprint "$@" ; then error "$(gettext "The fingerprint of a specified key could not be determined.")" exit 1 fi @@ -358,7 +373,7 @@ finger_keys() { import_trustdb() { local importdir local ret=0 - for importdir in "${IMPORT_DIRS[@]}"; do + for importdir in "$@"; do if [[ -f "${importdir}/trustdb.gpg" ]]; then gpg --homedir "${importdir}" --export-ownertrust | \ "${GPG_PACMAN[@]}" --import-ownertrust - @@ -379,7 +394,7 @@ import_trustdb() { import() { local importdir local ret=0 - for importdir in "${IMPORT_DIRS[@]}"; do + for importdir in "$@"; do if [[ -f "${importdir}/pubring.gpg" ]]; then if ! "${GPG_PACMAN[@]}" --quiet --batch --import "${importdir}/pubring.gpg" ; then error "$(gettext "%s could not be imported.")" "${importdir}/pubring.gpg" @@ -397,7 +412,7 @@ import() { list_keys() { check_keyids_exist - if ! "${GPG_PACMAN[@]}" --batch --list-keys "${KEYIDS[@]}" ; then + if ! "${GPG_PACMAN[@]}" --batch --list-keys "$@" ; then error "$(gettext "A specified key could not be listed.")" exit 1 fi @@ -405,7 +420,7 @@ list_keys() { list_sigs() { check_keyids_exist - if ! "${GPG_PACMAN[@]}" --batch --list-sigs "${KEYIDS[@]}" ; then + if ! "${GPG_PACMAN[@]}" --batch --list-sigs "$@" ; then error "$(gettext "A specified signature could not be listed.")" exit 1 fi @@ -413,7 +428,7 @@ list_sigs() { lsign_keys() { check_keyids_exist - printf 'y\ny\n' | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet --batch --lsign-key "${KEYIDS[@]}" 2>/dev/null + printf '%s\n' y y | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet --batch --lsign-key "$@" 2>/dev/null if (( PIPESTATUS[1] )); then error "$(gettext "A specified key could not be locally signed.")" exit 1 @@ -421,23 +436,27 @@ lsign_keys() { } receive_keys() { - if ! "${GPG_PACMAN[@]}" --recv-keys "${KEYIDS[@]}" ; then + if (( $# == 0 )); then + error "$(gettext "No keys specified")" + exit 1 + fi + if ! "${GPG_PACMAN[@]}" --recv-keys "$@" ; then error "$(gettext "Remote key not fetched correctly from keyserver.")" exit 1 fi } refresh_keys() { - check_keyids_exist - if ! "${GPG_PACMAN[@]}" --refresh-keys "${KEYIDS[@]}" ; then + check_keyids_exist "$@" + if ! "${GPG_PACMAN[@]}" --refresh-keys "$@" ; then error "$(gettext "A specified local key could not be updated from a keyserver.")" exit 1 fi } verify_sig() { - if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify $SIGNATURE | grep -qE 'TRUST_(FULLY|ULTIMATE)'; then - error "$(gettext "The signature identified by %s could not be verified.")" "$SIGNATURE" + if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$1" | grep -qE 'TRUST_(FULLY|ULTIMATE)'; then + error "$(gettext "The signature identified by %s could not be verified.")" "$1" exit 1 fi } @@ -457,48 +476,48 @@ if ! type gettext &>/dev/null; then } fi -OPT_SHORT="a::d:e::f::hl::r:uv:V" -OPT_LONG="add::,config:,delete:,edit-key:,export::,finger::,gpgdir:" -OPT_LONG+=",help,import:,import-trustdb:,init,keyserver:,list-keys::,list-sigs::" -OPT_LONG+=",lsign-key:,populate::,recv-keys:,refresh-keys::,updatedb" -OPT_LONG+=",verify:,version" -if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then +OPT_SHORT="adefhlruvV" +OPT_LONG="add,config:,delete,edit-key,export,finger,gpgdir:" +OPT_LONG+=",help,import,import-trustdb,init,keyserver:,list-keys,list-sigs" +OPT_LONG+=",lsign-key,populate,recv-keys,refresh-keys,updatedb" +OPT_LONG+=",verify,version" +if ! parseopts "$OPT_SHORT" "$OPT_LONG" "$@"; then echo; usage; exit 1 # E_INVALID_OPTION; fi -eval set -- "$OPT_TEMP" -unset OPT_SHORT OPT_LONG OPT_TEMP +set -- "${OPTRET[@]}" +unset OPT_SHORT OPT_LONG OPTRET if [[ $1 == "--" ]]; then usage; exit 0; fi -while true; do - case "$1" in - -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1); UPDATEDB=1 ;; +while (( $# )); do + case $1 in + -a|--add) ADD=1 UPDATEDB=1 ;; --config) shift; CONFIG=$1 ;; - -d|--delete) DELETE=1; shift; KEYIDS=($1); UPDATEDB=1 ;; - --edit-key) EDITKEY=1; shift; KEYIDS=($1); UPDATEDB=1 ;; - -e|--export) EXPORT=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; - -f|--finger) FINGER=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; + -d|--delete) DELETE=1 UPDATEDB=1 ;; + --edit-key) EDITKEY=1 UPDATEDB=1 ;; + -e|--export) EXPORT=1 ;; + -f|--finger) FINGER=1 ;; --gpgdir) shift; PACMAN_KEYRING_DIR=$1 ;; - --import) IMPORT=1; shift; IMPORT_DIRS=($1); UPDATEDB=1 ;; - --import-trustdb) IMPORT_TRUSTDB=1; shift; IMPORT_DIRS=($1); UPDATEDB=1 ;; + --import) IMPORT=1 UPDATEDB=1 ;; + --import-trustdb) IMPORT_TRUSTDB=1 UPDATEDB=1 ;; --init) INIT=1 ;; --keyserver) shift; KEYSERVER=$1 ;; - -l|--list-keys) LISTKEYS=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; - --list-sigs) LISTSIGS=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; - --lsign-key) LSIGNKEY=1; shift; KEYIDS=($1); UPDATEDB=1 ;; - --populate) POPULATE=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYRINGIDS=($1); UPDATEDB=1 ;; - -r|--recv-keys) RECEIVE=1; shift; KEYIDS=($1); UPDATEDB=1 ;; - --refresh-keys) REFRESH=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; + -l|--list-keys) LISTKEYS=1 ;; + --list-sigs) LISTSIGS=1 ;; + --lsign-key) LSIGNKEY=1 UPDATEDB=1 ;; + --populate) POPULATE=1 UPDATEDB=1 ;; + -r|--recv-keys) RECEIVE=1 UPDATEDB=1 ;; + --refresh-keys) REFRESH=1 ;; -u|--updatedb) UPDATEDB=1 ;; - -v|--verify) VERIFY=1; shift; SIGNATURE=$1 ;; + -v|--verify) VERIFY=1 ;; -h|--help) usage; exit 0 ;; -V|--version) version; exit 0 ;; - --) OPT_IND=0; shift; break;; + --) shift; break 2 ;; *) usage; exit 1 ;; esac shift @@ -506,7 +525,7 @@ done if ! type -p gpg >/dev/null; then - error "$(gettext "Cannot find the %s binary required for all %s operations.")" "gpg" "pacman-key" + error "$(gettext "Cannot find the %s binary required for all %s operations.")" "gpg" "pacman-key" exit 1 fi @@ -551,21 +570,21 @@ esac (( ! INIT )) && check_keyring -(( ADD )) && add_keys -(( DELETE )) && delete_keys -(( EDITKEY )) && edit_keys -(( EXPORT )) && export_keys -(( FINGER )) && finger_keys -(( IMPORT )) && import -(( IMPORT_TRUSTDB)) && import_trustdb +(( ADD )) && add_keys "$@" +(( DELETE )) && delete_keys "$@" +(( EDITKEY )) && edit_keys "$@" +(( EXPORT )) && export_keys "$@" +(( FINGER )) && finger_keys "$@" +(( IMPORT )) && import "$@" +(( IMPORT_TRUSTDB)) && import_trustdb "$@" (( INIT )) && initialize -(( LISTKEYS )) && list_keys -(( LISTSIGS )) && list_sigs -(( LSIGNKEY )) && lsign_keys -(( POPULATE )) && populate_keyring -(( RECEIVE )) && receive_keys -(( REFRESH )) && refresh_keys -(( VERIFY )) && verify_sig +(( LISTKEYS )) && list_keys "$@" +(( LISTSIGS )) && list_sigs "$@" +(( LSIGNKEY )) && lsign_keys "$@" +(( POPULATE )) && populate_keyring "$@" +(( RECEIVE )) && receive_keys "$@" +(( REFRESH )) && refresh_keys "$@" +(( VERIFY )) && verify_sig "$@" (( UPDATEDB )) && updatedb -- 1.7.10
On 13/04/12 00:54, Dave Reisner wrote:
This requires an ugly amount of reworking of how pacman-key handles options. The change simply to avoid passing keys, files, and directories as arguments to options, but to leave them as arguments to the overall program. This is reasonable since pacman-key limits the user to essentially one operation per invocation (like pacman).
Since we now pass around the positional parameters to the various operations, we can add some better sanity checking. Each operation is responsible for testing input and making sure it can operate properly, otherwise it throws an error and exits.
The doc is updated to reflect this, and uses similar verbiage as pacman, describing the non-option arguments now passed to pacman-key as targets.
Similar to the doc, --help is reorganized to separate operations and options and remove argument tokens from operations.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- I really hate this patch, but I don't think it makes sense to split it.
Agreed. One patch is fine. <snip>
+ if (( $# == 0 )); then + error "$(gettext "No keys specified")" + exit 1 + fi
This is repeated so many times... How about doing a single check below the check that only one operation is specified? <snip>
@@ -413,7 +428,7 @@ list_sigs() {
lsign_keys() { check_keyids_exist - printf 'y\ny\n' | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet --batch --lsign-key "${KEYIDS[@]}" 2>/dev/null + printf '%s\n' y y | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet --batch --lsign-key "$@" 2>/dev/null if (( PIPESTATUS[1] )); then error "$(gettext "A specified key could not be locally signed.")" exit 1
Is there a good reason beyond the cosmetic for the printf change? Because I think it fails if it is only cosmetic... Allan
On Thu, Apr 12, 2012 at 11:50 PM, Allan McRae <allan@archlinux.org> wrote:
On 13/04/12 00:54, Dave Reisner wrote:
This requires an ugly amount of reworking of how pacman-key handles options. The change simply to avoid passing keys, files, and directories as arguments to options, but to leave them as arguments to the overall program. This is reasonable since pacman-key limits the user to essentially one operation per invocation (like pacman).
Since we now pass around the positional parameters to the various operations, we can add some better sanity checking. Each operation is responsible for testing input and making sure it can operate properly, otherwise it throws an error and exits.
The doc is updated to reflect this, and uses similar verbiage as pacman, describing the non-option arguments now passed to pacman-key as targets.
Similar to the doc, --help is reorganized to separate operations and options and remove argument tokens from operations.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- I really hate this patch, but I don't think it makes sense to split it.
Agreed. One patch is fine.
<snip>
+ if (( $# == 0 )); then + error "$(gettext "No keys specified")" + exit 1 + fi
This is repeated so many times... How about doing a single check below the check that only one operation is specified?
Because that would break any option that really can accept 0 arguments, such as --list-keys or --refresh-keys.
<snip>
@@ -413,7 +428,7 @@ list_sigs() {
lsign_keys() { check_keyids_exist - printf 'y\ny\n' | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet --batch --lsign-key "${KEYIDS[@]}" 2>/dev/null + printf '%s\n' y y | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet --batch --lsign-key "$@" 2>/dev/null if (( PIPESTATUS[1] )); then error "$(gettext "A specified key could not be locally signed.")" exit 1
Is there a good reason beyond the cosmetic for the printf change? Because I think it fails if it is only cosmetic...
It's only cosmetic, but I don't see how it fails... Anyways, it shouldn't be in the patch, so it's gone from my tree.
Allan
On 13/04/12 22:31, Dave Reisner wrote:
On Thu, Apr 12, 2012 at 11:50 PM, Allan McRae <allan@archlinux.org> wrote:
On 13/04/12 00:54, Dave Reisner wrote:
This requires an ugly amount of reworking of how pacman-key handles options. The change simply to avoid passing keys, files, and directories as arguments to options, but to leave them as arguments to the overall program. This is reasonable since pacman-key limits the user to essentially one operation per invocation (like pacman).
Since we now pass around the positional parameters to the various operations, we can add some better sanity checking. Each operation is responsible for testing input and making sure it can operate properly, otherwise it throws an error and exits.
The doc is updated to reflect this, and uses similar verbiage as pacman, describing the non-option arguments now passed to pacman-key as targets.
Similar to the doc, --help is reorganized to separate operations and options and remove argument tokens from operations.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- I really hate this patch, but I don't think it makes sense to split it.
Agreed. One patch is fine.
<snip>
+ if (( $# == 0 )); then + error "$(gettext "No keys specified")" + exit 1 + fi
This is repeated so many times... How about doing a single check below the check that only one operation is specified?
Because that would break any option that really can accept 0 arguments, such as --list-keys or --refresh-keys.
Yeah. I meant something like: if (( (ADD || DELETE || EDITKEY || ....) && $# == 0 )); then
On Fri, Apr 13, 2012 at 8:39 AM, Allan McRae <allan@archlinux.org> wrote:
On Thu, Apr 12, 2012 at 11:50 PM, Allan McRae <allan@archlinux.org> wrote:
On 13/04/12 00:54, Dave Reisner wrote:
This requires an ugly amount of reworking of how pacman-key handles options. The change simply to avoid passing keys, files, and
as arguments to options, but to leave them as arguments to the overall program. This is reasonable since pacman-key limits the user to essentially one operation per invocation (like pacman).
Since we now pass around the positional parameters to the various operations, we can add some better sanity checking. Each operation is responsible for testing input and making sure it can operate properly, otherwise it throws an error and exits.
The doc is updated to reflect this, and uses similar verbiage as
On 13/04/12 22:31, Dave Reisner wrote: directories pacman,
describing the non-option arguments now passed to pacman-key as targets.
Similar to the doc, --help is reorganized to separate operations and options and remove argument tokens from operations.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- I really hate this patch, but I don't think it makes sense to split it.
Agreed. One patch is fine.
<snip>
+ if (( $# == 0 )); then + error "$(gettext "No keys specified")" + exit 1 + fi
This is repeated so many times... How about doing a single check below the check that only one operation is specified?
Because that would break any option that really can accept 0 arguments, such as --list-keys or --refresh-keys.
Yeah. I meant something like:
if (( (ADD || DELETE || EDITKEY || ....) && $# == 0 )); then
Ah, right. Yep, I can do that... looking back, seems I missed this in a few places anyways (import, verify, etc)
This is retired, as the two consumers of this function are now using the new parseopts instead. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- scripts/Makefile.am | 3 +- scripts/library/README | 6 --- scripts/library/parse_options.sh | 105 -------------------------------------- scripts/po/POTFILES.in | 1 - 4 files changed, 1 insertion(+), 114 deletions(-) delete mode 100644 scripts/library/parse_options.sh diff --git a/scripts/Makefile.am b/scripts/Makefile.am index fc70732..b8a1990 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -27,8 +27,7 @@ EXTRA_DIST = \ LIBRARY = \ library/output_format.sh \ - library/parseopts.sh \ - library/parse_options.sh + library/parseopts.sh # Files that should be removed, but which Automake does not know. MOSTLYCLEANFILES = $(bin_SCRIPTS) diff --git a/scripts/library/README b/scripts/library/README index 33f6321..53a0b2e 100644 --- a/scripts/library/README +++ b/scripts/library/README @@ -8,12 +8,6 @@ and can be silenced by defining 'QUIET'. The 'warning' and 'error' functions print to stderr with the appropriate prefix added to the message. -parse_options.sh: -A getopt replacement to avoids portability issues, in particular the -lack of long option name support in the default getopt provided by some -platforms. -Usage: parse_option $SHORT_OPTS $LONG_OPTS "$@" - parseopts.sh: A getopt_long-like parser which portably supports longopts and shortopts with some GNU extensions. It does not allow for options with optional diff --git a/scripts/library/parse_options.sh b/scripts/library/parse_options.sh deleted file mode 100644 index 039eef9..0000000 --- a/scripts/library/parse_options.sh +++ /dev/null @@ -1,105 +0,0 @@ -# getopt like parser -parse_options() { - local short_options=$1; shift; - local long_options=$1; shift; - local ret=0; - local unused_options="" - local i - - while [[ -n $1 ]]; do - if [[ ${1:0:2} = '--' ]]; then - if [[ -n ${1:2} ]]; then - local match="" - for i in ${long_options//,/ }; do - if [[ ${1:2} = ${i//:} ]]; then - match=$i - break - fi - done - if [[ -n $match ]]; then - local needsargument=0 - - [[ ${match} = ${1:2}: ]] && needsargument=1 - [[ ${match} = ${1:2}:: && -n $2 && ${2:0:1} != "-" ]] && needsargument=1 - - if (( ! needsargument )); then - printf ' %s' "$1" - else - if [[ -n $2 ]]; then - printf ' %s ' "$1" - shift - printf "'%q" "$1" - while [[ -n $2 && ${2:0:1} != "-" ]]; do - shift - printf " %q" "$1" - done - printf "'" - else - printf "@SCRIPTNAME@: $(gettext "option %s requires an argument\n")" "'$1'" >&2 - ret=1 - fi - fi - else - echo "@SCRIPTNAME@: $(gettext "unrecognized option") '$1'" >&2 - ret=1 - fi - else - shift - break - fi - elif [[ ${1:0:1} = '-' ]]; then - for ((i=1; i<${#1}; i++)); do - if [[ $short_options =~ ${1:i:1} ]]; then - local needsargument=0 - - [[ $short_options =~ ${1:i:1}: && ! $short_options =~ ${1:i:1}:: ]] && needsargument=1 - [[ $short_options =~ ${1:i:1}:: && \ - ( -n ${1:$i+1} || ( -n $2 && ${2:0:1} != "-" ) ) ]] && needsargument=1 - - if (( ! needsargument )); then - printf ' -%s' "${1:i:1}" - else - if [[ -n ${1:$i+1} ]]; then - printf ' -%s ' "${1:i:1}" - printf "'%q" "${1:$i+1}" - while [[ -n $2 && ${2:0:1} != "-" ]]; do - shift - printf " %q" "$1" - done - printf "'" - else - if [[ -n $2 ]]; then - printf ' -%s ' "${1:i:1}" - shift - printf "'%q" "$1" - while [[ -n $2 && ${2:0:1} != "-" ]]; do - shift - printf " %q" "$1" - done - printf "'" - - else - printf "@SCRIPTNAME@: $(gettext "option %s requires an argument\n")" "'-${1:i:1}'" >&2 - ret=1 - fi - fi - break - fi - else - echo "@SCRIPTNAME@: $(gettext "unrecognized option") '-${1:i:1}'" >&2 - ret=1 - fi - done - else - unused_options="${unused_options} '$1'" - fi - shift - done - - printf " --" - [[ $unused_options ]] && printf ' %s' "${unused_options[@]}" - [[ $1 ]] && printf " '%s'" "$@" - printf "\n" - - return $ret -} diff --git a/scripts/po/POTFILES.in b/scripts/po/POTFILES.in index 01cc235..162731b 100644 --- a/scripts/po/POTFILES.in +++ b/scripts/po/POTFILES.in @@ -8,5 +8,4 @@ scripts/pacman-optimize.sh.in scripts/pkgdelta.sh.in scripts/repo-add.sh.in scripts/library/output_format.sh -scripts/library/parse_options.sh scripts/library/parseopts.sh -- 1.7.10
Loop through arguments passed to verify_sig and treat each as a signature to be verified against a source file. Output each file as its checked to avoid ambiguity. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- doc/pacman-key.8.txt | 2 +- scripts/pacman-key.sh.in | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 96ac31c..4a2122f 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -96,7 +96,7 @@ Operations Displays the program version. *-v, \--verify*:: - Verify the given signature file. + Verify the given targets as signature files. Options ------- diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b2c3da9..2083a60 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -66,7 +66,7 @@ usage() { printf -- "$(gettext " -l, --list-keys List the specified or all keys")\n" printf -- "$(gettext " -r, --recv-keys Fetch the specified keyids")\n" printf -- "$(gettext " -u, --updatedb Update the trustdb of pacman")\n" - printf -- "$(gettext " -v, --verify Verify the file specified by the signature")\n" + printf -- "$(gettext " -v, --verify Verify the file(s) specified by the signature(s)")\n" printf -- "$(gettext " --edit-key Present a menu for key management task on keyids")\n" printf -- "$(gettext " --import Imports pubring.gpg from dir(s)")\n" printf -- "$(gettext " --import-trustdb Imports ownertrust values from trustdb.gpg in dir(s)")\n" @@ -455,10 +455,15 @@ refresh_keys() { } verify_sig() { - if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$1" | grep -qE 'TRUST_(FULLY|ULTIMATE)'; then - error "$(gettext "The signature identified by %s could not be verified.")" "$1" - exit 1 - fi + local ret=0 + for sig; do + msg "Checking %s ..." "$sig" + if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" | grep -qE 'TRUST_(FULLY|ULTIMATE)'; then + error "$(gettext "The signature identified by %s could not be verified.")" "$sig" + ret=1 + fi + done + exit $ret } updatedb() { -- 1.7.10
On 13/04/12 00:54, Dave Reisner wrote:
Loop through arguments passed to verify_sig and treat each as a signature to be verified against a source file. Output each file as its checked to avoid ambiguity.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- doc/pacman-key.8.txt | 2 +- scripts/pacman-key.sh.in | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 96ac31c..4a2122f 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -96,7 +96,7 @@ Operations Displays the program version.
*-v, \--verify*:: - Verify the given signature file. + Verify the given targets as signature files.
Not sure I like this wording... How about sticking with the wording in --help "Verify the file(s) specified by the signature(s)".
Options ------- diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b2c3da9..2083a60 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -66,7 +66,7 @@ usage() { printf -- "$(gettext " -l, --list-keys List the specified or all keys")\n" printf -- "$(gettext " -r, --recv-keys Fetch the specified keyids")\n" printf -- "$(gettext " -u, --updatedb Update the trustdb of pacman")\n" - printf -- "$(gettext " -v, --verify Verify the file specified by the signature")\n" + printf -- "$(gettext " -v, --verify Verify the file(s) specified by the signature(s)")\n" printf -- "$(gettext " --edit-key Present a menu for key management task on keyids")\n" printf -- "$(gettext " --import Imports pubring.gpg from dir(s)")\n" printf -- "$(gettext " --import-trustdb Imports ownertrust values from trustdb.gpg in dir(s)")\n" @@ -455,10 +455,15 @@ refresh_keys() { }
verify_sig() { - if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$1" | grep -qE 'TRUST_(FULLY|ULTIMATE)'; then - error "$(gettext "The signature identified by %s could not be verified.")" "$1" - exit 1 - fi + local ret=0 + for sig; do + msg "Checking %s ..." "$sig" + if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" | grep -qE 'TRUST_(FULLY|ULTIMATE)'; then + error "$(gettext "The signature identified by %s could not be verified.")" "$sig" + ret=1 + fi + done + exit $ret }
updatedb() {
On Thu, Apr 12, 2012 at 11:25 PM, Allan McRae <allan@archlinux.org> wrote:
On 13/04/12 00:54, Dave Reisner wrote:
Loop through arguments passed to verify_sig and treat each as a signature to be verified against a source file. Output each file as its checked to avoid ambiguity.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- doc/pacman-key.8.txt | 2 +- scripts/pacman-key.sh.in | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 96ac31c..4a2122f 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -96,7 +96,7 @@ Operations Displays the program version.
*-v, \--verify*:: - Verify the given signature file. + Verify the given targets as signature files.
Not sure I like this wording... How about sticking with the wording in --help "Verify the file(s) specified by the signature(s)".
Agreed.
Options ------- diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b2c3da9..2083a60 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -66,7 +66,7 @@ usage() { printf -- "$(gettext " -l, --list-keys List the
specified or all keys")\n"
printf -- "$(gettext " -r, --recv-keys Fetch the
specified keyids")\n"
printf -- "$(gettext " -u, --updatedb Update the
- printf -- "$(gettext " -v, --verify Verify the file specified by the signature")\n" + printf -- "$(gettext " -v, --verify Verify the file(s) specified by the signature(s)")\n" printf -- "$(gettext " --edit-key Present a menu for key management task on keyids")\n" printf -- "$(gettext " --import Imports
trustdb of pacman")\n" pubring.gpg from dir(s)")\n"
printf -- "$(gettext " --import-trustdb Imports
ownertrust values from trustdb.gpg in dir(s)")\n"
@@ -455,10 +455,15 @@ refresh_keys() { }
verify_sig() { - if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$1" | grep -qE 'TRUST_(FULLY|ULTIMATE)'; then - error "$(gettext "The signature identified by %s could not be verified.")" "$1" - exit 1 - fi + local ret=0 + for sig; do + msg "Checking %s ..." "$sig" + if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" | grep -qE 'TRUST_(FULLY|ULTIMATE)'; then + error "$(gettext "The signature identified by %s could not be verified.")" "$sig" + ret=1 + fi + done + exit $ret }
updatedb() {
Pass $(OURSCRIPTS) through the bash parser in read only mode to validate syntax. Note that this doesn't actually catch all errors, but it might be useful for developers working on these scripts. Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/Makefile.am | 1 + scripts/Makefile.am | 1 + 2 files changed, 2 insertions(+) diff --git a/contrib/Makefile.am b/contrib/Makefile.am index eca39e7..2953912 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -55,6 +55,7 @@ $(OURSCRIPTS): Makefile $(AM_V_GEN)$(edit) $(srcdir)/$@.in >$@.tmp $(AM_V_at)chmod +x,a-w $@.tmp $(AM_V_at)mv $@.tmp $@ + @$(BASH_SHELL) -O extglob -n $@ $(OURFILES): Makefile $(AM_V_at)$(RM) $@ $@.tmp diff --git a/scripts/Makefile.am b/scripts/Makefile.am index b8a1990..dbe61b8 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -63,6 +63,7 @@ edit = sed \ $(OURSCRIPTS): Makefile $(AM_V_at)$(RM) $@ $(AM_V_GEN)test -f $(srcdir)/$@.sh.in && m4 -P -I $(srcdir) $(srcdir)/$@.sh.in | $(edit) >$@ + @$(BASH_SHELL) -O extglob -n $@ $(AM_V_at)chmod +x,a-w $@ makepkg: \ -- 1.7.10
On 13/04/12 00:54, Dave Reisner wrote:
Pass $(OURSCRIPTS) through the bash parser in read only mode to validate syntax. Note that this doesn't actually catch all errors, but it might be useful for developers working on these scripts.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/Makefile.am | 1 + scripts/Makefile.am | 1 + 2 files changed, 2 insertions(+)
diff --git a/contrib/Makefile.am b/contrib/Makefile.am index eca39e7..2953912 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -55,6 +55,7 @@ $(OURSCRIPTS): Makefile $(AM_V_GEN)$(edit) $(srcdir)/$@.in >$@.tmp $(AM_V_at)chmod +x,a-w $@.tmp $(AM_V_at)mv $@.tmp $@ + @$(BASH_SHELL) -O extglob -n $@
I'm missing why we need extglob here?
$(OURFILES): Makefile $(AM_V_at)$(RM) $@ $@.tmp diff --git a/scripts/Makefile.am b/scripts/Makefile.am index b8a1990..dbe61b8 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -63,6 +63,7 @@ edit = sed \ $(OURSCRIPTS): Makefile $(AM_V_at)$(RM) $@ $(AM_V_GEN)test -f $(srcdir)/$@.sh.in && m4 -P -I $(srcdir) $(srcdir)/$@.sh.in | $(edit) >$@ + @$(BASH_SHELL) -O extglob -n $@ $(AM_V_at)chmod +x,a-w $@
makepkg: \
On 13.04.2012 05:30, Allan McRae wrote:
On 13/04/12 00:54, Dave Reisner wrote:
Pass $(OURSCRIPTS) through the bash parser in read only mode to validate syntax. Note that this doesn't actually catch all errors, but it might be useful for developers working on these scripts.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/Makefile.am | 1 + scripts/Makefile.am | 1 + 2 files changed, 2 insertions(+)
diff --git a/contrib/Makefile.am b/contrib/Makefile.am index eca39e7..2953912 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -55,6 +55,7 @@ $(OURSCRIPTS): Makefile $(AM_V_GEN)$(edit) $(srcdir)/$@.in >$@.tmp $(AM_V_at)chmod +x,a-w $@.tmp $(AM_V_at)mv $@.tmp $@ + @$(BASH_SHELL) -O extglob -n $@
I'm missing why we need extglob here?
bacman and paccache use extglobs -- Florian Pritz
On 13/04/12 20:49, Florian Pritz wrote:
On 13.04.2012 05:30, Allan McRae wrote:
On 13/04/12 00:54, Dave Reisner wrote:
Pass $(OURSCRIPTS) through the bash parser in read only mode to validate syntax. Note that this doesn't actually catch all errors, but it might be useful for developers working on these scripts.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/Makefile.am | 1 + scripts/Makefile.am | 1 + 2 files changed, 2 insertions(+)
diff --git a/contrib/Makefile.am b/contrib/Makefile.am index eca39e7..2953912 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -55,6 +55,7 @@ $(OURSCRIPTS): Makefile $(AM_V_GEN)$(edit) $(srcdir)/$@.in >$@.tmp $(AM_V_at)chmod +x,a-w $@.tmp $(AM_V_at)mv $@.tmp $@ + @$(BASH_SHELL) -O extglob -n $@
I'm missing why we need extglob here?
bacman and paccache use extglobs
Yes... and every time someone calls one of those scripts they have to manually set the extglob? No... because that would be stupid.
On Fri, Apr 13, 2012 at 6:57 AM, Allan McRae <allan@archlinux.org> wrote:
On 13/04/12 20:49, Florian Pritz wrote:
On 13.04.2012 05:30, Allan McRae wrote:
On 13/04/12 00:54, Dave Reisner wrote:
Pass $(OURSCRIPTS) through the bash parser in read only mode to validate syntax. Note that this doesn't actually catch all errors, but it might be useful for developers working on these scripts.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/Makefile.am | 1 + scripts/Makefile.am | 1 + 2 files changed, 2 insertions(+)
diff --git a/contrib/Makefile.am b/contrib/Makefile.am index eca39e7..2953912 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -55,6 +55,7 @@ $(OURSCRIPTS): Makefile $(AM_V_GEN)$(edit) $(srcdir)/$@.in >$@.tmp $(AM_V_at)chmod +x,a-w $@.tmp $(AM_V_at)mv $@.tmp $@ + @$(BASH_SHELL) -O extglob -n $@
I'm missing why we need extglob here?
bacman and paccache use extglobs
Yes... and every time someone calls one of those scripts they have to manually set the extglob? No... because that would be stupid.
It would be stupid, but this is bash... So let's ignore rational thought for a minute. $ bash -n ./scripts/repo-add ./scripts/repo-add: line 261: syntax error near unexpected token `(' ./scripts/repo-add: line 261: ` *.@(db|files).tar.gz) TAR_OPT="z" ;;' no-exec mode really does mean no-exec, including shopts that might change the behavior of the parser.
On 13/04/12 22:57, Dave Reisner wrote:
On Fri, Apr 13, 2012 at 6:57 AM, Allan McRae <allan@archlinux.org> wrote:
On 13/04/12 20:49, Florian Pritz wrote:
On 13.04.2012 05:30, Allan McRae wrote:
On 13/04/12 00:54, Dave Reisner wrote:
Pass $(OURSCRIPTS) through the bash parser in read only mode to validate syntax. Note that this doesn't actually catch all errors, but it might be useful for developers working on these scripts.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> --- contrib/Makefile.am | 1 + scripts/Makefile.am | 1 + 2 files changed, 2 insertions(+)
diff --git a/contrib/Makefile.am b/contrib/Makefile.am index eca39e7..2953912 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -55,6 +55,7 @@ $(OURSCRIPTS): Makefile $(AM_V_GEN)$(edit) $(srcdir)/$@.in >$@.tmp $(AM_V_at)chmod +x,a-w $@.tmp $(AM_V_at)mv $@.tmp $@ + @$(BASH_SHELL) -O extglob -n $@
I'm missing why we need extglob here?
bacman and paccache use extglobs
Yes... and every time someone calls one of those scripts they have to manually set the extglob? No... because that would be stupid.
It would be stupid, but this is bash... So let's ignore rational thought for a minute.
$ bash -n ./scripts/repo-add ./scripts/repo-add: line 261: syntax error near unexpected token `(' ./scripts/repo-add: line 261: ` *.@(db|files).tar.gz) TAR_OPT="z" ;;'
no-exec mode really does mean no-exec, including shopts that might change the behavior of the parser.
Ok then. Bash does something I don't expect yet again! One thing that will do is cause any script where extglob is needed but not enabled to "pass". But I think fixing that would be a pain and also really is not necessary so ignore...
--- This was on the top of my parse-opts branch. Figured I'd send it along to see if anyone else cares about this sort of thing. Easily nuked... Since we're using associative arrays in pacman-key, 4.0 is a requirement. However, 4.0 is also full of bugs, so Allan and I agreed that 4.1 would be a better minimum requirement to avoid unresolvable bug reports. configure.ac | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index a6114f8..b325fbb 100644 --- a/configure.ac +++ b/configure.ac @@ -148,7 +148,15 @@ AC_PROG_AWK AC_PROG_CC_C99 AC_PROG_INSTALL AC_CHECK_PROGS([PYTHON], [python2.7 python2.6 python2.5 python2 python], [false]) -AC_PATH_PROGS([BASH_SHELL], [bash bash4 bash3], [false]) +AC_PATH_PROGS([BASH_SHELL], [bash bash4], [false]) + +AS_IF([test "x$BASH_SHELL" = "xfalse"], + AC_MSG_WARN([*** bash >= 4.1.0 is required for pacman scripts]), + [bash_version_majmin=`$BASH_SHELL -c 'echo "${BASH_VERSINFO[[0]]}${BASH_VERSINFO[[1]]}"'` + if test "$bash_version_majmin" -lt 41; then + AC_MSG_ERROR([*** bash >= 4.1.0 is required for pacman scripts]) + fi + unset bash_version_majmin]) # find installed gettext AM_GNU_GETTEXT([external], [need-ngettext]) -- 1.7.10
On 13/04/12 00:54, Dave Reisner wrote:
--- This was on the top of my parse-opts branch. Figured I'd send it along to see if anyone else cares about this sort of thing. Easily nuked...
Since we're using associative arrays in pacman-key, 4.0 is a requirement. However, 4.0 is also full of bugs, so Allan and I agreed that 4.1 would be a better minimum requirement to avoid unresolvable bug reports.
I agree that we agreed that 4.1 is the way to go. Bash-4.1 was released in Jan 2010.
configure.ac | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac index a6114f8..b325fbb 100644 --- a/configure.ac +++ b/configure.ac @@ -148,7 +148,15 @@ AC_PROG_AWK AC_PROG_CC_C99 AC_PROG_INSTALL AC_CHECK_PROGS([PYTHON], [python2.7 python2.6 python2.5 python2 python], [false]) -AC_PATH_PROGS([BASH_SHELL], [bash bash4 bash3], [false]) +AC_PATH_PROGS([BASH_SHELL], [bash bash4], [false]) + +AS_IF([test "x$BASH_SHELL" = "xfalse"], + AC_MSG_WARN([*** bash >= 4.1.0 is required for pacman scripts]), + [bash_version_majmin=`$BASH_SHELL -c 'echo "${BASH_VERSINFO[[0]]}${BASH_VERSINFO[[1]]}"'` + if test "$bash_version_majmin" -lt 41; then + AC_MSG_ERROR([*** bash >= 4.1.0 is required for pacman scripts]) + fi + unset bash_version_majmin])
# find installed gettext AM_GNU_GETTEXT([external], [need-ngettext])
On 13/04/12 00:54, Dave Reisner wrote:
This will replace our current options parser used in pacman-key and makepkg. It follows heuristics closer to that of GNU getopt long (and thus pacman itself), with the exception that it does not allow for options with optional arguments. Due to the way this parser will be used, this sort of functionality will not be needed.
Instead of relying on eval+set, options are normalized into an array, OPTRET, which callers should expect to be populated after returning from parseopts. This avoids problems with quotes and spaces in arguments, assuming that the user quotes properly when passing into the application.
A new test harness for parseopts is added in test/scripts.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> ---
<snip>
+ + get_argreq() { + local o re="^($1)(:?:?)$"
You only want one :? in the regex.
+ for o in "${longopts[@]}"; do + # found option, return number of colons (0 = none, 1 = required) + [[ $o =~ $re ]] && return ${#BASH_REMATCH[2]} + done + # failure + return 255 + }
<snip>
+test_result() { + local result=$1 tokencount=$2 input=$3; shift 3 + + if { [[ $result = "$*" ]] || [[ $2 = NULL && -z $1 ]]; } && (( tokencount == $# )); then
What is the "|| [[ $2 = NULL && -z $1 ]];" part for? I can guess, but it seems unneeded. With the minor changes I pointed out in the last few emails, I ack this patchset. My biggest complaint is I prefer the name "parse_options" over "parseopts"! Allan
On Fri, Apr 13, 2012 at 12:46 AM, Allan McRae <allan@archlinux.org> wrote:
On 13/04/12 00:54, Dave Reisner wrote:
This will replace our current options parser used in pacman-key and makepkg. It follows heuristics closer to that of GNU getopt long (and thus pacman itself), with the exception that it does not allow for options with optional arguments. Due to the way this parser will be used, this sort of functionality will not be needed.
Instead of relying on eval+set, options are normalized into an array, OPTRET, which callers should expect to be populated after returning from parseopts. This avoids problems with quotes and spaces in arguments, assuming that the user quotes properly when passing into the application.
A new test harness for parseopts is added in test/scripts.
Signed-off-by: Dave Reisner <dreisner@archlinux.org> ---
<snip>
+ + get_argreq() { + local o re="^($1)(:?:?)$"
You only want one :? in the regex.
Ah yes.. a vestigial from when I was toying with optional arguments.
+ for o in "${longopts[@]}"; do + # found option, return number of colons (0 = none, 1 = required) + [[ $o =~ $re ]] && return ${#BASH_REMATCH[2]} + done + # failure + return 255 + }
<snip>
+test_result() { + local result=$1 tokencount=$2 input=$3; shift 3 + + if { [[ $result = "$*" ]] || [[ $2 = NULL && -z $1 ]]; } && (( tokencount == $# )); then
What is the "|| [[ $2 = NULL && -z $1 ]];" part for? I can guess, but it seems unneeded.
I have no idea what it's for. This test toy (and some of the parseopts code) dates back to July-ish of last year. Removing the entire middle check seems innocuous, so its gone.
With the minor changes I pointed out in the last few emails, I ack this patchset. My biggest complaint is I prefer the name "parse_options" over "parseopts"!
Allan
participants (4)
-
Allan McRae
-
Dave Reisner
-
Dave Reisner
-
Florian Pritz