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