[SSSD] [PATCH] BUILD: Add version symbol files for public libraries.

Lukas Slebodnik lslebodn at redhat.com
Tue Jul 8 21:04:27 UTC 2014


On (08/07/14 15:19), Sumit Bose wrote:
>On Tue, Jul 08, 2014 at 12:51:48PM +0200, Jakub Hrozek wrote:
>> On Tue, Jul 08, 2014 at 11:21:09AM +0200, Lukas Slebodnik wrote:
>> > 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 at 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..
>
>While discussing this on irc with Andreas (cmocka author) and Jakub we
>came up with
>
>SSS_NSS_IDMAP_0.0.1 {
>
>    # public functions
>    global:
>
>        sss_nss_getsidbyname;
>        sss_nss_getsidbyid;
>        sss_nss_getnamebysid;
>        sss_nss_getidbysid;
>
>    # everything else is local
>    local:
>        *;
>};
>
>UNIT_TESTING_ONLY {
>    global:
>        sss_nss_make_request;
>};
>
>which would move the sss_nss_make_request into a separate version which
>is not for public use. RPMs will still show a Provides like:
>
>libsss_nss_idmap.so.0(UNIT_TESTING_ONLY)
>
>which can be filtered out (at least on recent Fedora version) with
>
>%global __provides_exclude ^.*FOR_CMOCKA.*$
>
>in the spec file.
This change would only hide provides from rpm, which is not platform
independent. I tried build libsss_nss_idmap.so in different way for
test and for installation with custom make targets (check-local,
install-exec-local, install-exec-hook). It was not possible because
I would need targets which are executed before check or install and int the
middle or after targets (check or install).

I decided to build library libsss_nss_idmap twice.
    1) libsss_nss_idmap_test.so --will be used for sss_nss_idmap-test
    2) libsss_nss_idmap.so --will be installed

Two version script files were combined for libsss_nss_idmap_test.so
defualt sss_nss_idmap.exports and extra sss_nss_idmap.unit_tests, which
contains function sss_nss_make_request.

LS
-------------- next part --------------
>From 99cd145f34b7d18e8982b92a77d7b10daacbb955 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at 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.

The difference between libraries libsss_nss_idmap_test.so and
libsss_nss_idmap.so is that the 1st library will not be installed and has more
exported functions, which are necessary for mocking with cmocka for test
sss_nss_idmap-test.

Resolves:
https://fedorahosted.org/sssd/ticket/2194
---
 Makefile.am                                   | 31 +++++++++++++--
 src/lib/idmap/sss_idmap.exports               | 56 +++++++++++++++++++++++++++
 src/providers/ipa/ipa_hbac.exports            | 15 +++++++
 src/sss_client/idmap/sss_nss_idmap.exports    | 14 +++++++
 src/sss_client/idmap/sss_nss_idmap.unit_tests |  6 +++
 5 files changed, 119 insertions(+), 3 deletions(-)
 create mode 100644 src/lib/idmap/sss_idmap.exports
 create mode 100644 src/providers/ipa/ipa_hbac.exports
 create mode 100644 src/sss_client/idmap/sss_nss_idmap.exports
 create mode 100644 src/sss_client/idmap/sss_nss_idmap.unit_tests

diff --git a/Makefile.am b/Makefile.am
index 1c423fc6d40754852a9775dc72016703e398f17c..a44969ba79acf0c63115bc6ff7da0abdd45f0c46 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -711,23 +711,32 @@ endif # BUILD_IFP
 
 lib_LTLIBRARIES = libipa_hbac.la libsss_idmap.la libsss_nss_idmap.la
 pkgconfig_DATA += src/providers/ipa/ipa_hbac.pc
+libipa_hbac_la_DEPENDENCIES = src/providers/ipa/ipa_hbac.exports
 libipa_hbac_la_SOURCES = \
     src/providers/ipa/hbac_evaluator.c \
     src/util/sss_utf8.c
 libipa_hbac_la_LIBADD = \
     $(UNICODE_LIBS)
 libipa_hbac_la_LDFLAGS = \
+    -Wl,--version-script,$(srcdir)/src/providers/ipa/ipa_hbac.exports \
     -version-info 0:1:0
 
+dist_noinst_DATA += src/providers/ipa/ipa_hbac.exports
+
 pkgconfig_DATA += src/lib/idmap/sss_idmap.pc
+libsss_idmap_la_DEPENDENCIES = src/lib/idmap/sss_idmap.exports
 libsss_idmap_la_SOURCES = \
     src/lib/idmap/sss_idmap.c \
     src/lib/idmap/sss_idmap_conv.c \
     src/util/murmurhash3.c
 libsss_idmap_la_LDFLAGS = \
+    -Wl,--version-script,$(srcdir)/src/lib/idmap/sss_idmap.exports \
     -version-info 4:0:4
 
+dist_noinst_DATA += src/lib/idmap/sss_idmap.exports
+
 pkgconfig_DATA += src/sss_client/idmap/sss_nss_idmap.pc
+libsss_nss_idmap_la_DEPENDENCIES = src/sss_client/idmap/sss_nss_idmap.exports
 libsss_nss_idmap_la_SOURCES = \
     src/sss_client/idmap/sss_nss_idmap.c \
     src/sss_client/common.c \
@@ -735,8 +744,12 @@ libsss_nss_idmap_la_SOURCES = \
 libsss_nss_idmap_la_LIBADD = \
     $(CLIENT_LIBS)
 libsss_nss_idmap_la_LDFLAGS = \
+    -Wl,--version-script,$(srcdir)/src/sss_client/idmap/sss_nss_idmap.exports \
     -version-info 0:1:0
 
+dist_noinst_DATA += src/sss_client/idmap/sss_nss_idmap.exports
+
+
 include_HEADERS = \
     src/providers/ipa/ipa_hbac.h \
     src/lib/idmap/sss_idmap.h \
@@ -1105,7 +1118,9 @@ libsss_test_common_la_SOURCES += \
     src/tests/common_check.c
 
 check_LTLIBRARIES += \
-    libdlopen_test_providers.la
+    libdlopen_test_providers.la \
+    libsss_nss_idmap_tests.la \
+    $(NULL)
 
 libdlopen_test_providers_la_SOURCES = \
     $(sssd_be_SOURCES)
@@ -1125,6 +1140,16 @@ libdlopen_test_providers_la_LDFLAGS = \
     -rpath $(abs_top_builddir) \
     -export-dynamic
 
+libsss_nss_idmap_tests_la_SOURCES = $(libsss_nss_idmap_la_SOURCES)
+libsss_nss_idmap_tests_la_LIBADD = $(libsss_nss_idmap_la_LIBADD)
+libsss_nss_idmap_tests_la_LDFLAGS = \
+    $(libsss_nss_idmap_la_LDFLAGS) \
+    -shared \
+    -rpath $(libdir) \
+    -Wl,--version-script,$(srcdir)/src/sss_client/idmap/sss_nss_idmap.unit_tests
+
+dist_noinst_DATA += src/sss_client/idmap/sss_nss_idmap.unit_tests
+
 dlopen_tests_SOURCES = \
     src/tests/dlopen-tests.c
 dlopen_tests_CFLAGS = \
@@ -1664,7 +1689,8 @@ sss_nss_idmap_tests_CFLAGS = \
     $(AM_CFLAGS)
 sss_nss_idmap_tests_LDADD = \
     $(CMOCKA_LIBS) \
-    libsss_nss_idmap.la
+    libsss_nss_idmap_tests.la \
+    $(NULL)
 
 EXTRA_dyndns_tests_DEPENDENCIES = \
      $(ldblib_LTLIBRARIES)
@@ -2689,4 +2715,3 @@ prerelease-srpm:
 	$(MAKE) srpm
 	mv $(srcdir)/version.m4.orig $(srcdir)/version.m4
 endif
-
diff --git a/src/lib/idmap/sss_idmap.exports b/src/lib/idmap/sss_idmap.exports
new file mode 100644
index 0000000000000000000000000000000000000000..52115636d5a6b936f18b4392e9d12adc26c85f53
--- /dev/null
+++ b/src/lib/idmap/sss_idmap.exports
@@ -0,0 +1,56 @@
+SSS_IDMAP_0.4 {
+
+    # public functions
+    global:
+
+        sss_idmap_init;
+        sss_idmap_ctx_set_autorid;
+        sss_idmap_ctx_set_lower;
+        sss_idmap_ctx_set_upper;
+        sss_idmap_ctx_set_rangesize;
+        sss_idmap_ctx_get_autorid;
+        sss_idmap_ctx_get_lower;
+        sss_idmap_ctx_get_upper;
+        sss_idmap_ctx_get_rangesize;
+        sss_idmap_calculate_range;
+        sss_idmap_add_domain;
+        sss_idmap_add_domain_ex;
+        sss_idmap_check_collision;
+        sss_idmap_check_collision_ex;
+        sss_idmap_sid_to_unix;
+        sss_idmap_dom_sid_to_unix;
+        sss_idmap_bin_sid_to_unix;
+        sss_idmap_smb_sid_to_unix;
+        sss_idmap_check_sid_unix;
+        sss_idmap_check_dom_sid_to_unix;
+        sss_idmap_check_bin_sid_unix;
+        sss_idmap_check_smb_sid_unix;
+        sss_idmap_unix_to_sid;
+        sss_idmap_unix_to_dom_sid;
+        sss_idmap_unix_to_bin_sid;
+        sss_idmap_free;
+        sss_idmap_free_sid;
+        sss_idmap_free_dom_sid;
+        sss_idmap_free_smb_sid;
+        sss_idmap_free_bin_sid;
+        idmap_error_string;
+        is_domain_sid;
+        sss_idmap_domain_has_algorithmic_mapping;
+        sss_idmap_domain_by_name_has_algorithmic_mapping;
+        sss_idmap_bin_sid_to_dom_sid;
+        sss_idmap_bin_sid_to_sid;
+        sss_idmap_dom_sid_to_bin_sid;
+        sss_idmap_sid_to_bin_sid;
+        sss_idmap_dom_sid_to_sid;
+        sss_idmap_sid_to_dom_sid;
+        sss_idmap_sid_to_smb_sid;
+        sss_idmap_smb_sid_to_sid;
+        sss_idmap_dom_sid_to_smb_sid;
+        sss_idmap_smb_sid_to_dom_sid;
+        sss_idmap_bin_sid_to_smb_sid;
+        sss_idmap_smb_sid_to_bin_sid;
+
+    # everything else is local
+    local:
+        *;
+};
diff --git a/src/providers/ipa/ipa_hbac.exports b/src/providers/ipa/ipa_hbac.exports
new file mode 100644
index 0000000000000000000000000000000000000000..0115084e2b3a66569f97c4e7c035dffdb6450b43
--- /dev/null
+++ b/src/providers/ipa/ipa_hbac.exports
@@ -0,0 +1,15 @@
+IPA_HBAC_0.0.1 {
+
+    # public functions
+    global:
+
+        hbac_evaluate;
+        hbac_result_string;
+        hbac_error_string;
+        hbac_free_info;
+        hbac_rule_is_complete;
+
+    # everything else is local
+    local:
+        *;
+};
diff --git a/src/sss_client/idmap/sss_nss_idmap.exports b/src/sss_client/idmap/sss_nss_idmap.exports
new file mode 100644
index 0000000000000000000000000000000000000000..7b8488b2ae1ca0d562d2d782e0bd6965b3beaecf
--- /dev/null
+++ b/src/sss_client/idmap/sss_nss_idmap.exports
@@ -0,0 +1,14 @@
+SSS_NSS_IDMAP_0.0.1 {
+
+    # public functions
+    global:
+
+        sss_nss_getsidbyname;
+        sss_nss_getsidbyid;
+        sss_nss_getnamebysid;
+        sss_nss_getidbysid;
+
+    # everything else is local
+    local:
+        *;
+};
diff --git a/src/sss_client/idmap/sss_nss_idmap.unit_tests b/src/sss_client/idmap/sss_nss_idmap.unit_tests
new file mode 100644
index 0000000000000000000000000000000000000000..361cc3b134ead52cf458afe27c055739d6728441
--- /dev/null
+++ b/src/sss_client/idmap/sss_nss_idmap.unit_tests
@@ -0,0 +1,6 @@
+# version script files can be combined. They needn't be in single file
+UNIT_TEST_ONLY {
+    # should not be part of installed library
+    global:
+        sss_nss_make_request;
+};
-- 
1.9.3



More information about the sssd-devel mailing list