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

On 10/01/14 01:55, Dan McGee wrote:

On Thu, Jan 2, 2014 at 4:07 PM, Allan McRae 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 ---

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.

Great. #ifdef it is then.

...

$ 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?

Probably the codebase with my alternative patch to this issue.

...

...

/* 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.

My patch was rather simple, so no need (unless you are bored...). I'll add the extra #if -> #ifdef. Allan