[pacman-dev] [PATCH] Fix compilation without gpgme

[Dan McGee]

On Thu, Jan 2, 2014 at 4:07 PM, Allan McRae <allan at archlinux.org> wrote:

> On 03/01/14 04:37, Dan McGee wrote:
> > Two issues have snuck in that prevent the compile from working.
> >
> > Signed-off-by: Dan McGee <dan at archlinux.org>
> > ---
> >
> > This and the rest of the patches available on my 'random-fixes' branch.
> >
>
>
>
> >  lib/libalpm/sync.c   | 2 +-
> >  src/pacman/package.c | 8 +++++---
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> > index e358585..9531fa2 100644
> > --- a/lib/libalpm/sync.c
> > +++ b/lib/libalpm/sync.c
> > @@ -1225,7 +1225,7 @@ int _alpm_sync_commit(alpm_handle_t *handle,
> alpm_list_t **data)
> >               return -1;
> >       }
> >
> > -#if HAVE_LIBGPGME
> > +#ifdef HAVE_LIBGPGME
>
> I not that #if and #ifdef are both used in many places.
>
I think you want to stick with one of two approaches:

* #if defined(HAVE_LIBGPGME)
* #ifdef HAVE_LIBGPGME

We seem to use both of these; however we don't use the `#if <symbol>`
approach anywhere outside of signing.c. I agree that all of them in
signing.c should be brought into compliance, not sure why I only changed
one.


>
> $ git grep "#if HAVE_"
> lib/libalpm/signing.c:#if HAVE_LIBGPGME
> lib/libalpm/signing.c:#if HAVE_LIBGPGME
> lib/libalpm/signing.c:#if HAVE_LIBGPGME
> lib/libalpm/sync.c:#if HAVE_LIBGPGME
> m4/acinclude.m4:#if HAVE_SYS_UCRED_H
>
> Should all these change?
>
commit 87ffc648b7bce35d shows that we use #ifdef and not #if in sync.c; are
we looking at different codebases?


> >       /* make sure all required signatures are in keyring */
> >       if(check_keyring(handle)) {
> >               return -1;
> > diff --git a/src/pacman/package.c b/src/pacman/package.c
> > index 52219ff..c44696b 100644
> > --- a/src/pacman/package.c
> > +++ b/src/pacman/package.c
> > @@ -193,6 +193,10 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra)
> >       }
> >
> >       if(from == ALPM_PKG_FROM_SYNCDB && extra) {
> > +             string_display(_("MD5 Sum        :"),
> alpm_pkg_get_md5sum(pkg), cols);
> > +             string_display(_("SHA-256 Sum    :"),
> alpm_pkg_get_sha256sum(pkg), cols);
> > +
> > +#ifdef HAVE_LIBGPGME
>
>
> I went for the approach of exposing alpm_decode_signature even if GPGME
> is unavailable, because it does not require any GPGME functions and it
> could still be useful for a frontend:
> https://patchwork.archlinux.org/patch/1790/
>
> I like your approach, that makes more sense and means frontend code
doesn't have to know how the backend library was built.


> >               const char *base64_sig = alpm_pkg_get_base64_sig(pkg);
> >               alpm_list_t *keys = NULL;
> >               if(base64_sig) {
> > @@ -204,10 +208,8 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra)
> >               } else {
> >                       keys = alpm_list_add(keys, _("None"));
> >               }
> > -
> > -             string_display(_("MD5 Sum        :"),
> alpm_pkg_get_md5sum(pkg), cols);
> > -             string_display(_("SHA-256 Sum    :"),
> alpm_pkg_get_sha256sum(pkg), cols);
> >               list_display(_("Signatures     :"), keys, cols);
> > +#endif
> >       } else {
> >               list_display(_("Validated By   :"), validation, cols);
> >       }
> >
>
>
> Given the overlap in our changes and thoughts here, do you want me to
bother resubmitting this? I can go back and review your patches if that
makes more sense, let me know.

-Dan