[pacman-dev] [PATCH v2] makepkg: Move parseopts from library to libmakepkg
Signed-off-by: Alad Wenter <alad@archlinux.info> --- v2: Add missing signoff. scripts/libmakepkg/util/option.sh | 157 ++++++++++++++++++++++++++++++++++++++ scripts/library/README | 20 ----- scripts/library/parseopts.sh | 137 --------------------------------- scripts/makepkg.sh.in | 2 - 4 files changed, 157 insertions(+), 159 deletions(-) delete mode 100644 scripts/library/parseopts.sh diff --git a/scripts/libmakepkg/util/option.sh b/scripts/libmakepkg/util/option.sh index 54ba474..4d5fd78 100644 --- a/scripts/libmakepkg/util/option.sh +++ b/scripts/libmakepkg/util/option.sh @@ -140,3 +140,160 @@ check_buildoption() { # not found return 127 } + + +# 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. After the +# first argument containing the short opts, any number of valid long +# opts may be be passed. The end of the options delimiter must then be +# added, followed by the user arguments to the calling program. +# +# Recommended Usage: +# OPT_SHORT='fb:z' +# OPT_LONG=('foo' 'bar:' 'baz') +# if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then +# exit 1 +# fi +# set -- "${OPTRET[@]}" +# Returns: +# 0: parse success +# 1: parse failure (error message supplied) +parseopts() { + local opt= optarg= i= shortopts=$1 + local -a longopts=() unused_argv=() + + shift + while [[ $1 && $1 != '--' ]]; do + longopts+=("$1") + shift + done + shift + + longoptmatch() { + local o longmatch=() + for o in "${longopts[@]}"; do + if [[ ${o%:} = "$1" ]]; then + longmatch=("$o") + break + fi + [[ ${o%:} = "$1"* ]] && longmatch+=("$o") + done + + case ${#longmatch[*]} in + 1) + # success, override with opt and return arg req (0 == none, 1 == required) + opt=${longmatch%:} + if [[ $longmatch = *: ]]; then + return 1 + else + return 0 + fi ;; + 0) + # fail, no match found + return 255 ;; + *) + # fail, ambiguous match + printf "@SCRIPTNAME@: $(gettext "option '%s' is ambiguous; possibilities:")" "--$1" + printf " '%s'" "${longmatch[@]%:}" + printf '\n' + return 254 ;; + esac >&2 + } + + 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 + printf "@SCRIPTNAME@: $(gettext "invalid option") -- '%s'\n" "$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@: $(gettext "option requires an argument") -- '%s'\n" "$opt" >&2 + OPTRET=(--) + return 1 + fi + fi + done + ;; + --?*=*|--?*) # long option + IFS='=' read -r opt optarg <<< "${1#--}" + longoptmatch "$opt" + case $? in + 0) + # parse failure + if [[ $optarg ]]; then + printf "@SCRIPTNAME@: $(gettext "option '%s' does not allow an argument")\n" "--$opt" >&2 + OPTRET=(--) + return 1 + # --longopt + else + OPTRET+=("--$opt") + fi + ;; + 1) + # --longopt=optarg + if [[ $optarg ]]; then + OPTRET+=("--$opt" "$optarg") + # --longopt optarg + elif [[ $2 ]]; then + OPTRET+=("--$opt" "$2" ) + shift + # parse failure + else + printf "@SCRIPTNAME@: $(gettext "option '%s' requires an argument")\n" "--$opt" >&2 + OPTRET=(--) + return 1 + fi + ;; + 254) + # ambiguous option -- error was reported for us by longoptmatch() + OPTRET=(--) + return 1 + ;; + 255) + # parse failure + printf "@SCRIPTNAME@: $(gettext "invalid option") '--%s'\n" "$opt" >&2 + OPTRET=(--) + return 1 + ;; + 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[@]}" "$@") + unset longoptmatch + + return 0 +} diff --git a/scripts/library/README b/scripts/library/README index e9615a2..a9d15f1 100644 --- a/scripts/library/README +++ b/scripts/library/README @@ -8,26 +8,6 @@ stdout and can be silenced by defining 'QUIET'. The 'warning' and 'error' functions print to stderr with the appropriate prefix added to the message. -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. After the first argument containing -the short opts, any number of valid long opts may be be passed. The end -of the options delimiter must then be added, followed by the user arguments -to the calling program. - -Recommended Usage: - OPT_SHORT='fb:z' - OPT_LONG=('foo' 'bar:' 'baz') - if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then - exit 1 - fi - set -- "${OPTRET[@]}" -Returns: - 0: parse success - 1: parse failure (error message supplied) - human_to_size.sh: A function to convert human readable sizes (such as "5.3 GiB") to raw byte equivalents. base10 and base2 suffixes are supported, case sensitively. If diff --git a/scripts/library/parseopts.sh b/scripts/library/parseopts.sh deleted file mode 100644 index cf6aa6c..0000000 --- a/scripts/library/parseopts.sh +++ /dev/null @@ -1,137 +0,0 @@ -# getopt-like parser -parseopts() { - local opt= optarg= i= shortopts=$1 - local -a longopts=() unused_argv=() - - shift - while [[ $1 && $1 != '--' ]]; do - longopts+=("$1") - shift - done - shift - - longoptmatch() { - local o longmatch=() - for o in "${longopts[@]}"; do - if [[ ${o%:} = "$1" ]]; then - longmatch=("$o") - break - fi - [[ ${o%:} = "$1"* ]] && longmatch+=("$o") - done - - case ${#longmatch[*]} in - 1) - # success, override with opt and return arg req (0 == none, 1 == required) - opt=${longmatch%:} - if [[ $longmatch = *: ]]; then - return 1 - else - return 0 - fi ;; - 0) - # fail, no match found - return 255 ;; - *) - # fail, ambiguous match - printf "@SCRIPTNAME@: $(gettext "option '%s' is ambiguous; possibilities:")" "--$1" - printf " '%s'" "${longmatch[@]%:}" - printf '\n' - return 254 ;; - esac >&2 - } - - 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 - printf "@SCRIPTNAME@: $(gettext "invalid option") -- '%s'\n" "$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@: $(gettext "option requires an argument") -- '%s'\n" "$opt" >&2 - OPTRET=(--) - return 1 - fi - fi - done - ;; - --?*=*|--?*) # long option - IFS='=' read -r opt optarg <<< "${1#--}" - longoptmatch "$opt" - case $? in - 0) - # parse failure - if [[ $optarg ]]; then - printf "@SCRIPTNAME@: $(gettext "option '%s' does not allow an argument")\n" "--$opt" >&2 - OPTRET=(--) - return 1 - # --longopt - else - OPTRET+=("--$opt") - fi - ;; - 1) - # --longopt=optarg - if [[ $optarg ]]; then - OPTRET+=("--$opt" "$optarg") - # --longopt optarg - elif [[ $2 ]]; then - OPTRET+=("--$opt" "$2" ) - shift - # parse failure - else - printf "@SCRIPTNAME@: $(gettext "option '%s' requires an argument")\n" "--$opt" >&2 - OPTRET=(--) - return 1 - fi - ;; - 254) - # ambiguous option -- error was reported for us by longoptmatch() - OPTRET=(--) - return 1 - ;; - 255) - # parse failure - printf "@SCRIPTNAME@: $(gettext "invalid option") '--%s'\n" "$opt" >&2 - OPTRET=(--) - return 1 - ;; - 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[@]}" "$@") - unset longoptmatch - - return 0 -} diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 7b2ce51..b3cafa8 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1153,8 +1153,6 @@ run_split_packaging() { pkgname=("${pkgname_backup[@]}") } -m4_include(library/parseopts.sh) - usage() { printf "makepkg (pacman) %s\n" "$makepkg_version" echo -- 2.10.0
2016-10-08 7:18 GMT+02:00 Alad Wenter <alad@archlinux.info>:
Signed-off-by: Alad Wenter <alad@archlinux.info> --- v2: Add missing signoff.
scripts/libmakepkg/util/option.sh | 157 ++++++++++++++++++++++++++++++ ++++++++ scripts/library/README | 20 ----- scripts/library/parseopts.sh | 137 --------------------------------- scripts/makepkg.sh.in | 2 - 4 files changed, 157 insertions(+), 159 deletions(-) delete mode 100644 scripts/library/parseopts.sh
parseopts.sh is used in a couple of other scripts, is referenced in a couple of makefiles, and even has test cases. Each of these would need to be changed. See below. Also, IMO, for a change like this, a short motivation should go into the commit message (one sentence may be enough). Moving parseopts to libmakepkg would make a couple of "pacman"-scripts depend on libmakepkg (instead of library), which feels wrong, but feel free to ignore me, I'm not a pacman dev. :) [~/code/pacman]./autogen.sh && ./configure && make ... make[3]: *** No rule to make target 'library/parseopts.sh', needed by 'makepkg'. Stop. make[2]: *** [Makefile:623: all-recursive] Error 1 make[1]: *** [Makefile:969: all-recursive] Error 1 git grep parseopts\.sh -- '*' ':!scripts/po/*' contrib/Makefile.am:paccache: $(srcdir)/paccache.sh.in $(top_srcdir)/scripts/library/parseopts.sh $(top_srcdir)/scripts/library/size_to_human.sh contrib/bacman.sh.in:m4_include(../scripts/library/parseopts.sh) contrib/paccache.sh.in:m4_include(../scripts/library/parseopts.sh) scripts/Makefile.am: library/parseopts.sh \ scripts/Makefile.am: $(srcdir)/library/parseopts.sh \ scripts/Makefile.am: $(srcdir)/library/parseopts.sh scripts/Makefile.am: $(srcdir)/library/parseopts.sh scripts/Makefile.am: $(srcdir)/library/parseopts.sh \ scripts/libmakepkg/util/option.sh:# parseopts.sh: scripts/pacman-db-upgrade.sh.in:m4_include(library/parseopts.sh) scripts/pacman-key.sh.in:m4_include(library/parseopts.sh) scripts/pkgdelta.sh.in:m4_include(library/parseopts.sh) test/scripts/parseopts_test.sh:lib=${1:-${PMTEST_SCRIPTLIB_DIR}parseopts.sh} /Rikard
On 10/08/2016 01:01 PM, Rikard Falkeborn wrote:
2016-10-08 7:18 GMT+02:00 Alad Wenter <alad@archlinux.info>:
Signed-off-by: Alad Wenter <alad@archlinux.info> --- v2: Add missing signoff.
scripts/libmakepkg/util/option.sh | 157 ++++++++++++++++++++++++++++++ ++++++++ scripts/library/README | 20 ----- scripts/library/parseopts.sh | 137 --------------------------------- scripts/makepkg.sh.in | 2 - 4 files changed, 157 insertions(+), 159 deletions(-) delete mode 100644 scripts/library/parseopts.sh
parseopts.sh is used in a couple of other scripts, is referenced in a couple of makefiles, and even has test cases. Each of these would need to be changed. See below.
Also, IMO, for a change like this, a short motivation should go into the commit message (one sentence may be enough).
Moving parseopts to libmakepkg would make a couple of "pacman"-scripts depend on libmakepkg (instead of library), which feels wrong, but feel free to ignore me, I'm not a pacman dev. :)
[~/code/pacman]./autogen.sh && ./configure && make ... make[3]: *** No rule to make target 'library/parseopts.sh', needed by 'makepkg'. Stop. make[2]: *** [Makefile:623: all-recursive] Error 1 make[1]: *** [Makefile:969: all-recursive] Error 1
git grep parseopts\.sh -- '*' ':!scripts/po/*' contrib/Makefile.am:paccache: $(srcdir)/paccache.sh.in $(top_srcdir)/scripts/library/parseopts.sh $(top_srcdir)/scripts/library/size_to_human.sh contrib/bacman.sh.in:m4_include(../scripts/library/parseopts.sh) contrib/paccache.sh.in:m4_include(../scripts/library/parseopts.sh) scripts/Makefile.am: library/parseopts.sh \ scripts/Makefile.am: $(srcdir)/library/parseopts.sh \ scripts/Makefile.am: $(srcdir)/library/parseopts.sh scripts/Makefile.am: $(srcdir)/library/parseopts.sh scripts/Makefile.am: $(srcdir)/library/parseopts.sh \ scripts/libmakepkg/util/option.sh:# parseopts.sh: scripts/pacman-db-upgrade.sh.in:m4_include(library/parseopts.sh) scripts/pacman-key.sh.in:m4_include(library/parseopts.sh) scripts/pkgdelta.sh.in:m4_include(library/parseopts.sh) test/scripts/parseopts_test.sh:lib=${1:-${PMTEST_SCRIPTLIB_DIR}parseopts.sh}
/Rikard
Thanks for your reply. Good point about the other scripts, but I'm not sure about contrib since those were proposed for removal... https://lists.archlinux.org/pipermail/pacman-dev/2016-October/021533.html Alad
participants (2)
-
Alad Wenter
-
Rikard Falkeborn