[pacman-dev] [PATCH 0/7] pacman-key patch series
This is a patch series that I hope gets pacman-key into near ready territory. The big first patch is to make pacman-key use our option parser. That fixes a lot of issues that Denis and Ivan were trying to work around with --config and --gpgdir and whether they needed to be specified first. The remaining patches are all fairly small. They should capture almost all the work that has been submitted to this list over the past few months (the exception being the --import patches by Pang Yan Han that I am yet to look at in detail). When the submitted patches applied somewhat cleanly, the patch authorship is obviously maintained. When the major changes due to option parsing meant I needed to scrape out the still needed parts of patches and rewrite them, I have given criedit for the original work in the commit messages. A further patch is to follow updating the man page for the changes made here. Allan McRae (5): pacman-key: use our option parser pacman-key: remove the --adv option pacman-key: rename --del to --delete pacman-key: allow the export of all key ids pacman-key: rename --trust to --edit-key Ivan Kanakarakis (2): pacman-key: fix quotation on several variable assignments pacman-key: hide output of executed commands on logic checks scripts/pacman-key.sh.in | 210 ++++++++++++++++++++++----------------------- 1 files changed, 103 insertions(+), 107 deletions(-) -- 1.7.6
The pacman-key script is complicated enough to warrent usage of the parse_options script. This is especially helpful in dealing with all the configuration file override flags as the no longer need to be specified first. It also allows us to do the right thing early with --help/--version and no option cases cleanly. This change also makde the check for root privileges only occur on operations where they are needed. This patch is inspired by and supercedes some patches submitted by Denis A. Altoé Falqueto and Ivan Kanakarakis who were altering the previous option handling in an attempt to deal with the above issues. Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-key.sh.in | 191 +++++++++++++++++++++++---------------------- 1 files changed, 98 insertions(+), 93 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 833943c..8e797f8 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -26,20 +26,30 @@ export TEXTDOMAINDIR='@localedir@' myver="@PACKAGE_VERSION@" +# Options +ADD=0 +ADVANCED=0 +DELETE=0 +EXPORT=0 +FINGER=0 +LIST=0 +RECEIVE=0 +RELOAD=0 +TRUST=0 +UPDATEDB=0 + m4_include(library/output_format.sh) +m4_include(library/parse_options.sh) + usage() { printf "pacman-key (pacman) %s\n" ${myver} echo - printf "$(gettext "Usage: %s [options] <command> [arguments]")\n" $(basename $0) + printf "$(gettext "Usage: %s [options]")\n" $(basename $0) echo printf "$(gettext "Manage pacman\'s list of trusted keys")\n" echo - echo "$(gettext "Options must be placed before commands. The available options are:")" - printf "$(gettext " --config <file> Use an alternate config file (instead of '%s')")\n" "$CONFIG" - printf "$(gettext " --gpgdir Set an alternate directory for gnupg (instead of '%s')")\n" "$PACMAN_KEYRING_DIR" - echo - echo "$(gettext "The available commands are:")" + echo "$(gettext "Options:")" echo "$(gettext " -a, --add [<file(s)>] Add the specified keys (empty for stdin)")" echo "$(gettext " -d, --del <keyid(s)> Remove the specified keyids")" echo "$(gettext " -e, --export <keyid(s)> Export the specified keyids")" @@ -51,8 +61,11 @@ usage() { echo "$(gettext " -u, --updatedb Update the trustdb of pacman")" echo "$(gettext " -V, --version Show program version")" echo "$(gettext " --adv <params> Use pacman's keyring with advanced gpg commands")" - printf "$(gettext " --reload Reload the default keys")" - echo + echo "$(gettext " --config <file> Use an alternate config file")" + printf "$(gettext " (instead of '%s')")\n" "@sysconfdir@/pacman.conf" + echo "$(gettext " --gpgdir <dir> Set an alternate directory for gnupg")" + printf "$(gettext " (instead of '%s')")\n" "@sysconfdir@/pacman.d/gnupg" + echo "$(gettext " --reload Reload the default keys")" } version() { @@ -198,116 +211,108 @@ if ! type gettext &>/dev/null; then } fi -if [[ $1 != "--version" && $1 != "-V" && $1 != "--help" && $1 != "-h" && $1 != "" ]]; then - if type -p gpg >/dev/null 2>&1 = 1; then - error "$(gettext "gnupg does not seem to be installed.")" - msg2 "$(gettext "pacman-key requires gnupg for most operations.")" - exit 1 - elif (( EUID != 0 )); then - error "$(gettext "pacman-key needs to be run as root.")" - exit 1 - fi +OPT_SHORT="a::d:e:f::hlr:t:uV" +OPT_LONG="add,adv:,config:,del:,export:,finger::,gpgdir:,help,list" +OPT_LONG+=",receive:,reload,trust:,updatedb,version" +if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then + echo; usage; exit 1 # E_INVALID_OPTION; +fi +eval set -- "$OPT_TEMP" +unset OPT_SHORT OPT_LONG OPT_TEMP + +if [[ $1 == "--" ]]; then + usage; + exit 0; fi -# Parse global options -CONFIG="@sysconfdir@/pacman.conf" -PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg" -while [[ $1 =~ ^--(config|gpgdir)$ ]]; do +while true; do case "$1" in - --config) shift; CONFIG="$1" ;; - --gpgdir) shift; PACMAN_KEYRING_DIR="$1" ;; + -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;; + --adv) ADVANCED=1; shift; ARGUMENTS=($1) ;; + --config) shift; CONFIG=$1 ;; + -d|--del) DELETE=1; shift; KEYIDS=($1) ;; + -e|--export) EXPORT=1; shift; KEYIDS=($1) ;; + -f|--finger) FINGER=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; + --gpgdir) shift; PACMAN_KEYRING_DIR=$1 ;; + -l|--list) LIST=1 ;; + -r|--receive) RECEIVE=1; shift; KEYSERVER="${1[0]}"; KEYIDS=("${1[@]:1}") ;; + --reload) RELOAD=1 ;; + -t|--trust) TRUST=1; shift; KEYIDS=($1) ;; + -u|--updatedb) UPDATEDB=1 ;; + + -h|--help) usage; exit 0 ;; + -V|--version) version; exit 0 ;; + + --) OPT_IND=0; shift; break;; + *) usage; exit 1 ;; esac shift done + +if ! type -p gpg >/dev/null; then + error "$(gettext "Cannot find the %s binary required for all %s operations.")" "gpg" "pacman-key" + exit 1 +fi + +if (( (ADD || ADVANCED || DELETE || RECEIVE || RELOAD || TRUST || UPDATEDB) && EUID != 0 )); then + error "$(gettext "%s needs to be run as root for this operation.")" "pacman-key" + exit 1 +fi + +CONFIG=${CONFIG:-@sysconfdir@/pacman.conf} if [[ ! -r "${CONFIG}" ]]; then - error "$(gettext "%s not found.")" "$CONFIG" + error "$(gettext "%s configuation file '%s' not found.")" "pacman" "$CONFIG" exit 1 fi -# Read GPGDIR from $CONFIG. -if [[ GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then +# Get GPGDIR from pacman.conf iff not specified on command line +if [[ -z PACMAN_KEYRING_DIR && GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then PACMAN_KEYRING_DIR="${GPGDIR}" fi -GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" +PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg} # Try to create $PACMAN_KEYRING_DIR if non-existent # Check for simple existence rather than for a directory as someone may want # to use a symlink here [[ -e ${PACMAN_KEYRING_DIR} ]] || mkdir -p -m 755 "${PACMAN_KEYRING_DIR}" -# Parse and execute command -command="$1" -if [[ -z "${command}" ]]; then - usage - exit 1 +GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" + + +(( ADD )) && ${GPG_PACMAN} --quiet --batch --import "${KEYFILES[@]}" +(( DELETE )) && ${GPG_PACMAN} --quiet --batch --delete-key --yes "${KEYIDS[@]}" +(( EXPORT )) && ${GPG_PACMAN} --armor --export "${KEYIDS[@]}" +(( FINGER )) && ${GPG_PACMAN} --batch --fingerprint "${KEYIDS[@]}" +(( LIST )) && ${GPG_PACMAN} --batch --list-sigs "${KEYIDS[@]}" +(( RELOAD )) && reload_keyring +(( UPDATEDB )) && ${GPG_PACMAN} --batch --check-trustdb + +if (( ADVANCED )); then + msg "$(gettext "Executing: %s %s")" "${GPG_PACMAN}" "${ARGUMENTS[@]}" + ${GPG_PACMAN} "${ARGUMENTS[@]}" || ret=$? + exit $ret fi -shift - -case "${command}" in - -a|--add) - # If there is no extra parameter, gpg will read stdin - ${GPG_PACMAN} --quiet --batch --import "$@" - ;; - -d|--del) - if (( $# == 0 )); then - error "$(gettext "You need to specify at least one key identifier")" - exit 1 - fi - ${GPG_PACMAN} --quiet --batch --delete-key --yes "$@" - ;; - -u|--updatedb) - ${GPG_PACMAN} --batch --check-trustdb - ;; - --reload) - reload_keyring - ;; - -l|--list) - ${GPG_PACMAN} --batch --list-sigs "$@" - ;; - -f|--finger) - ${GPG_PACMAN} --batch --fingerprint "$@" - ;; - -e|--export) - ${GPG_PACMAN} --armor --export "$@" - ;; - -r|--receive) - if (( $# < 2 )); then - error "$(gettext "You need to specify the keyserver and at least one key identifier")" - exit 1 - fi - keyserver="$1" - shift - ${GPG_PACMAN} --keyserver "${keyserver}" --recv-keys "$@" - ;; - -t|--trust) - if (( $# == 0 )); then - error "$(gettext "You need to specify at least one key identifier")" - exit 1 - fi - while (( $# > 0 )); do + +if (( RECEIVE )); then + if [[ -z ${KEYIDS[@]} ]]; then + error "$(gettext "You need to specify the keyserver and at least one key identifier")" + exit 1 + fi + ${GPG_PACMAN} --keyserver "$KEYSERVER" --recv-keys "${KEYIDS[@]}" +fi + +if (( TRUST )); then + for key in ${KEYIDS[@]}; do # Verify if the key exists in pacman's keyring - if ${GPG_PACMAN} --list-keys "$1" > /dev/null 2>&1; then - ${GPG_PACMAN} --edit-key "$1" + if ${GPG_PACMAN} --list-keys "$key" > /dev/null 2>&1; then + ${GPG_PACMAN} --edit-key "$key" else - error "$(gettext "The key identified by %s doesn't exist")" "$1" + error "$(gettext "The key identified by %s does not exist")" "$key" exit 1 fi shift done - ;; - --adv) - msg "$(gettext "Executing: %s ")$*" "${GPG_PACMAN}" - ${GPG_PACMAN} "$@" || ret=$? - exit $ret - ;; - -h|--help) - usage; exit 0 ;; - -V|--version) - version; exit 0 ;; - *) - error "$(gettext "Unknown command:") $command" - usage; exit 1 ;; -esac +fi # vim: set ts=2 sw=2 noet: -- 1.7.6
The conversion to using parse_options causes this option to break. It is preferable to remove the option rather than fix it as it is simply a wrapper for "gpg --homedir @sysconfdir@/pacman.d/gnupg". Any user using more advanced keyring management than provided by pacman-key can manage to point gpg at the right place themselves... How to manually edit the keyring with gpg will instead be documented in the man page in a later commit. Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-key.sh.in | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 8e797f8..e49811c 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -28,7 +28,6 @@ myver="@PACKAGE_VERSION@" # Options ADD=0 -ADVANCED=0 DELETE=0 EXPORT=0 FINGER=0 @@ -60,7 +59,6 @@ usage() { echo "$(gettext " -t, --trust <keyid(s)> Set the trust level of the given keyids")" echo "$(gettext " -u, --updatedb Update the trustdb of pacman")" echo "$(gettext " -V, --version Show program version")" - echo "$(gettext " --adv <params> Use pacman's keyring with advanced gpg commands")" echo "$(gettext " --config <file> Use an alternate config file")" printf "$(gettext " (instead of '%s')")\n" "@sysconfdir@/pacman.conf" echo "$(gettext " --gpgdir <dir> Set an alternate directory for gnupg")" @@ -228,7 +226,6 @@ fi while true; do case "$1" in -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;; - --adv) ADVANCED=1; shift; ARGUMENTS=($1) ;; --config) shift; CONFIG=$1 ;; -d|--del) DELETE=1; shift; KEYIDS=($1) ;; -e|--export) EXPORT=1; shift; KEYIDS=($1) ;; @@ -255,7 +252,7 @@ if ! type -p gpg >/dev/null; then exit 1 fi -if (( (ADD || ADVANCED || DELETE || RECEIVE || RELOAD || TRUST || UPDATEDB) && EUID != 0 )); then +if (( (ADD || DELETE || RECEIVE || RELOAD || TRUST || UPDATEDB) && EUID != 0 )); then error "$(gettext "%s needs to be run as root for this operation.")" "pacman-key" exit 1 fi @@ -288,12 +285,6 @@ GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" (( RELOAD )) && reload_keyring (( UPDATEDB )) && ${GPG_PACMAN} --batch --check-trustdb -if (( ADVANCED )); then - msg "$(gettext "Executing: %s %s")" "${GPG_PACMAN}" "${ARGUMENTS[@]}" - ${GPG_PACMAN} "${ARGUMENTS[@]}" || ret=$? - exit $ret -fi - if (( RECEIVE )); then if [[ -z ${KEYIDS[@]} ]]; then error "$(gettext "You need to specify the keyserver and at least one key identifier")" -- 1.7.6
There is already the short -d alias provided, so stay verbose with the longer option name. Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-key.sh.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index e49811c..f4f5f70 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -50,7 +50,7 @@ usage() { echo echo "$(gettext "Options:")" echo "$(gettext " -a, --add [<file(s)>] Add the specified keys (empty for stdin)")" - echo "$(gettext " -d, --del <keyid(s)> Remove the specified keyids")" + echo "$(gettext " -d, --delete <keyid(s)> Remove the specified keyids")" echo "$(gettext " -e, --export <keyid(s)> Export the specified keyids")" echo "$(gettext " -f, --finger [<keyid(s)>] List fingerprint for specified or all keyids")" echo "$(gettext " -h, --help Show this help message and exit")" @@ -227,7 +227,7 @@ while true; do case "$1" in -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;; --config) shift; CONFIG=$1 ;; - -d|--del) DELETE=1; shift; KEYIDS=($1) ;; + -d|--delete) DELETE=1; shift; KEYIDS=($1) ;; -e|--export) EXPORT=1; shift; KEYIDS=($1) ;; -f|--finger) FINGER=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;; --gpgdir) shift; PACMAN_KEYRING_DIR=$1 ;; -- 1.7.6
The gpg --export will exprt all keys if none are specified. Replicate this behavior in pacman-key. Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-key.sh.in | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index f4f5f70..d75c111 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -51,7 +51,7 @@ usage() { echo "$(gettext "Options:")" echo "$(gettext " -a, --add [<file(s)>] Add the specified keys (empty for stdin)")" echo "$(gettext " -d, --delete <keyid(s)> Remove the specified keyids")" - echo "$(gettext " -e, --export <keyid(s)> Export the specified keyids")" + echo "$(gettext " -e, --export [<keyid(s)>] Export the specified or all keyids")" echo "$(gettext " -f, --finger [<keyid(s)>] List fingerprint for specified or all keyids")" echo "$(gettext " -h, --help Show this help message and exit")" echo "$(gettext " -l, --list List keys")" @@ -210,7 +210,7 @@ if ! type gettext &>/dev/null; then fi OPT_SHORT="a::d:e:f::hlr:t:uV" -OPT_LONG="add,adv:,config:,del:,export:,finger::,gpgdir:,help,list" +OPT_LONG="add,adv:,config:,del:,export::,finger::,gpgdir:,help,list" OPT_LONG+=",receive:,reload,trust:,updatedb,version" if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then echo; usage; exit 1 # E_INVALID_OPTION; @@ -228,7 +228,7 @@ while true; do -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;; --config) shift; CONFIG=$1 ;; -d|--delete) DELETE=1; shift; KEYIDS=($1) ;; - -e|--export) EXPORT=1; shift; KEYIDS=($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) ;; --gpgdir) shift; PACMAN_KEYRING_DIR=$1 ;; -l|--list) LIST=1 ;; -- 1.7.6
From: Ivan Kanakarakis <ivan.kanak@gmail.com> This commit adds quotes to several variable assignments. Unquoted values can cause problems on several occasions if the value is empty. It is safer to have every assignment quoted. Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com> --- scripts/pacman-key.sh.in | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index d75c111..b15a3a5 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -137,12 +137,12 @@ reload_keyring() { if [[ -r "${REMOVED_KEYS}" ]]; then while read key; do local key_values name - key_values=$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5,10 --output-delimiter=' ') + key_values="$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5,10 --output-delimiter=' ')" if [[ -n $key_values ]]; then # The first word is the key_id - key_id=${key_values%% *} + key_id="${key_values%% *}" # the rest if the name of the owner - name=${key_values#* } + name="${key_values#* }" if [[ -n ${key_id} ]]; then # Mark this key to be deleted removed_ids[$key_id]="$name" @@ -152,12 +152,12 @@ reload_keyring() { fi # List of keys that must be kept installed, even if in the list of keys to be removed - local HOLD_KEYS=$(get_from "$CONFIG" "HoldKeys") + local HOLD_KEYS="$(get_from "$CONFIG" "HoldKeys")" # Remove the keys that must be kept from the set of keys that should be removed if [[ -n ${HOLD_KEYS} ]]; then for key in ${HOLD_KEYS}; do - key_id=$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5) + key_id="$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5)" if [[ -n "${removed_ids[$key_id]}" ]]; then unset removed_ids[$key_id] fi @@ -168,7 +168,7 @@ reload_keyring() { # be updated automatically. if [[ -r "${ADDED_KEYS}" ]]; then msg "$(gettext "Appending official keys...")" - local add_keys=$(${GPG_NOKEYRING} --keyring "${ADDED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5) + local add_keys="$(${GPG_NOKEYRING} --keyring "${ADDED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5)" for key_id in ${add_keys}; do # There is no point in adding a key that will be deleted right after if [[ -z "${removed_ids[$key_id]}" ]]; then @@ -179,7 +179,7 @@ reload_keyring() { if [[ -r "${DEPRECATED_KEYS}" ]]; then msg "$(gettext "Appending deprecated keys...")" - local add_keys=$(${GPG_NOKEYRING} --keyring "${DEPRECATED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5) + local add_keys="$(${GPG_NOKEYRING} --keyring "${DEPRECATED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5)" for key_id in ${add_keys}; do # There is no point in adding a key that will be deleted right after if [[ -z "${removed_ids[$key_id]}" ]]; then @@ -264,7 +264,7 @@ if [[ ! -r "${CONFIG}" ]]; then fi # Get GPGDIR from pacman.conf iff not specified on command line -if [[ -z PACMAN_KEYRING_DIR && GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then +if [[ -z PACMAN_KEYRING_DIR && GPGDIR="$(get_from "$CONFIG" "GPGDir")" == 0 ]]; then PACMAN_KEYRING_DIR="${GPGDIR}" fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg} -- 1.7.6
This keeps the naming of the option more consistent with what is actually being called by gpg. Original-patch-by: Denis A. Altoé Falqueto <denisfalqueto@gmail.com> Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-key.sh.in | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b15a3a5..9017ea9 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -29,12 +29,12 @@ myver="@PACKAGE_VERSION@" # Options ADD=0 DELETE=0 +EDITKEY=0 EXPORT=0 FINGER=0 LIST=0 RECEIVE=0 RELOAD=0 -TRUST=0 UPDATEDB=0 m4_include(library/output_format.sh) @@ -56,11 +56,11 @@ usage() { echo "$(gettext " -h, --help Show this help message and exit")" echo "$(gettext " -l, --list List keys")" echo "$(gettext " -r, --receive <keyserver> <keyid(s)> Fetch the specified keyids")" - echo "$(gettext " -t, --trust <keyid(s)> Set the trust level of the given keyids")" echo "$(gettext " -u, --updatedb Update the trustdb of pacman")" echo "$(gettext " -V, --version Show program version")" echo "$(gettext " --config <file> Use an alternate config file")" printf "$(gettext " (instead of '%s')")\n" "@sysconfdir@/pacman.conf" + echo "$(gettext " --edit-key <keyid(s)> Present a menu for key management task on keyids")" echo "$(gettext " --gpgdir <dir> Set an alternate directory for gnupg")" printf "$(gettext " (instead of '%s')")\n" "@sysconfdir@/pacman.d/gnupg" echo "$(gettext " --reload Reload the default keys")" @@ -228,13 +228,13 @@ while true; do -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;; --config) shift; CONFIG=$1 ;; -d|--delete) DELETE=1; shift; KEYIDS=($1) ;; + --edit-key) EDITKEY=1; shift; KEYIDS=($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) ;; --gpgdir) shift; PACMAN_KEYRING_DIR=$1 ;; -l|--list) LIST=1 ;; -r|--receive) RECEIVE=1; shift; KEYSERVER="${1[0]}"; KEYIDS=("${1[@]:1}") ;; --reload) RELOAD=1 ;; - -t|--trust) TRUST=1; shift; KEYIDS=($1) ;; -u|--updatedb) UPDATEDB=1 ;; -h|--help) usage; exit 0 ;; @@ -252,7 +252,7 @@ if ! type -p gpg >/dev/null; then exit 1 fi -if (( (ADD || DELETE || RECEIVE || RELOAD || TRUST || UPDATEDB) && EUID != 0 )); then +if (( (ADD || DELETE || EDITKEY || RECEIVE || RELOAD || UPDATEDB) && EUID != 0 )); then error "$(gettext "%s needs to be run as root for this operation.")" "pacman-key" exit 1 fi @@ -293,7 +293,7 @@ if (( RECEIVE )); then ${GPG_PACMAN} --keyserver "$KEYSERVER" --recv-keys "${KEYIDS[@]}" fi -if (( TRUST )); then +if (( EDITKEY )); then for key in ${KEYIDS[@]}; do # Verify if the key exists in pacman's keyring if ${GPG_PACMAN} --list-keys "$key" > /dev/null 2>&1; then -- 1.7.6
From: Ivan Kanakarakis <ivan.kanak@gmail.com> This commit correctly redirects to /dev/null the output of several commands that get executed on logic checks. Original-patch-by: Denis A. Altoé Falqueto <denisfalqueto@gmail.com> Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com> Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-key.sh.in | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 9017ea9..b2b5669 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -108,7 +108,7 @@ reload_keyring() { # Verify signatures of related files, if they exist if [[ -r "${ADDED_KEYS}" ]]; then msg "$(gettext "Verifying official keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${ADDED_KEYS}.sig" 1>/dev/null; then + if ! ${GPG_PACMAN} --verify "${ADDED_KEYS}.sig" &>/dev/null; then error "$(gettext "The signature of file %s is not valid.")" "${ADDED_KEYS}" exit 1 fi @@ -116,7 +116,7 @@ reload_keyring() { if [[ -r "${DEPRECATED_KEYS}" ]]; then msg "$(gettext "Verifying deprecated keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${DEPRECATED_KEYS}.sig" 1>/dev/null; then + if ! ${GPG_PACMAN} --verify "${DEPRECATED_KEYS}.sig" &>/dev/null; then error "$(gettext "The signature of file %s is not valid.")" "${DEPRECATED_KEYS}" exit 1 fi @@ -124,7 +124,7 @@ reload_keyring() { if [[ -r "${REMOVED_KEYS}" ]]; then msg "$(gettext "Verifying deleted keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${REMOVED_KEYS}.sig"; then + if ! ${GPG_PACMAN} --verify "${REMOVED_KEYS}.sig" &>/dev/null; then error "$(gettext "The signature of file %s is not valid.")" "${REMOVED_KEYS}" exit 1 fi @@ -296,7 +296,7 @@ fi if (( EDITKEY )); then for key in ${KEYIDS[@]}; do # Verify if the key exists in pacman's keyring - if ${GPG_PACMAN} --list-keys "$key" > /dev/null 2>&1; then + if ${GPG_PACMAN} --list-keys "$key" &>/dev/null; then ${GPG_PACMAN} --edit-key "$key" else error "$(gettext "The key identified by %s does not exist")" "$key" -- 1.7.6
On Fri, Jul 08, 2011 at 09:59:30PM +1000, Allan McRae wrote:
From: Ivan Kanakarakis <ivan.kanak@gmail.com>
This commit adds quotes to several variable assignments. Unquoted values can cause problems on several occasions if the value is empty. It is safer to have every assignment quoted.
Perhaps this is just academic, but is there a test case for this? I'm not aware of any situation where this it's necessary to quote to assignment of a command sub or simple variable to a var to avoid any oddities. $ foo= bar="" baz=${notavar} $ echo ${#foo} ${#bar} ${#baz} 0 0 0
Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com> --- scripts/pacman-key.sh.in | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index d75c111..b15a3a5 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -137,12 +137,12 @@ reload_keyring() { if [[ -r "${REMOVED_KEYS}" ]]; then while read key; do local key_values name - key_values=$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5,10 --output-delimiter=' ') + key_values="$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5,10 --output-delimiter=' ')" if [[ -n $key_values ]]; then # The first word is the key_id - key_id=${key_values%% *} + key_id="${key_values%% *}" # the rest if the name of the owner - name=${key_values#* } + name="${key_values#* }" if [[ -n ${key_id} ]]; then # Mark this key to be deleted removed_ids[$key_id]="$name" @@ -152,12 +152,12 @@ reload_keyring() { fi
# List of keys that must be kept installed, even if in the list of keys to be removed - local HOLD_KEYS=$(get_from "$CONFIG" "HoldKeys") + local HOLD_KEYS="$(get_from "$CONFIG" "HoldKeys")"
# Remove the keys that must be kept from the set of keys that should be removed if [[ -n ${HOLD_KEYS} ]]; then for key in ${HOLD_KEYS}; do - key_id=$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5) + key_id="$(${GPG_PACMAN} --quiet --with-colons --list-key "${key}" | grep ^pub | cut -d: -f5)" if [[ -n "${removed_ids[$key_id]}" ]]; then unset removed_ids[$key_id] fi @@ -168,7 +168,7 @@ reload_keyring() { # be updated automatically. if [[ -r "${ADDED_KEYS}" ]]; then msg "$(gettext "Appending official keys...")" - local add_keys=$(${GPG_NOKEYRING} --keyring "${ADDED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5) + local add_keys="$(${GPG_NOKEYRING} --keyring "${ADDED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5)" for key_id in ${add_keys}; do # There is no point in adding a key that will be deleted right after if [[ -z "${removed_ids[$key_id]}" ]]; then @@ -179,7 +179,7 @@ reload_keyring() {
if [[ -r "${DEPRECATED_KEYS}" ]]; then msg "$(gettext "Appending deprecated keys...")" - local add_keys=$(${GPG_NOKEYRING} --keyring "${DEPRECATED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5) + local add_keys="$(${GPG_NOKEYRING} --keyring "${DEPRECATED_KEYS}" --with-colons --list-keys | grep ^pub | cut -d: -f5)" for key_id in ${add_keys}; do # There is no point in adding a key that will be deleted right after if [[ -z "${removed_ids[$key_id]}" ]]; then @@ -264,7 +264,7 @@ if [[ ! -r "${CONFIG}" ]]; then fi
# Get GPGDIR from pacman.conf iff not specified on command line -if [[ -z PACMAN_KEYRING_DIR && GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then +if [[ -z PACMAN_KEYRING_DIR && GPGDIR="$(get_from "$CONFIG" "GPGDir")" == 0 ]]; then
This doesn't pertain to this patch, but I don't understand this logic. get_from should be writing the value of GPGDir as it's read from $CONFIG. It looks like the goal here was to make sure that get_from was successful, which would be written as: if [[ -z PACMAN_KEYRING_DIR ]] && GPGDIR=$(get_from "$CONFIG" "GPGDir"); then PACMAN_KEYRING_DIR=$GPGDIR fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg} Or through a single default assignment to tidy the whole thing up: # if PACMAN_KEYRING_DIR isn't assigned, try to get it from the config # file, falling back on a hard default. : ${PACMAN_KEYRING_DIR:=$(get_from "$CONFIG" "GPGDir" || echo "@sysconfdir@/pacman.d/gnupg")} Will happily write up a patch if this is what was actually intended... d
PACMAN_KEYRING_DIR="${GPGDIR}" fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg} -- 1.7.6
On Fri, Jul 8, 2011 at 6:59 AM, Allan McRae <allan@archlinux.org> wrote:
The conversion to using parse_options causes this option to break. It is preferable to remove the option rather than fix it as it is simply a wrapper for "gpg --homedir @sysconfdir@/pacman.d/gnupg". Any user using more advanced keyring management than provided by pacman-key can manage to point gpg at the right place themselves...
How to manually edit the keyring with gpg will instead be documented in the man page in a later commit.
I won't lie here, I'm not a fan of this but maybe because I've become accustomed to the option being available. It was way easier than typing out the long-form gpg command line. "pacman-key --adv --verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig" is something I just pulled out of my command history. What if we just enforced instead that the entire arg string was quoted: pacman-key --adv "--verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig" Or perhaps the "don't parse anymore" option: pacman-key --adv -- --verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig -Dan
Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-key.sh.in | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 8e797f8..e49811c 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -28,7 +28,6 @@ myver="@PACKAGE_VERSION@"
# Options ADD=0 -ADVANCED=0 DELETE=0 EXPORT=0 FINGER=0 @@ -60,7 +59,6 @@ usage() { echo "$(gettext " -t, --trust <keyid(s)> Set the trust level of the given keyids")" echo "$(gettext " -u, --updatedb Update the trustdb of pacman")" echo "$(gettext " -V, --version Show program version")" - echo "$(gettext " --adv <params> Use pacman's keyring with advanced gpg commands")" echo "$(gettext " --config <file> Use an alternate config file")" printf "$(gettext " (instead of '%s')")\n" "@sysconfdir@/pacman.conf" echo "$(gettext " --gpgdir <dir> Set an alternate directory for gnupg")" @@ -228,7 +226,6 @@ fi while true; do case "$1" in -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;; - --adv) ADVANCED=1; shift; ARGUMENTS=($1) ;; --config) shift; CONFIG=$1 ;; -d|--del) DELETE=1; shift; KEYIDS=($1) ;; -e|--export) EXPORT=1; shift; KEYIDS=($1) ;; @@ -255,7 +252,7 @@ if ! type -p gpg >/dev/null; then exit 1 fi
-if (( (ADD || ADVANCED || DELETE || RECEIVE || RELOAD || TRUST || UPDATEDB) && EUID != 0 )); then +if (( (ADD || DELETE || RECEIVE || RELOAD || TRUST || UPDATEDB) && EUID != 0 )); then error "$(gettext "%s needs to be run as root for this operation.")" "pacman-key" exit 1 fi @@ -288,12 +285,6 @@ GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" (( RELOAD )) && reload_keyring (( UPDATEDB )) && ${GPG_PACMAN} --batch --check-trustdb
-if (( ADVANCED )); then - msg "$(gettext "Executing: %s %s")" "${GPG_PACMAN}" "${ARGUMENTS[@]}" - ${GPG_PACMAN} "${ARGUMENTS[@]}" || ret=$? - exit $ret -fi - if (( RECEIVE )); then if [[ -z ${KEYIDS[@]} ]]; then error "$(gettext "You need to specify the keyserver and at least one key identifier")" -- 1.7.6
On 08/07/11 23:55, Dan McGee wrote:
On Fri, Jul 8, 2011 at 6:59 AM, Allan McRae<allan@archlinux.org> wrote:
The conversion to using parse_options causes this option to break. It is preferable to remove the option rather than fix it as it is simply a wrapper for "gpg --homedir @sysconfdir@/pacman.d/gnupg". Any user using more advanced keyring management than provided by pacman-key can manage to point gpg at the right place themselves...
How to manually edit the keyring with gpg will instead be documented in the man page in a later commit.
I won't lie here, I'm not a fan of this but maybe because I've become accustomed to the option being available. It was way easier than typing out the long-form gpg command line. "pacman-key --adv --verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig" is something I just pulled out of my command history.
What if we just enforced instead that the entire arg string was quoted: pacman-key --adv "--verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig" Or perhaps the "don't parse anymore" option: pacman-key --adv -- --verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig
The latter would work, but I am still not entirely convinced about the need for this... I intend to add a --verify option to pacman-key because I think that would be a fairly common command to use. Anything else with common usage should also be added to pacman-key. Is there anything else you used this for? I just have this nagging feeling that hiding what gpg is doing (we already have --no-permission-warning there by default) is not the way to go. Not that my opinion is overly strong on this. Allan
On Friday, July 8, 2011, Allan McRae <allan@archlinux.org> wrote:
On 08/07/11 23:55, Dan McGee wrote:
On Fri, Jul 8, 2011 at 6:59 AM, Allan McRae<allan@archlinux.org> wrote:
The conversion to using parse_options causes this option to break. It is preferable to remove the option rather than fix it as it is simply a wrapper for "gpg --homedir @sysconfdir@/pacman.d/gnupg". Any user using more advanced keyring management than provided by pacman-key can manage to point gpg at the right place themselves...
How to manually edit the keyring with gpg will instead be documented in the man page in a later commit.
I won't lie here, I'm not a fan of this but maybe because I've become accustomed to the option being available. It was way easier than typing out the long-form gpg command line. "pacman-key --adv --verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig" is something I just pulled out of my command history.
What if we just enforced instead that the entire arg string was quoted: pacman-key --adv "--verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig" Or perhaps the "don't parse anymore" option: pacman-key --adv -- --verify /tmp/cryptsetup-1.3.1-1-x86_64.pkg.tar.xz.sig
The latter would work, but I am still not entirely convinced about the need for this...
I intend to add a --verify option to pacman-key because I think that would be a fairly common command to use. Anything else with common usage should also be added to pacman-key.
Is there anything else you used this for? I just have this nagging feeling that hiding what gpg is doing (we already have --no-permission-warning there by default) is not the way to go. Not that my opinion is overly strong on this.
That works too- if we add a --verify then I'm fine with this patch. The --no-perm-warn is likely something we can/should move to a default gpg.conf file. -Dan
On 08/07/11 22:56, Dave Reisner wrote:
@@ -264,7 +264,7 @@ if [[ ! -r "${CONFIG}" ]]; then fi
# Get GPGDIR from pacman.conf iff not specified on command line -if [[ -z PACMAN_KEYRING_DIR&& GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then +if [[ -z PACMAN_KEYRING_DIR&& GPGDIR="$(get_from "$CONFIG" "GPGDir")" == 0 ]]; then
This doesn't pertain to this patch, but I don't understand this logic. get_from should be writing the value of GPGDir as it's read from $CONFIG. It looks like the goal here was to make sure that get_from was successful, which would be written as:
if [[ -z PACMAN_KEYRING_DIR ]]&& GPGDIR=$(get_from "$CONFIG" "GPGDir"); then PACMAN_KEYRING_DIR=$GPGDIR fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg}
Or through a single default assignment to tidy the whole thing up:
# if PACMAN_KEYRING_DIR isn't assigned, try to get it from the config # file, falling back on a hard default. : ${PACMAN_KEYRING_DIR:=$(get_from "$CONFIG" "GPGDir" || echo "@sysconfdir@/pacman.d/gnupg")}
Will happily write up a patch if this is what was actually intended...
Send the patch. The key is just to have the value is assigned in this priority: 1) --gpgdir value 2) $CONFIF value 3) default Allan
On Sat, Jul 09, 2011 at 06:47:34AM +1000, Allan McRae wrote:
On 08/07/11 22:56, Dave Reisner wrote:
@@ -264,7 +264,7 @@ if [[ ! -r "${CONFIG}" ]]; then fi
# Get GPGDIR from pacman.conf iff not specified on command line -if [[ -z PACMAN_KEYRING_DIR&& GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then +if [[ -z PACMAN_KEYRING_DIR&& GPGDIR="$(get_from "$CONFIG" "GPGDir")" == 0 ]]; then
This doesn't pertain to this patch, but I don't understand this logic. get_from should be writing the value of GPGDir as it's read from $CONFIG. It looks like the goal here was to make sure that get_from was successful, which would be written as:
if [[ -z PACMAN_KEYRING_DIR ]]&& GPGDIR=$(get_from "$CONFIG" "GPGDir"); then PACMAN_KEYRING_DIR=$GPGDIR fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg}
Or through a single default assignment to tidy the whole thing up:
# if PACMAN_KEYRING_DIR isn't assigned, try to get it from the config # file, falling back on a hard default. : ${PACMAN_KEYRING_DIR:=$(get_from "$CONFIG" "GPGDir" || echo "@sysconfdir@/pacman.d/gnupg")}
Will happily write up a patch if this is what was actually intended...
Send the patch.
The key is just to have the value is assigned in this priority: 1) --gpgdir value 2) $CONFIF value 3) default
Allan
I'll wait till this work is merged before sending, because I'm really not sure what to base the patch on. Also noticing now that I have beef with get_from -- the comment says that the equal sign _can_ be surrounded by random whitespace, but in reality it _must_ be surrounded by whitespace or else the method will fail to find anything. dave
On 09/07/11 07:00, Dave Reisner wrote:
On Sat, Jul 09, 2011 at 06:47:34AM +1000, Allan McRae wrote:
On 08/07/11 22:56, Dave Reisner wrote:
@@ -264,7 +264,7 @@ if [[ ! -r "${CONFIG}" ]]; then fi
# Get GPGDIR from pacman.conf iff not specified on command line -if [[ -z PACMAN_KEYRING_DIR&& GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then +if [[ -z PACMAN_KEYRING_DIR&& GPGDIR="$(get_from "$CONFIG" "GPGDir")" == 0 ]]; then
This doesn't pertain to this patch, but I don't understand this logic. get_from should be writing the value of GPGDir as it's read from $CONFIG. It looks like the goal here was to make sure that get_from was successful, which would be written as:
if [[ -z PACMAN_KEYRING_DIR ]]&& GPGDIR=$(get_from "$CONFIG" "GPGDir"); then PACMAN_KEYRING_DIR=$GPGDIR fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg}
Or through a single default assignment to tidy the whole thing up:
# if PACMAN_KEYRING_DIR isn't assigned, try to get it from the config # file, falling back on a hard default. : ${PACMAN_KEYRING_DIR:=$(get_from "$CONFIG" "GPGDir" || echo "@sysconfdir@/pacman.d/gnupg")}
Will happily write up a patch if this is what was actually intended...
Send the patch.
The key is just to have the value is assigned in this priority: 1) --gpgdir value 2) $CONFIF value 3) default
Allan
I'll wait till this work is merged before sending, because I'm really not sure what to base the patch on.
Also noticing now that I have beef with get_from -- the comment says that the equal sign _can_ be surrounded by random whitespace, but in reality it _must_ be surrounded by whitespace or else the method will fail to find anything.
I'm not sure about the get_from function at all... I really do not think it is very robust. In fact, from memory I'm not sure an = sign is even actually checked for. Allan
On Sat, Jul 09, 2011 at 07:13:33AM +1000, Allan McRae wrote:
On 09/07/11 07:00, Dave Reisner wrote:
On Sat, Jul 09, 2011 at 06:47:34AM +1000, Allan McRae wrote:
On 08/07/11 22:56, Dave Reisner wrote:
@@ -264,7 +264,7 @@ if [[ ! -r "${CONFIG}" ]]; then fi
# Get GPGDIR from pacman.conf iff not specified on command line -if [[ -z PACMAN_KEYRING_DIR&& GPGDIR=$(get_from "$CONFIG" "GPGDir") == 0 ]]; then +if [[ -z PACMAN_KEYRING_DIR&& GPGDIR="$(get_from "$CONFIG" "GPGDir")" == 0 ]]; then
This doesn't pertain to this patch, but I don't understand this logic. get_from should be writing the value of GPGDir as it's read from $CONFIG. It looks like the goal here was to make sure that get_from was successful, which would be written as:
if [[ -z PACMAN_KEYRING_DIR ]]&& GPGDIR=$(get_from "$CONFIG" "GPGDir"); then PACMAN_KEYRING_DIR=$GPGDIR fi PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-@sysconfdir@/pacman.d/gnupg}
Or through a single default assignment to tidy the whole thing up:
# if PACMAN_KEYRING_DIR isn't assigned, try to get it from the config # file, falling back on a hard default. : ${PACMAN_KEYRING_DIR:=$(get_from "$CONFIG" "GPGDir" || echo "@sysconfdir@/pacman.d/gnupg")}
Will happily write up a patch if this is what was actually intended...
Send the patch.
The key is just to have the value is assigned in this priority: 1) --gpgdir value 2) $CONFIF value 3) default
Allan
I'll wait till this work is merged before sending, because I'm really not sure what to base the patch on.
Also noticing now that I have beef with get_from -- the comment says that the equal sign _can_ be surrounded by random whitespace, but in reality it _must_ be surrounded by whitespace or else the method will fail to find anything.
I'm not sure about the get_from function at all... I really do not think it is very robust. In fact, from memory I'm not sure an = sign is even actually checked for.
Allan
It's not. I'm less concerned about the syntax checking because pacman will do that for us. An entry such as 'GPGDir is /dev/null' will be parsed as a valid entry by pacman-key, but pacman won't be happy about it. More concerning is the fact that 'key=val' isn't separated properly. We should be setting IFS to '=' and doing whitespace trimming of the key and value. Quickly... get_from() { while IFS='=' read -r key value; do if [[ ${key%% *} = "$2" ]]; then echo "${value##* }" return 0 fi done return 1 } Amazingly, we have the 0/1 return value in place in the current function (its magical). The break will imply a return of 0, and read will return 1 on EOF, forcing return of 1. Probably should make it explicit... d
participants (3)
-
Allan McRae
-
Dan McGee
-
Dave Reisner