Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 196401 (mozldap)
Summary: | Review Request: mozldap | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Rich Megginson <rmeggins> | ||||||
Component: | Package Review | Assignee: | Dennis Gilmore <dennis> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | CC: | axel.thimm, nkinder, seg, sundaram | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2007-01-09 22:15:08 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | 196393 | ||||||||
Bug Blocks: | 163779, 219869 | ||||||||
Attachments: |
|
Description
Rich Megginson
2006-06-22 23:50:43 UTC
181691 is private; we cannot view it. Sorry. Here is an excerpt that explains the differences between openldap and mozldap: [The reason why we can just replace openldap with mozldap] "... is that there are dozens of apps that require the openldap client libraries. This is a problem for two reasons: 1) the openldap api is different than the mozilla ldap api, in terms of the function parameters, constants, and behavior. In many cases it is only slightly different, and we might be able to mitigate this by either writing a shim layer on top of the mozilla ldap c sdk that makes it look like openldap, or by simply changing the mozilla ldap c sdk to make it like openldap. I prefer the latter. 2) openldap clients expect to be able to use openssl for crypto, while mozilla uses NSS. I don't know of an easy way around this, except perhaps to just automatically import CA certs listed in /etc/openldap/ldap.conf to an NSS cert db. Similarly for clients that use cert based auth, import their keys/certs/password files." mozldap depends on svrcore-devel I don't have sponsor status, but can do the review and then kick someone who does have sponsor status when the package is approved. Initial feedback on first pass through the spec and some rpmlint'ing: 1) Should add %dist tag 2) Source0: should be a URL, if not, explain why 3) Remove useless Provides: 4) Requires: on sub-packages should be explicit %{name} = %{version}-%{release} instead of >= 5) Use %configure instead of ./configure 6) Should quote around "$RPM_OPT_FLAGS", I've seen issues if not done 7) %install is missing buildroot cleaning 8) don't create directories/files in the buildroot in %build, needs to be done in %install (since the first thing in %install is supposed to be a buildroot purging). 9) extraneous slashes in some path names (ex: $RPM_BUILD_ROOT/%{_libdir}) 10) standard practice in Fedora is to symlink back to the actual .so rather than create a trail of symlinks 11) preferred ldconfig post/postun format is "%post -p /sbin/ldconfig" 12) %defattr should be (-,root,root,-) 13) lots of extra, unnecessary %dir lines in various %files sections 14) unversioned .so files must go in -devel package when there are also versioned .so's 15) rpmlint complains about invalid sonames, I presume this is a side-effect of renaming them: E: mozldap invalid-soname /usr/lib64/libssldap-5.0.so.5.17 libssldap50.so E: mozldap invalid-soname /usr/lib64/libprldap-5.0.so.5.17 libprldap50.so E: mozldap invalid-soname /usr/lib64/libldap-5.0.so.5.17 libldap50.so 16) the binaries all define rpaths, which is a big no-no The following spec diff should address all but 15 and 16: ---------- --- mozldap-orig.spec 2006-07-13 15:58:24.000000000 -0400 +++ mozldap.spec 2006-07-13 16:50:58.000000000 -0400 @@ -10,7 +10,7 @@ Summary: Mozilla LDAP C SDK Name: mozldap Version: %{major}.%{minor} -Release: 3 +Release: 3%{?dist} License: MPL/GPL/LGPL URL: http://www.mozilla.org/directory/csdk.html Group: System Environment/Libraries @@ -20,8 +20,8 @@ BuildRequires: %{nspr_name}-devel >= %{nspr_version} BuildRequires: %{nss_name}-devel >= %{nss_version} BuildRequires: %{svrcore_name} >= %{svrcore_version} -Provides: mozldap +# Only available from cvs, tag LDAPCSDK_5_1_7_RTM Source0: %{name}-%{version}.tar.gz %description @@ -35,11 +35,10 @@ %package tools Summary: Tools for the Mozilla LDAP C SDK Group: System Environment/Base -Requires: %{name} >= %{version}-%{release} +Requires: %{name} = %{version}-%{release} BuildRequires: %{nspr_name}-devel >= %{nspr_version} BuildRequires: %{nss_name}-devel >= %{nss_version} BuildRequires: %{svrcore_name} >= %{svrcore_version} -Provides: %{name}-tools %description tools The mozldap-tools package provides the ldapsearch, @@ -50,10 +49,9 @@ %package devel Summary: Development libraries and examples for Mozilla LDAP C SDK Group: Development/Libraries -Requires: %{name} >= %{version}-%{release} +Requires: %{name} = %{version}-%{release} Requires: %{nspr_name}-devel >= %{nspr_version} Requires: %{nss_name}-devel >= %{nss_version} -Provides: %{name}-devel %description devel Header and Library files for doing development with the Mozilla LDAP C SDK @@ -64,7 +62,8 @@ arg64="--enable-64bit" %endif cd mozilla/directory/c-sdk -./configure $arg64 --with-system-svrcore --enable-optimize --disable-debug +%configure $arg64 --with-system-svrcore --enable-optimize --disable-debug +#./configure $arg64 --with-system-svrcore --enable-optimize --disable-debug %build @@ -73,7 +72,7 @@ export BUILD_OPT # Generate symbolic info for debuggers -XCFLAGS=$RPM_OPT_FLAGS +XCFLAGS="$RPM_OPT_FLAGS" export XCFLAGS PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 @@ -90,9 +89,12 @@ cd mozilla/directory/c-sdk make BUILDCLU=1 HAVE_SVRCORE=1 BUILD_OPT=1 +%install +%{__rm} -rf $RPM_BUILD_ROOT + # Set up our package file -%{__mkdir_p} $RPM_BUILD_ROOT/%{_libdir}/pkgconfig -%{__cat} %{name}.pc.in | sed -e "s,%%libdir%%,%{_libdir},g" \ +%{__mkdir_p} $RPM_BUILD_ROOT%{_libdir}/pkgconfig +%{__cat} mozilla/directory/c-sdk/%{name}.pc.in | sed -e "s,%%libdir%%,%{_libdir},g" \ -e "s,%%prefix%%,%{_prefix},g" \ -e "s,%%exec_prefix%%,%{_prefix},g" \ -e "s,%%includedir%%,%{_includedir}/%{name},g" \ @@ -100,69 +102,63 @@ -e "s,%%NSS_VERSION%%,%{nss_version},g" \ -e "s,%%SVRCORE_VERSION%%,%{svrcore_version},g" \ -e "s,%%MOZLDAP_VERSION%%,%{version},g" > \ - $RPM_BUILD_ROOT/%{_libdir}/pkgconfig/%{name}.pc - -%install + $RPM_BUILD_ROOT%{_libdir}/pkgconfig/%{name}.pc # There is no make install target so we'll do it ourselves. -%{__mkdir_p} $RPM_BUILD_ROOT/%{_includedir}/%{name} -%{__mkdir_p} $RPM_BUILD_ROOT/%{_libdir} -%{__mkdir_p} $RPM_BUILD_ROOT/%{_libdir}/%{name} +%{__mkdir_p} $RPM_BUILD_ROOT%{_includedir}/%{name} +%{__mkdir_p} $RPM_BUILD_ROOT%{_libdir} +%{__mkdir_p} $RPM_BUILD_ROOT%{_libdir}/%{name} # Copy the binary libraries we want for file in libssldap50.so libprldap50.so libldap50.so do - %{__install} -m 755 mozilla/dist/lib/$file $RPM_BUILD_ROOT/%{_libdir} + %{__install} -m 755 mozilla/dist/lib/$file $RPM_BUILD_ROOT%{_libdir} done # Copy the binaries we want for file in ldapsearch ldapmodify ldapdelete ldapcmp ldapcompare do - %{__install} -m 755 mozilla/dist/bin/$file $RPM_BUILD_ROOT/%{_libdir}/%{name} + %{__install} -m 755 mozilla/dist/bin/$file $RPM_BUILD_ROOT%{_libdir}/%{name} done # Copy the include files for file in mozilla/dist/public/ldap/*.h do - %{__install} -m 644 $file $RPM_BUILD_ROOT/%{_includedir}/%{name} + %{__install} -m 644 $file $RPM_BUILD_ROOT%{_includedir}/%{name} done # Copy the developer files %{__mkdir_p} $RPM_BUILD_ROOT%{_datadir}/%{name} cp -r mozilla/directory/c-sdk/ldap/examples $RPM_BUILD_ROOT%{_datadir}/%{name} -%{__mkdir_p} $RPM_BUILD_ROOT%{_datadir}/%{name}/etc +%{__mkdir_p} $RPM_BUILD_ROOT%{_datadir}%{name}/etc %{__install} -m 644 mozilla/directory/c-sdk/ldap/examples/xmplflt.conf $RPM_BUILD_ROOT%{_datadir}/%{name}/etc %{__install} -m 644 mozilla/directory/c-sdk/ldap/libraries/libldap/ldaptemplates.conf $RPM_BUILD_ROOT%{_datadir}/%{name}/etc %{__install} -m 644 mozilla/directory/c-sdk/ldap/libraries/libldap/ldapfilter.conf $RPM_BUILD_ROOT%{_datadir}/%{name}/etc %{__install} -m 644 mozilla/directory/c-sdk/ldap/libraries/libldap/ldapsearchprefs.conf $RPM_BUILD_ROOT%{_datadir}/%{name}/etc # Rename the libraries and create the symlinks -cd $RPM_BUILD_ROOT/%{_libdir} +cd $RPM_BUILD_ROOT%{_libdir} for file in libssldap50.so libprldap50.so libldap50.so do mv $file $file.%{major}.%{minor} ln -s $file.%{major}.%{minor} $file.%{major} - ln -s $file.%{major} $file + ln -s $file.%{major}.%{minor} $file done %clean %{__rm} -rf $RPM_BUILD_ROOT -%post -/sbin/ldconfig >/dev/null 2>/dev/null +%post -p /sbin/ldconfig -%postun -/sbin/ldconfig >/dev/null 2>/dev/null +%postun -p /sbin/ldconfig %files -%defattr(0755,root,root) -%{_libdir}/libssldap50.so -%{_libdir}/libprldap50.so -%{_libdir}/libldap50.so +%defattr(-,root,root,-) +%doc mozilla/directory/c-sdk/README.rpm %{_libdir}/libssldap50.so.%{major} %{_libdir}/libprldap50.so.%{major} %{_libdir}/libldap50.so.%{major} @@ -171,8 +167,8 @@ %{_libdir}/libldap50.so.%{major}.%{minor} %files tools -%defattr(0755,root,root) -%attr(0755,root,root) %dir %{_libdir}/%{name} +%defattr(-,root,root,-) +%dir %{_libdir}/%{name} %{_libdir}/%{name}/ldapsearch %{_libdir}/%{name}/ldapmodify %{_libdir}/%{name}/ldapdelete @@ -180,13 +176,12 @@ %{_libdir}/%{name}/ldapcompare %files devel -%defattr(0644,root,root) +%defattr(-,root,root,-) +%{_libdir}/libssldap50.so +%{_libdir}/libprldap50.so +%{_libdir}/libldap50.so %{_libdir}/pkgconfig/%{name}.pc -%attr(0755,root,root) %dir %{_includedir}/%{name} %{_includedir}/%{name} -%attr(0755,root,root) %dir %{_datadir}/%{name} -%attr(0755,root,root) %dir %{_datadir}/%{name}/etc -%attr(0755,root,root) %dir %{_datadir}/%{name}/examples %{_datadir}/%{name} %changelog Forgot to add 14b: add "%doc mozilla/directory/c-sdk/README.rpm" > 10) standard practice in Fedora is to symlink back to the actual .so rather > than create a trail of symlinks I'm not sure what this means? > 14) unversioned .so files must go in -devel package when there are > also versioned .so's But then if someone links an app against libldap50.so (in the devel package), how does the app find libldap50.so at runtime when it doesn't exist on the system, only libldap50.so.5.17? > 15) rpmlint complains about invalid sonames, I presume this is a side-effect > of renaming them: > E: mozldap invalid-soname /usr/lib64/libssldap-5.0.so.5.17 libssldap50.so > E: mozldap invalid-soname /usr/lib64/libprldap-5.0.so.5.17 libprldap50.so > E: mozldap invalid-soname /usr/lib64/libldap-5.0.so.5.17 libldap50.so Where do /usr/lib64/libssldap-5.0.so.5.17 and the others come from? All of my lib names should begin with lib[ss,pr]ldap50 (In reply to comment #7) > > 10) standard practice in Fedora is to symlink back to the actual .so rather > > than create a trail of symlinks > > I'm not sure what this means? In this bit of the spec... for file in libssldap50.so libprldap50.so libldap50.so do mv $file $file.%{major}.%{minor} ln -s $file.%{major}.%{minor} $file.%{major} ln -s $file.%{major} $file done ...$file is a symlink to $file.major, which is a symlink to $file.major.minor. $file should just symlink up to $file.major.minor instead. > > 14) unversioned .so files must go in -devel package when there are > > also versioned .so's > > But then if someone links an app against libldap50.so (in the devel package), > how does the app find libldap50.so at runtime when it doesn't exist on the > system, only libldap50.so.5.17? I can't say I know the specifics of it, but I know it works. :) Its explicitly called out that way in the packaging guidelines, and does work for umpteen libraries already in Fedora Core and Extras. > > 15) rpmlint complains about invalid sonames, I presume this is a side-effect > > of renaming them: > > > E: mozldap invalid-soname /usr/lib64/libssldap-5.0.so.5.17 libssldap50.so > > E: mozldap invalid-soname /usr/lib64/libprldap-5.0.so.5.17 libprldap50.so > > E: mozldap invalid-soname /usr/lib64/libldap-5.0.so.5.17 libldap50.so > > Where do /usr/lib64/libssldap-5.0.so.5.17 and the others come from? All of my > lib names should begin with lib[ss,pr]ldap50 Bah, that was from me playing around a bit to see if it was just the file name it didn't like. The actual output to be concerned about is: E: mozldap invalid-soname /usr/lib64/libssldap50.so.5.17 libssldap50.so E: mozldap invalid-soname /usr/lib64/libprldap50.so.5.17 libprldap50.so E: mozldap invalid-soname /usr/lib64/libldap50.so.5.17 libldap50.so I believe the versioning should be encoded somewhere in the shared lib, but at build time, unversioned libs are being created. A 'strings /usr/lib(64)/lib<whatever>' for any library with a versioning includes lib<whatever.so.<version> in the output, while lib*ldap50.so.* only includes lib*ldap50.so. That being the case, it looks like either something needs to be done at build time to put the right versioning info in there, or only the unversioned lib should be packaged. Ah, and the versioning being encoded in the shared lib has something to do with how the linking against the devel package works when the devel package isn't installed at runtime. Or so I believe... :) There is a new version of mozldap - version 6.0.0. I would like to add this version to Fedora instead of the previous 5.x version. Spec URL: ftp://ftp.mozilla.org/pub/mozilla.org/directory/c-sdk/releases/v6.0.0/src/mozldap.spec SRPM URL: ftp://ftp.mozilla.org/pub/mozilla.org/directory/c-sdk/releases/v6.0.0/src/mozldap6-6.0.0-1.src.rpm The general description is the same. v6.0.0 adds support for BER types in the API, adds support for SASL/GSSAPI/DIGEST MD5, and has improved IPv6 support, among other minor features and several bug fixes. Eep, sorry for the delay, this has been sitting in my queue for a while now... Anyhow, just did a run-through of the mozlap6 package. We're in much better shape with this version! The only remaining issue I see at the moment is that rpmlint is still complaining about invalid sonames: $ rpmlint /build/RPMS/x86_64/mozldap6-6.0.0-1.fc6.x86_64.rpm E: mozldap6 invalid-soname /usr/lib64/libprldap60.so.6.0.0 libprldap60.so E: mozldap6 invalid-soname /usr/lib64/libssldap60.so.6.0.0 libssldap60.so E: mozldap6 invalid-soname /usr/lib64/libldap60.so.6.0.0 libldap60.so I'm not sure what it is complaining about. Does it not like the fact that the soname is libname60.so, and not libname-6.so or libname.so.6? The name libldap60.so is a convention for historical purposes, and also serves to distinguish the library name from the openldap libldap-2.3.so etc. No, it doesn't like that the soname is libname60.so.6.0.0 instead of just libname60.so, a side-effect of it being compiled at libname60.so, then moved. Check out the last bit of comment #8. I believe libname60.so.6.0.0 needs to be encoded into the library at compile-time somehow. You need to BR cyrus-sasl-devel @@ -28,2 +29,3 @@ BuildRequires: %{svrcore_name} >= %{svrcore_version} +BuildRequires: cyrus-sasl-devel Wrt lib*ldap60.so: Upstream seems to prefer to disambiguate the library by changing the core name (the embedded "60") and therefore sees no urge to use properly versioned libraries. You can either a) go along and *not* do any symlinking on the packaging level. E.g. the 'so's remain in the core package and there are no so symlinks in *-devel b) clean it up and rename the SONAMEs from lib*ldap60.so -> lib*ldap.so.6 with a patch against the build in the tarball. a) is quick and dirty and matches current upstream methology, b) is clean and proper, but requires you to get the patch submitted upstream, otherwise we'll be breaking library ABI. (In reply to comment #14) > You can either > a) go along and *not* do any symlinking on the packaging level. E.g. the 'so's > remain in the core package and there are no so symlinks in *-devel > b) clean it up and rename the SONAMEs from lib*ldap60.so -> lib*ldap.so.6 with > a patch against the build in the tarball. > > a) is quick and dirty and matches current upstream methology, b) is clean and > proper, but requires you to get the patch submitted upstream, otherwise we'll be > breaking library ABI. Note that mozldap follows the Mozilla library naming conventions that are used by NSPR and NSS which have been in Fedora for a while now (e.g. libnspr4.so, libnss3.so, etc.). So, unless there are plans to also convert those libraries to the proper naming convention, I think it suffices to stick with libldap60.so.6.0.0. It should not be a big deal to put the proper so name in the shared libraries, so that the main package will have libldap60.so.6.0.0 and the devel package will have libldap60.so Going with the Mozilla library naming conventions is OK, but these don't suggest adding a more or less artificial .6.0.0 suffix (artificial since it's part of the packaging layer and not upstream intention). If you check NSPR and NSS they keep the soname as is, e..g don't add a versioning suffix, and thus the respective devel file has no library parts at all, not even symlinks. So it's a) in comment #14. I played a bit with the specfile and implemented a), I'll post the patch in the next comment. Created attachment 143200 [details]
specfile patch
This patch makes rpmlint happy and the package more inline with both Fedora and
Mozilla standards. There is still room for improvement, e.g. eliminating the
major/minor/subminor macros, but I wouldn't block on that. Please test this
together with Toshio's new svrcore package (e.g. through the SDK acceptance
test).
* Fri Dec 8 2006 Axel Thimm <Axel.Thimm> - 6.0.0-2
- Rename to mozldap.
- move configure step to %%build section.
- clean up excessive use of %%defines, make more Fedora like.
- fix mismatching soname issue.
- generic specfile cosmetics.
Works fine, and with the new svrcore as well. I've added libldif to the public API of mozldap, and changed the version to 6.0.1. New files: SPEC URL: ftp://ftp.mozilla.org/pub/mozilla.org/directory/c-sdk/releases/v6.0.1/src/mozldap.spec SRPM URL: ftp://ftp.mozilla.org/pub/mozilla.org/directory/c-sdk/releases/v6.0.1/src/mozldap-6.0.1-1.src.rpm SRC URL: ftp://ftp.mozilla.org/pub/mozilla.org/directory/c-sdk/releases/v6.0.1/src/mozldap-6.0.1.tar.gz Axel, I have added back some of the macros to the spec file. The macros make our life easier as maintainers - we share the same spec file between RHEL-4 (which uses different nspr and nss) and Solaris. I hope that this is acceptable. rpmlint complains about E: mozldap invalid-soname /usr/lib/libssldap60.so libssldap60.so E: mozldap invalid-soname /usr/lib/libldif60.so libldif60.so E: mozldap invalid-soname /usr/lib/libldap60.so libldap60.so E: mozldap invalid-soname /usr/lib/libprldap60.so libprldap60.so we should get rid of if [ $RPM_BUILD_ROOT != "/" ] ; then %{__rm} -rf $RPM_BUILD_ROOT ; fi in the %build and %install section of the spec and add rm -rf $RPM_BUILD_ROOT to the start of %install and change %clean to rm -rf $RPM_BUILD_ROOT md5sums match 6f651d0e5c4b04352c64207623f8ef4f mozldap-6.0.1.tar.gz 6f651d0e5c4b04352c64207623f8ef4f ../SOURCES/mozldap-6.0.1.tar.gz builds in mock for FC-6 license is ok Owns its own directories Fix the %build %install and %clean lines and its APPROVED Feel free to fix when importing Created attachment 145200 [details] spec file diffs Thanks Dennis. Here are the final files: SPEC URL: ftp://ftp.mozilla.org/pub/mozilla.org/directory/c-sdk/releases/v6.0.1/src/mozldap.spec SRPM URL: ftp://ftp.mozilla.org/pub/mozilla.org/directory/c-sdk/releases/v6.0.1/src/mozldap-6.0.1-2.src.rpm SOURCE URL: ftp://ftp.mozilla.org/pub/mozilla.org/directory/c-sdk/releases/v6.0.1/src/mozldap-6.0.1.tar.gz The attachment is the diffs in the spec file proposed by Dennis. Successfully imported into /cvs/extras. Submitted branch request. |