[pacman-dev] [PATCH 4/5] Parameter to select key to sign

[Allan McRae]

On 27/07/10 06:26, Denis A. Altoé Falqueto wrote:
> There is a possibility of another key being used, instead of
> the user's default. For exemple, the pacman-keyring package
> will be signed by a master key, because it needs to be trusted
> explicitly by the user before the installation of that package.
> So, the parameter --signwithkey will be used to supply an
> id of a key that will be used to sign a database or package.
>
> Signed-off-by: Denis A. Altoé Falqueto<denisfalqueto at gmail.com>
> ---
>   scripts/makepkg.sh.in  |   38 ++++++++++++++++++++--------
>   scripts/repo-add.sh.in |   63 +++++++++++++++++++++++++++++++++++-------------
>   2 files changed, 73 insertions(+), 28 deletions(-)


I would prefer this patch to be split into makepkg and repo-add parts 
and have the documentation patch similarly split and then the changes 
and documentation merged together.


> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 080e530..f6f9dfe 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -28,7 +28,7 @@
>   # makepkg uses quite a few external programs during its execution. You
>   # need to have at least the following installed for makepkg to function:
>   #   bsdtar (libarchive), bzip2, coreutils, fakeroot, find (findutils),
> -#   gettext, grep, gzip, openssl, sed, tput (ncurses), xz
> +#   gettext, grep, gzip, openssl, sed, tput (ncurses), xz, gpg

Try keeping this alphabetical.

>
>   # gettext initialization
>   export TEXTDOMAIN='pacman'
> @@ -43,6 +43,8 @@ BUILDSCRIPT='@BUILDSCRIPT@'
>   startdir="$PWD"
>   srcdir="$startdir/src"
>   pkgdir="$startdir/pkg"
> +GPG="gpg2"

Why change to gpg2?  In fact, I would prefer a separate patch that 
changes the "gpg" references to $GPG (=gpg by default) but allows the 
value to be overridden by environmental variables.  So if someone wants 
to use gpg2 they would do something like "GPG=gpg2 makepkg".

> +SIG_EXT=".sig"
We consider ".sig" hard-coded in pacman.  We should do the same here.

<snip>  otherwise makepkg changes are fine.

> diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> index 4ee63d8..ac734aa 100644
> --- a/scripts/repo-add.sh.in
> +++ b/scripts/repo-add.sh.in
> @@ -26,6 +26,8 @@ export TEXTDOMAINDIR='@localedir@'
>
>   myver='@PACKAGE_VERSION@'
>   confdir='@sysconfdir@'
> +GPG="gpg2"
> +SIG_EXT=".sig"

Same comments as applied to makepkg.

>   QUIET=0
>   SIGN=0
> @@ -62,8 +64,8 @@ error() {
>   # print usage instructions
>   usage() {
>   	printf "repo-add, repo-remove (pacman) %s\n\n" "$myver"
> -	printf "$(gettext "Usage: repo-add [-q] [-s] [-v]<path-to-db>  <package|delta>  ...\n")"
> -	printf "$(gettext "Usage: repo-remove [-q]<path-to-db>  <packagename|delta>  ...\n\n")"
> +	printf "$(gettext "Usage: repo-add [-q] [-s [-k|--signwithkey key]] [-v]<path-to-db>  <package|delta>  ...\n")"
> +	printf "$(gettext "Usage: repo-remove [-q] [-s [-k|--signwithkey key]]<path-to-db>  <packagename|delta>  ...\n\n")"

Just list the short option here as is done with all other options.

<snip>

> @@ -492,10 +505,24 @@ trap 'trap_exit "$(gettext "An unknown error has occured. Exiting...")"' ERR
>
>   success=0
>   # parse arguments
> -for arg in "$@"; do
> +while [[ $#>  0 ]] ; do
> +	arg="$1"
>   	case "$arg" in
>   		-q|--quiet) QUIET=1;;
> -		-s|--sign) SIGN=1;;
> +		-s|--sign)
> +			SIGN=1
> +			# The signature will be made, even if there are no operations
> +			success=1
> +			;;
> +		-k|--signwithkey)
> +			shift
> +			SIGNKEY="$1"
> +			# Check if key really exists

We have not done any checks for the gpg binary at this stage...

> +			if ! ${GPG} --list-key ${SIGNKEY} 1>  /dev/null 2>&1; then
> +				error "$(gettext "Cannot find key $SIGNKEY.")"
> +				exit 1
> +			fi
> +			;;
>   		-v|--verify) VERIFY=1;;
>   		*)
>   			if [[ -z $REPO_DB_FILE ]]; then
> @@ -510,6 +537,7 @@ for arg in "$@"; do
>   			fi
>   			;;
>   	esac
> +	shift
>   done
>
>   # if at least one operation was a success, re-zip database
> @@ -529,18 +557,19 @@ if (( success )); then
>   	cd "$tmpdir"
>   	if [[ -n $(ls) ]]; then
>   		bsdtar -c${TAR_OPT}f "$filename" *
> -		create_signature "$filename"
>   	else
>   		# we have no packages remaining? zip up some emptyness
>   		warning "$(gettext "No packages remain, creating empty database.")"
>   		bsdtar -c${TAR_OPT}f "$filename" -T /dev/null
>   	fi
> +	# The signature must be dealt with in both cases, empty repo or not.
> +	create_signature "$filename"

Good catch