[pacman-dev] [PATCH] scripts/pacman-key.sh.in: fix processing of --help/-h and --version/-V options
From: Ivan Kanakarakis <ivan.kanak@gmail.com> ok, I think this is much better. The process is as follows: The script first tries to read the options as defined in the usage. It then expects to read a command. If the command exists and it's one of --help/-h/--version/-V it skips any check and goes straight to the case loop, and thus executes those. If the command is something else, it enters the big if block and checks for the needed conditions. also when an unknown command is given there now an error message instead of just the usage unknown command: <--the_command> some more comments: I saw some things like error "$(gettext "The key identified by %s doesn't exist")" "$1" while error() only knows about the first argument given to it, so %s there is actually left blank. also I think there are some cases where gettext is redundant as in called twice for the same text. I can go fixing this too. Signed-off-by: Ivan Kanakarakis <ivan.kanak@gmail.com> --- scripts/pacman-key.sh.in | 218 +++++++++++++++++++++++++--------------------- 1 files changed, 117 insertions(+), 101 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 89e52fc..9e0b73d 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -63,7 +63,7 @@ usage() { echo "$(gettext " -d, --del <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 This help")" + echo "$(gettext " -h, --help Display this help message")" 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")" @@ -211,115 +211,131 @@ 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 -fi - -# Parse global options +# Set default values CONFIG="@sysconfdir@/pacman.conf" PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg" + +# Parse command line options while [[ $1 =~ ^--(config|gpgdir)$ ]]; do - case "$1" in - --config) shift; CONFIG="$1" ;; - --gpgdir) shift; PACMAN_KEYRING_DIR="$1" ;; - esac - shift + case "$1" in + --config) shift; cli_config="$1" + ;; + --gpgdir) shift; PACMAN_KEYRING_DIR="$1" + ;; + esac + shift done -if [[ ! -r "${CONFIG}" ]]; then - error "$(gettext "%s not found.")" "$CONFIG" - exit 1 -fi - -# Read GPGDIR from $CONFIG. -# The pattern is: any spaces or tabs, GPGDir, any spaces or tabs, equal sign -# and the rest of the line. The string is splitted after the first occurrence of = -if [[ GPGDIR=$(find_config "GPGDir") == 0 ]]; then - PACMAN_KEYRING_DIR="${GPGDIR}" -fi -GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" - -# 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 +# Parse the command command="$1" if [[ -z "${command}" ]]; then - usage - exit 1 + usage + exit 1 fi shift +# If command is --help/-h or --version/-V then skip checks and execute those else +# check for: dependencies , permissions and state of needed directories and files +if [[ ! ${command} =~ ^(--help|-h|--version|-V)$ ]]; then + if ! type -p gpg &>/dev/null; then + error "$(gettext "gnupg does not seem to be installed.")" + msg2 "$(gettext "pacman-key requires gnupg for most operations.")" + exit 1 + fi + if (( EUID != 0 )); then + error "$(gettext "pacman-key needs to be run as root.")" + exit 1 + fi + # 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 + if [[ ! -e ${PACMAN_KEYRING_DIR} ]]; then + mkdir -p -m 755 "${PACMAN_KEYRING_DIR}" + fi + # Check given configuration file. Failback to default configuration if the + # specified cannot be found or read. + if [[ ${cli_config} ]]; then + if [[ ! -r ${cli_config} ]]; then + error "$(gettext "Couldn't read configuration file: %s. Using default configuration.")" "$cli_config" + else + CONFIG="$cli_config" + fi + unset cli_config + fi + # Read GPGDIR from $CONFIG. + # The pattern is: any spaces or tabs, GPGDir, any spaces or tabs, equal sign + # and the rest of the line. The string is splitted after the first occurrence of = + if [[ GPGDIR=$(find_config "GPGDir" &>/dev/null) ]]; then + PACMAN_KEYRING_DIR="${GPGDIR}" + fi +fi + +# Construct the command +GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning" + +# Execute the command 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 - # 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" - else - error "$(gettext "The key identified by %s doesn't exist")" "$1" - 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 ;; - *) - usage; exit 1 ;; + -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 + # 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" + else + error "$(gettext "The key identified by %s doesn't exist")" "$1" + 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 -- 1.7.4.2
On 30/03/11 07:22, ivan.kanak@gmail.com wrote:
From: Ivan Kanakarakis<ivan.kanak@gmail.com>
ok, I think this is much better. The process is as follows: The script first tries to read the options as defined in the usage. It then expects to read a command. If the command exists and it's one of --help/-h/--version/-V it skips any check and goes straight to the case loop, and thus executes those. If the command is something else, it enters the big if block and checks for the needed conditions.
also when an unknown command is given there now an error message instead of just the usage unknown command:<--the_command>
some more comments: I saw some things like error "$(gettext "The key identified by %s doesn't exist")" "$1" while error() only knows about the first argument given to it, so %s there is actually left blank. also I think there are some cases where gettext is redundant as in called twice for the same text. I can go fixing this too.
Signed-off-by: Ivan Kanakarakis<ivan.kanak@gmail.com>
I'll add a note that we also have this patch floating about that actually handles --config and --gpgdir properly. (among other changes). http://mailman.archlinux.org/pipermail/pacman-dev/2011-February/012515.html Allan
so it seems nobody merged anything yet, I got denis' patch, the one indicated by Allan, and applied it to master, then built mine on top. I actually didn't like his patch, most things were reverted back. I'll post my patches in a while, working my way with git-send-email.. On 30 March 2011 07:30, Allan McRae <allan@archlinux.org> wrote:
On 30/03/11 07:22, ivan.kanak@gmail.com wrote:
From: Ivan Kanakarakis<ivan.kanak@gmail.com>
ok, I think this is much better. The process is as follows: The script first tries to read the options as defined in the usage. It then expects to read a command. If the command exists and it's one of --help/-h/--version/-V it skips any check and goes straight to the case loop, and thus executes those. If the command is something else, it enters the big if block and checks for the needed conditions.
also when an unknown command is given there now an error message instead of just the usage unknown command:<--the_command>
some more comments: I saw some things like error "$(gettext "The key identified by %s doesn't exist")" "$1" while error() only knows about the first argument given to it, so %s there is actually left blank. also I think there are some cases where gettext is redundant as in called twice for the same text. I can go fixing this too.
Signed-off-by: Ivan Kanakarakis<ivan.kanak@gmail.com>
I'll add a note that we also have this patch floating about that actually handles --config and --gpgdir properly. (among other changes).
http://mailman.archlinux.org/pipermail/pacman-dev/2011-February/012515.html
Allan
-- Ivan c00kiemon5ter V Kanak http://c00kiemon5ter.github.com
On 20/04/11 06:04, Ivan Kanak wrote:
so it seems nobody merged anything yet, I got denis' patch, the one indicated by Allan, and applied it to master, then built mine on top. I actually didn't like his patch, most things were reverted back. I'll post my patches in a while, working my way with git-send-email..
I am going to be sitting around with little to do for the next day so I am intending to try and pull all these patches together. Allan
participants (3)
-
Allan McRae
-
Ivan Kanak
-
ivan.kanak@gmail.com