On (08/07/14 09:27), Jakub Hrozek wrote:
>On Thu, Jun 26, 2014 at 10:31:27AM +0200, Lukas Slebodnik wrote:
>> ehlo,
>>
>> attached patch fixes ticket #2194.
>>
>> If you wan to know more about version script (version maps) here are links:
>>
>>
http://people.redhat.com/drepper/dsohowto.pdf
>> (sections 2.2.5 .. 2.2.7, 3.4, 3.5)
>>
https://www.gnu.org/software/gnulib/manual/html_node/LD-Version-Scripts.html
>>
ftp://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_node/ld_25.html
>>
>> LS
>
>I verified with:
> nm -D $libname | awk '$2 == "T" {print $3";"}' |
sort
>that the functions exported by the library match the functions defined
>in the version file.
>
>The versions also match the SONAMEs.
>
>One question inline:
>
>> From 0779624cf96480fe3b62f5c881fb9b123d24a965 Mon Sep 17 00:00:00 2001
>> From: Lukas Slebodnik <lslebodn(a)redhat.com>
>> Date: Mon, 9 Jun 2014 14:38:31 +0200
>> Subject: [PATCH] BUILD: Add version symbol files for public libraries.
>>
>> Version symbol files will help package systems to catch backward compatible
>> changes (newly added functions) into library.
>>
>> Resolves:
>>
https://fedorahosted.org/sssd/ticket/2194
>
>[...]
>
>> new file mode 100644
>> index
0000000000000000000000000000000000000000..bd3abb211120784494fe366ddd19b94d9b982657
>> --- /dev/null
>> +++ b/src/sss_client/idmap/sss_nss_idmap.exports
>> @@ -0,0 +1,17 @@
>> +SSS_NSS_IDMAP_0.0.1 {
>> +
>> + # public functions
>> + global:
>> +
>> + sss_nss_getsidbyname;
>> + sss_nss_getsidbyid;
>> + sss_nss_getnamebysid;
>> + sss_nss_getidbysid;
>> +
>> + # exported for mocking
>
>I don't think the comment is right. The symbol is exported because
>libsss_nss_idmap links with the client code:
>740 libsss_nss_idmap_la_SOURCES = \
>741 src/sss_client/idmap/sss_nss_idmap.c \
>742 src/sss_client/common.c \
>743 src/util/strtonum.c
>
>And libsss_nss_idmap uses the function to call out to the NSS responder
>(see src/sss_client/idmap/sss_nss_idmap.c:100)
>
>Can we not mark the function as global? It's not part of the sss_nss_idmap's
>API.
Try to comment out this symbol from file
--- a/src/sss_client/idmap/sss_nss_idmap.exports
+++ b/src/sss_client/idmap/sss_nss_idmap.exports
@@ -9,7 +9,7 @@ SSS_NSS_IDMAP_0.0.1 {
sss_nss_getidbysid;
# exported for mocking
- sss_nss_make_request;
+ # sss_nss_make_request;
# everything else is local
local:
sss_nss_idmap will fail, because function sss_nss_make_request cannot be mocked
sh-4.2$ cat sss_nss_idmap-tests.log
[==========] Running 1 test(s).
[ RUN ] test_getsidbyname
0x2 != 0
../sssd/src/tests/cmocka/sss_nss_idmap-tests.c:102: error: Failure!
[ FAILED ] test_getsidbyname
[==========] 1 test(s) run.
[ PASSED ] 0 test(s).
[ FAILED ] 1 test(s), listed below:
[ FAILED ] test_getsidbyname
We can build libsss_nss_idmap twice.
1) real library with version symbol file
2) library without version symbol file only for tests (sss_nss_idmap-tests)
I was not able to find better solution.
LS
I tried playing a bit with Makefile.am and the only viable solution I
see is to hardcode the list of libsss_nss_idmap sources in Makefile.am's
sss_nss_idmap_tests_SOURCES except sss_client/common.c and mock
sss_nss_lock and sss_nss_unlock..
But I'm not sure it's worth it, the rist is that a 3rd party links with
sss_nss_make_request (which has no declaration in the public headers).
So now I prefer to push your original patch, but I would also like to
hear other opinions, I'm not the biggest packaging expert around..