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

[Allan McRae]

On 10/01/14 01:55, Dan McGee wrote:
> 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.
> 

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