[pacman-dev] [PATCH 3/3] pacman-key: better handling of options and supressing gpg output

[Allan McRae]

On 19/02/11 11:30, Denis A. Altoé Falqueto wrote:
> The option --trus was changed to --edit-key, for better alignment
> with the underlying --edit-key of gnupg.
>
> The options --config and --gpgdir were not being handled correctly.
> They would not work if were not used as first arguments always.
> Now the handling is more flexible.
>
> The use of gpg for verification purposes was leaking inconvenient
> messages to the output, so they were quieted with --quiet,
> 1>/dev/null and 2>&1.
>
> Signed-off-by: Denis A. Altoé Falqueto<denisfalqueto at gmail.com>
> ---
>   doc/pacman-key.8.txt     |    4 +-
>   scripts/pacman-key.sh.in |   55 ++++++++++++++++++++++++++++++----------------
>   2 files changed, 38 insertions(+), 21 deletions(-)
>
> diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt
> index 5ebbd0a..ba97b82 100644
> --- a/doc/pacman-key.8.txt
> +++ b/doc/pacman-key.8.txt
> @@ -59,8 +59,8 @@ Commands
>   *\--reload*::
>   	Reloads the keys from the keyring package
>
> -*-t*, *\--trust* 'keyid'::
> -	Set the trust level of the given key
> +*-t*, *\--edit-key* 'keyid ...'::
> +	Edit trust properties for the given keys
>
>   *-u*, *\--updatedb*::
>   	Equivalent to \--check-trustdb in GnuPG
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index ccaf4b2..d97b071 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -71,7 +71,7 @@ usage() {
>   	echo "$(gettext "  -l | --list                            - list keys")"
>   	echo "$(gettext "  -r | --receive<keyserver>  <keyid>  ... - fetch the keyids from the specified")"
>   	echo "$(gettext "                                           keyserver URL")"
> -	echo "$(gettext "  -t | --trust<keyid>  ...               - set the trust level of the given key")"
> +	echo "$(gettext "  -t | --edit-key<keyid>  ...            - edit trust properties for the given keys")"
>   	echo "$(gettext "  -u | --updatedb                        - update the trustdb of pacman")"
>   	echo "$(gettext "  -v | --version                         - displays the current version")"
>   	echo "$(gettext "  --adv<params>                          - use pacman's keyring as target for")"
> @@ -117,7 +117,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} --quiet --verify "${ADDED_KEYS}.sig" 1>/dev/null 2>&1; then

using "&>/dev/null" would be cleaner.  And given --quiet is obviously 
not doing much, should we just remove it?

As an aside, the man page for gpg says --verify should "verify it 
without generating any output".  Clearly there is output...

>   			error "$(gettext "The signature of file %s is not valid.")" "${ADDED_KEYS}"
>   			exit 1
>   		fi
> @@ -125,7 +125,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} --quiet --verify "${DEPRECATED_KEYS}.sig" 1>/dev/null 2>&1; then
>   			error "$(gettext "The signature of file %s is not valid.")" "${DEPRECATED_KEYS}"
>   			exit 1
>   		fi
> @@ -133,7 +133,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} --quiet --verify "${REMOVED_KEYS}.sig" 1>/dev/null 2>&1; then
>   			error "$(gettext "The signature of file %s is not valid.")" "${REMOVED_KEYS}"
>   			exit 1
>   		fi
> @@ -229,15 +229,27 @@ if [[ $1 != "--version"&&  $1 != "-v"&&  $1 != "--help"&&  $1 != "-h"&&  $1 != "
>   	fi
>   fi
>
> -# Parse global options
> +# Iterate over the parameters to get --config and --gpgdir
> +# This time, the parameters will not be consumed. This is needed
> +# because the code needs to know where is pacman's keyring before
> +# signing or verifying any files.
>   CONFIG="@sysconfdir@/pacman.conf"
> -PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg"
> -while [[ $1 =~ ^--(config|gpgdir)$ ]]; do
> -	case "$1" in
> -		--config) shift; CONFIG="$1" ;;
> -		--gpgdir) shift; PACMAN_KEYRING_DIR="$1" ;;
> +GPGDIR=""
> +isconfig=0
> +isgpgdir=0
> +for arg in "$@"; do
> +	if (( isconfig )); then
> +		isconfig=0
> +		CONFIG="$arg"
> +	fi
> +	if (( isgpgdir )); then
> +		isgpgdir=0
> +		GPGDIR="$arg"
> +	fi
> +	case "$arg" in
> +		--config) isconfig=1;;
> +		--gpgdir) isgpgdir=1;;


This leaves --config and --gpgdir in "$@".  So if I run (e.g.)
"pacman-key --delete <keyid> --config <file>"

Then the command run will be:

${GPG_PACMAN} --quiet --batch --delete-key --yes "$@"

where "$@" is expanded to "<keyid> --config <file>", which clearly is 
bad...  So this needs to be slightly more clever.


Allan