[pacman-dev] [PATCH] GPG signature option in makepkg

[Dan McGee]

On Sun, Jun 1, 2008 at 8:04 PM,  <geoffroy.carrier at koon.fr> wrote:
> From: Geoffroy Carrier <geoffroy.carrier at koon.fr>
>
Please add at least a line or two of comments for your next submit,
but you already let me know you just forgot here. :)

> ---
>  scripts/makepkg.sh.in |   24 ++++++++++++++++++------
>  1 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 6e2f1ad..1314a51 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -54,6 +54,7 @@ INFAKEROOT=0
>  GENINTEG=0
>  INSTALL=0
>  NOBUILD=0
> +SIGN=0
>  NODEPS=0
>  NOEXTRACT=0
>  RMDEPS=0
> @@ -855,6 +856,19 @@ create_package() {
>        fi
>  }
>
> +create_signature() {
> +       [ "$SIGN" = "0" ] && return
> +       msg "$(gettext "Signing package...")"
> +       if [ ! $(type -p "gpg") ]; then
> +               error "$(gettext "Cannot find the gpg binary! Is gnupg installed?")"
> +               exit 1 # $E_MISSING_PROGRAM
Hmm, maybe a warning rather than an error here as the package build
itself did not fail? This is similar to the patch we just added so
that failure to uninstall deps does not result in a failure code being
returned.

> +       fi
> +       if ! gpg --detach-sign "$PKGDEST/${pkgname}-${pkgver}-${pkgrel}-${CARCH}${PKGEXT}"; then
Minor issue, but we tend to use this format around makepkg:
local ret = 0
gpg ..... || ret=$?
if [ $ret -eq 0 ]; then
  # success
else
  #failure
fi

That way we can also have a:
msg2 "$(gettext "Created signature file %s.")" $filename.sig

I can't believe we don't have a variable for that file name, hmm. We
use it an awful lot.

> +               error "$(gettext "Failed to sign package file.")"
> +               # exit 1 # It's easy to sign manually, so doesn't seem a big failure to gcarrier.
Yeah, I'd drop this to a warning. I think it will be more clear in a second.

> +       fi
> +}
> +
>  create_xdelta() {
>        if [ "$(check_buildenv xdelta)" != "y" ]; then
>                return
> @@ -1127,7 +1141,7 @@ SRCDEST=${SRCDEST:-$startdir} #default to $startdir if undefined
>  # Parse Command Line Options.
>  OPT_SHORT="AbcCdefFghiLmop:rRsSV"
>  OPT_LONG="ignorearch,asroot,builddeps,clean,cleancache,nodeps,noextract,force,forcever:,geninteg,help,holdver"
> -OPT_LONG="$OPT_LONG,install,log,nocolor,nobuild,rmdeps,repackage,source,syncdeps,usesudo,version"
> +OPT_LONG="$OPT_LONG,install,log,nocolor,nobuild,rmdeps,repackage,source,syncdeps,sign,version"
>  # Pacman Options
>  OPT_LONG="$OPT_LONG,noconfirm,noprogressbar"
>  OPT_TEMP="$(getopt -o "$OPT_SHORT" -l "$OPT_LONG" -n "$(basename "$0")" -- "$@" || echo 'GETOPT GO BANG!')"
> @@ -1166,11 +1180,7 @@ while true; do
>                -R|--repackage)   REPKG=1 ;;
>                --source)         SOURCEONLY=1 ;;
>                -s|--syncdeps)    DEP_BIN=1 ;;
> -
> -               # BEGIN DEPRECATED
> -               -S|--usesudo)
> -                       warning "$(gettext "Sudo is used by default now. The --usesudo option is deprecated!")" ;;
> -               # END DEPRECATED
> +               -S|--sign)        SIGN=1 ;;
Rather than do this as an option, why don't we add it as a buildenv
option so we don't have to manually specify -S every time? This is how
xdelta, distcc, ccache, etc. work now and I think it would be the best
way to go. That way a developer could easily enable or disable the
signing of all packages in one place.

>
>                -h|--help)        usage; exit 0 ;; # E_OK
>                -V|--version)     version; exit 0 ;; # E_OK
> @@ -1494,6 +1504,8 @@ fi
>
>  msg "$(gettext "Finished making: %s")" "$pkgname $pkgver-$pkgrel $CARCH ($(date))"
>
> +create_signature
> +
>  install_package
>
>  exit 0 #E_OK
> --

Thanks for starting the whole signed packages thing. We have to do it
one step at a time, and this is definitely the best place to start.

-Dan