FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 04-12-2012, 02:54 PM
Dave Reisner
 
Default pacman-key: adopt parseopts for option parsing

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
" ${myver}
echo
- printf -- "$(gettext "Usage: %s [options]")
" $(basename $0)
+ printf -- "$(gettext "Usage: %s [options] operation [targets]")
" $(basename $0)
echo
printf -- "$(gettext "Manage pacman's list of trusted keys")
"
echo
- printf -- "$(gettext "Options:")
"
- printf -- "$(gettext " -a, --add [file(s)] Add the specified keys (empty for stdin)")
"
- printf -- "$(gettext " -d, --delete <keyid(s)> Remove the specified keyids")
"
- printf -- "$(gettext " -e, --export [keyid(s)] Export the specified or all keyids")
"
- printf -- "$(gettext " -f, --finger [keyid(s)] List fingerprint for specified or all keyids")
"
- printf -- "$(gettext " -h, --help Show this help message and exit")
"
- printf -- "$(gettext " -l, --list-keys [keyid(s)] List the specified or all keys")
"
- printf -- "$(gettext " -r, --recv-keys <keyid(s)> Fetch the specified keyids")
"
+ printf -- "$(gettext "Operations:")
"
+ printf -- "$(gettext " -a, --add Add the specified keys (empty for stdin)")
"
+ printf -- "$(gettext " -d, --delete Remove the specified keyids")
"
+ printf -- "$(gettext " -e, --export Export the specified or all keyids")
"
+ printf -- "$(gettext " -f, --finger List fingerprint for specified or all keyids")
"
+ printf -- "$(gettext " -l, --list-keys List the specified or all keys")
"
+ printf -- "$(gettext " -r, --recv-keys Fetch the specified keyids")
"
printf -- "$(gettext " -u, --updatedb Update the trustdb of pacman")
"
- printf -- "$(gettext " -v, --verify <signature> Verify the file specified by the signature")
"
- printf -- "$(gettext " -V, --version Show program version")
"
+ printf -- "$(gettext " -v, --verify Verify the file specified by the signature")
"
+ printf -- "$(gettext " --edit-key Present a menu for key management task on keyids")
"
+ printf -- "$(gettext " --import Imports pubring.gpg from dir(s)")
"
+ printf -- "$(gettext " --import-trustdb Imports ownertrust values from trustdb.gpg in dir(s)")
"
+ printf -- "$(gettext " --init Ensure the keyring is properly initialized")
"
+ printf -- "$(gettext " --list-sigs List keys and their signatures")
"
+ printf -- "$(gettext " --lsign-key Locally sign the specified keyid")
"
+ printf -- "$(gettext " --populate Reload the default keys from the (given) keyrings

+ in '%s'")
" "@pkgdatadir@/keyrings"
+ printf -- "$(gettext " --refresh-keys Update specified or all keys from a keyserver")
"
+ echo
+ printf -- "$(gettext "Options:")
"
printf -- "$(gettext " --config <file> Use an alternate config file (instead of

'%s')")
" "@sysconfdir@/pacman.conf"
- printf -- "$(gettext " --edit-key <keyid(s)> Present a menu for key management task on keyids")
"
printf -- "$(gettext " --gpgdir <dir> Set an alternate directory for GnuPG (instead

of '%s')")
" "@sysconfdir@/pacman.d/gnupg"
- printf -- "$(gettext " --import <dir(s)> Imports pubring.gpg from dir(s)")
"
- printf -- "$(gettext " --import-trustdb <dir(s)> Imports ownertrust values from trustdb.gpg in dir(s)")
"
- printf -- "$(gettext " --init Ensure the keyring is properly initialized")
"
- printf -- "$(gettext " --keyserver Specify a keyserver to use if necessary")
"
- printf -- "$(gettext " --list-sigs [keyid(s)] List keys and their signatures")
"
- printf -- "$(gettext " --lsign-key <keyid> Locally sign the specified keyid")
"
- printf -- "$(gettext " --populate [keyring(s)] Reload the default keys from the (given) keyrings

- in '%s'")
" "@pkgdatadir@/keyrings"
- printf -- "$(gettext " --refresh-keys [keyid(s)] Update specified or all keys from a keyserver")
"
+ printf -- "$(gettext " --keyserver <server-url> Specify a keyserver to use if necessary")
"
+ echo
+ printf -- "$(gettext " -h, --help Show this help message and exit")
"
+ printf -- "$(gettext " -V, --version Show program version")
"
}

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
y
' | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet --batch --lsign-key "${KEYIDS[@]}" 2>/dev/null
+ printf '%s
' 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
 
Old 04-13-2012, 12:31 PM
Dave Reisner
 
Default pacman-key: adopt parseopts for option parsing

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
y
' | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet
> --batch --lsign-key "${KEYIDS[@]}" 2>/dev/null
> > + printf '%s
' 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
>
>
>
 
Old 04-13-2012, 12:54 PM
Dave Reisner
 
Default pacman-key: adopt parseopts for option parsing

On Fri, Apr 13, 2012 at 8:39 AM, Allan McRae <allan@archlinux.org> wrote:

> 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
>
>
Ah, right. Yep, I can do that... looking back, seems I missed this in a
few places anyways (import, verify, etc)
 

Thread Tools




All times are GMT. The time now is 12:36 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org