[pacman-dev] [PATCH 6/7] pacman-key: fix parsing of command line options

[Ivan Kanak]

On 21 April 2011 20:08, Dan McGee <dpmcgee at gmail.com> wrote:

> On Thu, Apr 21, 2011 at 8:59 AM,  <ivan.kanak at gmail.com> wrote:
> > From: Ivan Kanakarakis <ivan.kanak at gmail.com>
> >
> > This commit fixes handling of --help/-h --version/-V options. Previously
> > $ pacman-key --help
> > would return errors if the configuration file was missing or the user
> > did not have root privileges. Same was for --version switch.
> > Moreover this fixes a bug that caused the command line specified
> > gpgdir (PACMAN_KEYRING_DIR) to be overridden by the values from the
> > configuration file.
> >
> > The parsing now happens as follows:
> > a. The script parses --config and/or --gpgdir if those are specified.
> > These options should be the first arguments given to the script.
> > b. The script reads the first command, that is the first argument after
> > the --config and/or --gpgdir parsing is done.
> > If no command was provided the script exits with an error.
> > If the command is one of --help -h --version -V then it skip the checks
> > that would otherwise be needed, executes those and exits.
> > If the command is other, the script checks by order:
> >  - the dependencies are met (currently existence of gpg executable)
> >  - the user has root privileges
> >  - the configuration file is readable
> >  - the gpgdir is set and exists
> > c. If all checks pass the command is matched to a case and the
> > appropriate set of commands is executed.
> >
> > Signed-off-by: Ivan Kanakarakis <ivan.kanak at gmail.com>
> > ---
> >  scripts/pacman-key.sh.in |   66
> +++++++++++++++++++++++++---------------------
> >  1 files changed, 36 insertions(+), 30 deletions(-)
> >
> > diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> > index b5fca2b..1d0229b 100644
> > --- a/scripts/pacman-key.sh.in
> > +++ b/scripts/pacman-key.sh.in
> > @@ -217,20 +217,11 @@ if ! type gettext &>/dev/null; then
> >        }
> >  fi
> >
> > -if [[ $1 != "--version" && $1 != "-V" && $1 != "--help" && $1 != "-h" &&
> $1 != "" ]]; 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
> > -       elif (( EUID != 0 )); then
> > -               error "$(gettext "pacman-key needs to be run as root.")"
> > -               exit 1
> > -       fi
> > -fi
> > +# Set default values
> > +CONFIG_DEF="@sysconfdir@/pacman.conf"
> > +PACMAN_KEYRING_DIR_DEF="@sysconfdir@/pacman.d/gnupg"
> >
> > -# Parse global options
> > -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" ;;
> > @@ -239,23 +230,7 @@ while [[ $1 =~ ^--(config|gpgdir)$ ]]; do
> >        shift
> >  done
> >
> > -if [[ ! -r "${CONFIG}" ]]; then
> > -       error "$(gettext "%s not found.")" "$CONFIG"
> > -       exit 1
> > -fi
> > -
> > -# Read GPGDIR from $CONFIG.
> > -if [[ GPGDIR="$(get_from "$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
> > @@ -263,6 +238,37 @@ if [[ -z "${command}" ]]; then
> >  fi
> >  shift
> >
> > +# If command is --help/-h or --version/-V then skip checks and execute
> those
> > +if [[ ! ${command} =~ ^(--help|-h|--version|-V)$ ]]; then
> > +    # check dependencies
> > +    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
> > +    # check permissions
> > +    if (( ! EUID )); then
> > +        error "$(gettext "pacman-key needs to be run as root.")"
> > +        exit 1
> > +    fi
> > +    # Use the default CONFIG if the user didn't specify one
> > +    # If configuration file is not readable use the default
> > +    # even if it was specified by the user with --config
> -1 from me. This makes no sense other than confusion for the user. If
> I specify a bogus or unreadable file, blow up on me. Fixing this to
> not be magic also means you don't need CONFIG_DEF at all either.
>
>

Oh, I agree on this. I had an error message there informing the user
that the configuration was unreadable and that the script would fall
back to the default. I missed it on this commit, sorry.



> > +    [[ ! -r $CONFIG ]] && CONFIG="$CONFIG_DEF"
> > +    # if the user didn't specify PACMAN_KEYRING_DIR
> > +    # then read GPGDir from the configuration file
> > +    [[ -z $PACMAN_KEYRING_DIR ]] && PACMAN_KEYRING_DIR="$(get_from
> "$CONFIG" "GPGDir")"
> > +    # if no such setting exists, use the default value
> > +    [[ -z $PACMAN_KEYRING_DIR ]] &&
> PACMAN_KEYRING_DIR="$PACMAN_KEYRING_DIR_DEF"
> > +    # Try to create $PACMAN_KEYRING_DIR if non-existent
> > +    # Check for simple existence rather than a directory
> > +    # as someone may want to use a symlink here
> > +    [[ ! -e ${PACMAN_KEYRING_DIR} ]] && mkdir -p -m 755
> "${PACMAN_KEYRING_DIR}"
> > +fi
> > +
> > +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
> > --
>
> Looks OK to me otherwise, but I'll let Allan weigh in as well.
>
> -Dan
>
>


-- 
Ivan c00kiemon5ter V Kanak
http://c00kiemon5ter.github.com