fix build with lld linker

[kraj]

was added this symbol comes from libgpg-error however, therefore its needed to add -lgpg-error to cmdline to resolve this symbol especially with gold and lld linker. Fixes

aarch64-yoe-linux-ld.lld: error: undefined reference due to --no-allow-shlib-undefined: gpg_strerror_r

referenced by ./.libs/libostree-1.so

[openshift-ci]

Hi @kraj. Thanks for your PR.

I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[dbnicholson]

/ok-to-test

[dbnicholson]

Hmm, strange that $(OT_INTERNAL_GPGME_LIBS) doesn't already have this. Oh, I think this is a small regression from dd98a2a. In the case that you don't have the gpgme pkg-config file then it falls back to using gpgme-config and gpg-error-config and ultimately does:

OT_DEP_GPGME_LIBS="${OT_DEP_GPGME_LIBS} ${OT_DEP_GPG_ERROR_LIBS}"

However, in the case that you do have the gpgme pkg-config file then it doesn't do that. I think the more appropriate fix is to actually explicitly include gpg-error in the initial gpgme pkg-config check since apparently this user's gpgme.pc doesn't include it and we're currently not doing anything with the checked value.

[dbnicholson]

@kraj could you try this change instead:

diff --git a/configure.ac b/configure.ac
index f7ae2d64b..5d6023660 100644
--- a/configure.ac
+++ b/configure.ac
@@ -242,9 +242,7 @@ AC_ARG_WITH(gpgme,
            AS_HELP_STRING([--with-gpgme], [Use gpgme @<:@default=yes@:>@]),
            [], [with_gpgme=yes])
 AS_IF([test x$with_gpgme != xno], [
-    have_gpgme=yes
-    PKG_CHECK_MODULES([OT_DEP_GPGME], gpgme >= $LIBGPGME_DEPENDENCY, [], have_gpgme=no)
-    PKG_CHECK_MODULES([OT_DEP_GPG_ERROR], [gpg-error], [], have_gpgme=no)
+    PKG_CHECK_MODULES([OT_DEP_GPGME], [gpgme >= $LIBGPGME_DEPENDENCY gpg-error], [have_gpgme=yes], [have_gpgme=no])
     ]
 )
 AS_IF([test x$with_gpgme != xno && test x$have_gpgme != xyes], [

[cgwalters]

But wouldn't that make us start hard failing on sytems without gpg-error?

[dbnicholson]

I don't think it will do anything different. All that does is merge the pkg-config checks, but you already would have fallen into the have_gpgme=no case if you didn't have gpg-error. That said, gpg-error is already a hard requirement for gpgme. I'm frankly surprised that pkg-config --libs gpgme doesn't include -lgpg-error for @kraj already. It does on my system, but this whole area has been built on downstream hacks for a long time since upstream wouldn't just DTRT. What I'm proposing above likely fixes the issue, but it's also more correct since libostree directly calls gpg_strerror_r from gpg-error when GPG support is in use.

[kraj]

@kraj could you try this change instead:

diff --git a/configure.ac b/configure.ac
index f7ae2d64b..5d6023660 100644
--- a/configure.ac
+++ b/configure.ac
@@ -242,9 +242,7 @@ AC_ARG_WITH(gpgme,
            AS_HELP_STRING([--with-gpgme], [Use gpgme @<:@default=yes@:>@]),
            [], [with_gpgme=yes])
 AS_IF([test x$with_gpgme != xno], [
-    have_gpgme=yes
-    PKG_CHECK_MODULES([OT_DEP_GPGME], gpgme >= $LIBGPGME_DEPENDENCY, [], have_gpgme=no)
-    PKG_CHECK_MODULES([OT_DEP_GPG_ERROR], [gpg-error], [], have_gpgme=no)
+    PKG_CHECK_MODULES([OT_DEP_GPGME], [gpgme >= $LIBGPGME_DEPENDENCY gpg-error], [have_gpgme=yes], [have_gpgme=no])
     ]
 )
 AS_IF([test x$with_gpgme != xno && test x$have_gpgme != xyes], [

The configure.ac change works for me too. The reason I am seeing it is because I am using lld linker which does not have bfd linker linker transitive dependency checker. it will fail with gold linker too perhaps in same way.

@dbnicholson do you want me to update this pull with your suggestion ?

[dbnicholson]

The configure.ac change works for me too. The reason I am seeing it is because I am using lld linker which does not have bfd linker linker transitive dependency checker. it will fail with gold linker too perhaps in same way.

That part makes sense. We definitely need -lgpg-error one way or another because the gpg_strerror_r symbol needs to be resolved from libostree. What I'm surprised about is how you were getting a link command that didn't already include it because pkg-config --libs gpgme should contain it.

Ah, now I see this upstream commit that changes from Requires to Requires.private. My installed version doesn't have that. That is correct but also does break libostree without this change as noted in the commit message:

It will break build of an application which directly uses gpg-error or libassuan without specifying them

So, yes, I think this was the correct fix for the issue.