Bug 466183
| Summary: | Review Request: sblim-sfcb - Small Footprint CIM Broker | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Emily Ratliff <ejratl> |
| Component: | Package Review | Assignee: | Praveen K Paladugu <praveen_paladugu> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, jwboyer, kevin, matt_domsch, notting, praveen_paladugu, shyam_iyer, srinivas_ramanatha, vcrhonek, wwlinuxengineering |
| Target Milestone: | --- | Flags: | praveen_paladugu:
fedora-review+
kevin: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2009-12-08 22:41:26 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: | 468287 | ||
| Bug Blocks: | 462697, 468400 | ||
|
Description
Emily Ratliff
2008-10-08 21:58:50 UTC
$ rpmlint sblim-sfcb.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint sblim-sfcb-1.3.2-1.src.rpm sblim-sfcb.src: W: non-standard-group System Tools 1 packages and 0 specfiles checked; 0 errors, 1 warnings. $ rpmlint sblim*rpm sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcCertificateAuthentication.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcFileRepository.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcProfileProvider.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcCimXmlCodec.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcBasicPAMAuthentication.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcClassProviderMem.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcClassProvider.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcIndCIMXMLHandler.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcBasicAuthentication.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcBrokerCore.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcInteropServerProvider.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcInteropProvider.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcInternalProvider.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcUtil.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcHttpAdapter.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcQualifierProvider.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libcimcClientSfcbLocal.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcObjectImplSwapI32toP32.so sblim-sfcb.i386: W: devel-file-in-non-devel-package /usr/lib/libsfcClassProviderGz.so sblim-sfcb.i386: W: non-standard-group System Tools sblim-sfcb.i386: W: service-default-enabled /etc/init.d/sfcb sblim-sfcb.i386: W: incoherent-init-script-name sfcb sblim-sfcb-schema.i386: W: no-documentation sblim-sfcb-schema.i386: W: non-standard-group System Tools sblim-sfcb-schema.i386: W: invalid-license Distributed Management Task Force 3 packages and 0 specfiles checked; 0 errors, 25 warnings. I believe that the warnings about the libraries are all false positives because sfcb loads support libraries as needed. They are not development libraries. (19 warnings) I left the init script named sfcb rather than sblim-sfcb because that is what upstream does and that is what other community distros do. (2 warnings) My understanding is that the group name doesn't really matter. (2 warnings) The schema is copyright DMTF, see Release notes http://www.dmtf.org/standards/cim/cim_schema_v219/ReleaseNotes.html (1 warning) I left the schema package as a subpackage follow upstream's lead - advise on whether this is correct or not would be appreciated. Some updates based on comments from Srini Ramanatha - including the inclusion of %{dist} in the release led to updates as below:
Spec URL: http://sblim.sourceforge.net/fedora-rpms/sblim-sfcb.spec
SRPM URL: http://sblim.sourceforge.net/fedora-rpms/sblim-sfcb-1.3.2-2.fc9.src.rpm
Description: Small Footprint CIM Broker (sfcb) is a CIM server conforming to
the CIM Operations over HTTP protocol. It is robust, with low resource consumption
and therefore specifically suited for embedded and resource constrained
environments. sfcb supports providers written against the Common Manageability
Programming Interface (CMPI).
Emily, thank you for packaging this up. > echo "%doc %{_datadir}/man/man1/*" >> _pkg_list Please remove the %doc from this line. > sed s?$RPM_BUILD_ROOT??g _pkg_list > _pkg_list_2 > mv -f _pkg_list_2 _pkg_list You can use sed -i (in place edit) to avoid the need to do the mv. schema subpackage has only a single directory, no files in /usr/share/sfcb/CIM. Are these created at runtime or at install time? If other packages will be laying down files in there (and thus own them), that's fine, but then you should use %dir %{_datadir}/sfcb/CIM/ and the license on an empty directory is then the same as the main package. bug: initscript is named 'sfcb' but chkconfig and service calls in %post use sblim-sfcb. Then you won't need the exit 0 line either. Can you get sfcb to put its 19 dlopen()d libraries (plus their two symlinks each, 57 files in total) into %{_libdir}/sblim-sfcb/* instead of %{_libdir}, drop a .conf file into /etc/ld.so.conf.d/ and remove the rpaths? Would be cleaner. also, the .so name vs other names. This is because support.c has: void *loadLibib(const char *libname) { char filename[255]; sprintf(filename, "lib%s.so", libname); return dlopen(filename, RTLD_LAZY); } As these are private libraries included with the package, it would seem one could use the .so.0 name instead of the .so name, and could eliminate the .so symlink entirely (as you don't need it for a devel package, nothing links to these). upon reflection, you won't need a conf file in /etc/ld.so.conf.d/ if the calls to dlopen() are made to take a fully specified path, which can be determined at compile time if so desired. In this way, private libraries won't ever be seen by other applications that shouldn't see them. https://sourceforge.net/tracker/?func=detail&atid=712784&aid=2158091&group_id=128809 filed with upstream about the library placement. Until upstream fixes, it's acceptable to leave them in %{_libdir}. Comparing with openSUSE, they use a shared cim-schema package, which is the MOFs from http://www.dmtf.org/standards/cim/cim_schema_v2191, instead of using the copy of the MOFs included in the sfcb package. Could/should we do that too? I would expect so. Thanks for the review Matt. An updated sblim-sfcb.spec file should go up on SF shortly. It should address all of your comments other than the location of the libraries which I am still working on and the comments about the schema package, both previous comments and the one today. I really don't have a strong opinion on how the schema should be handled. I did notice the openSUSE approach which seems reasonable, but as a first pass I went with the upstream approach. If you think a separate schema package is the right way to go, then I'll change this spec file to omit the schema subpackage. Emily Yes, I hadn't realized the sfcb Makefile was using curl to download the latest schema from the web site at buildtime. This is not permissible; packages may not access network resources at buildtime. I've taken the openSUSE copy of cim-schema and cleaned it up to meet Fedora packaging guidelines. I'll open a new review ticket for that. Package review #468287 cim-schema now blocks this package, so we can avoid needing network connectivity at buildtime. Let's try to get this part straight...
In OpenSUSE "factory " (equivalent of Fedora rawhide):
sblim-sfcb
Provides: cim-server
Provides: cimserver
tog-pegasus
Provides: cim-server
In the current sblim-sfcb for Fedora being proposed, you have:
Provides: cimserver = %{version}-%{release}
and unfortunately, Fedora's tog-pegasus has neither of these lines.
Can we all agree that:
Provides: cim-server
is the "right" thing to add? That way package Requires: and/or yum install lines can all agree to use cim-server.
(In reply to comment #0) > Spec URL: http://sblim.sourceforge.net/fedora-rpms/sblim-sfcb.spec > SRPM URL: http://sblim.sourceforge.net/fedora-rpms/sblim-sfcb-1.3.2-1.src.rpm > Description: Small Footprint CIM Broker (sfcb) is a CIM server conforming to > the > CIM Operations over HTTP protocol. It is robust, with low resource consumption > and therefore specifically suited for embedded and resource constrained > environments. sfcb supports providers written against the Common Manageability > Programming Interface (CMPI). Hi Emily, The above links are not working. Could you please check them and provide working links. Thank you Praveen K Paladugu Hi Praveen and Matt, Try Spec URL: http://ratliff.net/sfcb/sblim-sfcb.spec SRPM URL: http://ratliff.net/sfcb/sblim-sfcb-1.3.4-4.fc11.src.rpm It has bee updated and it now requires cim-schema as defined in https://bugzilla.redhat.com/show_bug.cgi?id=468287 for both building and running. It has Provides: cim-server Here are a couple of things that don't feel entirely clean yet: * I removed %{?_smp_mflags} from the make directive because it was causing build failures since later steps depend on earlier steps which hadn't yet completed. * I added sfcbrepos -f statement to %post so that sfcb can find the cim-schema so that adds some time during the install. In OpenSUSE, they add this to the init file instead. It only has to be run once. * The init script is still named sfcb rather than sblim-sfcb which is consistent with upstream and OpenSUSE, but rpmlint complains. * Very likely incorrectly, I added a statement about the static libraries to silence an rpmbuild complaint. Emily Review ## rpmlint sblim-sfcb.spec sblim-sfcb.spec:16: W: unversioned-explicit-provides cim-server ## rpmlint sblim-sfcb-1.3.4-4.fc11.src.rpm sblim-sfcb.src:16: W: unversioned-explicit-provides cim-server 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Since cim-server is not a package, the above warning can be ignored. 1) There doesn't seem to be a explicit LICENSE file inside the sources. In such cases it is always better to contact upstream and get them to add the LICENSE file to the source tar. 2) Every Explicit "Requires:" tag has to be accompanied with a comment with a justification. (https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires) 3) Unversioned shared object (ending with *.so) have to moved to devel package. Thus created devel package should "Requires" the base package with its precise versioning. Wherever not required, please exclude the static libraries from the package (remove all *.la files if possible). (https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries) 4) All the init scripts have to be moved to /etc/rc.d/init.d .Use the macro %{_initddir} to move the init files. (https://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscripts_on_the_filesystem). 5) The service sfcb seems to be started by default, by turning on the service using the chkconfig. If a service has to be started by default in any runlevel, those runlevels must be listed using a $Default-Start tag. (https://fedoraproject.org/wiki/Packaging:SysVInitScript#.23_Default-Start:_line). 6) All Fedora SysV-style initscripts must contain the # Short-Description: line in the LSB Header. (https://fedoraproject.org/wiki/Packaging:SysVInitScript#.23_Short-Description:_line). All Fedora SysV-style initscripts must contain the # Description: line in the LSB Header. 7) The init scripts in Fedora must have the functions condrestart and try-restart defined. The current version only defines the condrestart. https://fedoraproject.org/wiki/Packaging:SysVInitScript#condrestart_and_try-restart. Above listed are some of the changes required in this package. I added an url with every comment for your reference. Review item 3) is not applicable to plugins which are dlopen()ed, which is the case with this package.
I believe there is a newer upstream package by now, which moves all the plugins into a private directory under /usr/lib{64}/sblim-sfcb/ or similar, so the plugins aren't littering /usr/lib{64} directly.
The cim-schema package just got approved, so I'll build that shortly. That package review was blocking this package review. So now is the time to get moving on this one again. Emily, are you still available to do so?
Thanks,
Matt
I built Emily's 1.3.4 package now. Yes, the plugins have moved into /usr/lib*/sfcb/. Thank you. Are the /usr/lib*/sfcb/*.la files needed? I wouldn't expect so. I believe that generating the RSA keypair should be moved from RPM installtime to the initscript, similar to how sshd's initscript works. At installtime (which may well be in a factory), there may not be sufficient entropy to generate the keypair which would hang the install process. At least at first boot initscript startup time, there is a higher likelihood of a console being present to add entropy if needed. For Praveen's comment 1), the source package contains a COPYING file listing the Eclipse Public License 1.0. which is getting installed. That handles the concern. Removing %{_smp_mflags} because parallel build doesn't work is fine. Please file a bug with upstream noting this though. Resetting the review flag, so Praveen can set it back to ? himself to take over the review. Can we get this reviewed and approved today? To build this properly, I asked release engineering to add cim-schemas to the buildroot, but they want to remove this special case override ASAP. To do so, we need to get sblim-sfcb and any other packages that are depending on cim-schemas (is that all the sblim-cmpi-* packages? I hope not) built ASAP. The override is for Fedora 11 only; rawhide of course doesn't need it. If it looks like there will be some delay, we can drop the override for Fedora 11, and reinstate it after all the dependent packages have been built in rawhide. (In reply to comment #17) > To do so, we need to get sblim-sfcb > and any other packages that are depending on cim-schemas (is that all the > sblim-cmpi-* packages? I hope not) built ASAP. sblim-cmpi-* packages include it's own schema files. I prefer to complete review of sblim-cmpi-* packages without dependency on cim-schema and add it later. If someone includes the above suggested changes, I will do a final review. Praveen Hi Matt and Praveen, I'm just back from vacation, apologies for the delay. I'll get the updates that I can out within the next 2 days. Thanks for the URLs Praveen, that is very helpful. Emily Hey Emily, Could you please find time to make the above suggested changes? Praveen sblim-sfcb version 1.3.4 had the following rpmlint errors: rpmlint sblim-sfcb.spec sblim-sfcb.spec:16: W: unversioned-explicit-provides cim-server 0 packages and 1 specfiles checked; 0 errors, 1 warnings. rpmlint ../SRPMS/sblim-sfcb-1.3.4-4.fc11.src.rpm sblim-sfcb.src:16: W: unversioned-explicit-provides cim-server 1 packages and 0 specfiles checked; 0 errors, 1 warnings. rpmlint ../RPMS/x86_64/sblim-sfcb-* sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcClassProviderGz.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcHttpAdapter.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcIndCIMXMLHandler.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcInternalProvider.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcBasicPAMAuthentication.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcUtil.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcFileRepository.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcClassProviderMem.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcInteropProvider.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libcimcClientSfcbLocal.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcInteropServerProvider.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcCimXmlCodec.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcQualifierProvider.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcBasicAuthentication.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcCertificateAuthentication.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcClassProvider.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcObjectImplSwapI32toP32.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcProfileProvider.so sblim-sfcb.x86_64: W: devel-file-in-non-devel-package /usr/lib64/sfcb/libsfcBrokerCore.so sblim-sfcb.x86_64: W: service-default-enabled /etc/init.d/sfcb sblim-sfcb.x86_64: W: incoherent-init-script-name sfcb 3 packages and 0 specfiles checked; 0 errors, 21 warnings. Fixed these warnings and made some changes as suggested by Praveen in comment #13. Here's the rpmlint output of the modified packages: rpmlint sblim-sfcb.spec sblim-sfcb.spec:16: W: unversioned-explicit-provides cim-server sblim-sfcb.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 34, tab: line 86) 0 packages and 1 specfiles checked; 0 errors, 2 warnings. [root@srini-f11 SPECS]# rpmlint ../SRPMS/sblim-sfcb-1.3.4-6.fc11.src.rpm sblim-sfcb.src:16: W: unversioned-explicit-provides cim-server sblim-sfcb.src: W: mixed-use-of-spaces-and-tabs (spaces: line 34, tab: line 86) 1 packages and 0 specfiles checked; 0 errors, 2 warnings. [root@srini-f11 SPECS]# rpmlint ../RPMS/x86_64/sblim-sfcb-* 3 packages and 0 specfiles checked; 0 errors, 0 warnings. The modified srpm and spec can be found at the following location: http://linux.dell.com/files/fedora/sblim-sfcb/ Final Review:
1) The build doesn't use the standard build flags defined by rpm for this platform. So instead of using "CFLAGS = -D_GNU_SOURCE" with "configure",
CFLAGS = "$CFLAGS -D_GNU_SOURCE" should be used. (current line :44)
- %configure --enable-debug --enable-ssl --enable-pam --enable-ipv6 CFLAGS= -D_GNU_SOURCE
+ %configure --enable-debug --enable-ssl --enable-pam --enable-ipv6 CFLAGS="$CFLAGS -D_GNU_SOURCE"
2) The base package (sblim-sfcb-1.3.4-6.fc10.i386.rpm) doesn't contain any libraries in it. To fix this, (current line: 69)
- #echo "%{_libdir}/sfcb/*.so*" >> _pkg_list
+ echo "%{_libdir}/sfcb/*.so.*" >> _pkg_list
This will add *.so.0, *.so.0.0 files into the package.
3) The devel package should only have the *.so files. So, (current lines 100,101)
- %{_libdir}/sfcb/*.so*
- %{_libdir}/sfcb/*.la
+ %{_libdir}/sfcb/*.so
The static libraries should not be part of either the base or the devel package.
4) The tar present in the source RPM doesn't match the original sources. This is a requirement with Fedora Packaging. If any changes have to be made, they have to be added as patches.
5) The package builds fine on mock currently, will check again once the above changes are implemented.
Thank you
Praveen
Changes implemented and the files are placed under http://linux.dell.com/files/fedora/sblim-sfcb/ Almost there. Final touch-ups: 1) All the init scripts have to be in added to /etc/rc.d/init.d. Currently they are added to /etc/init.d directory. Moving the sblim-sfcb script to the /etc/rc/init.d after "make install" should take care of this. 2) I verified that the devel package doesn't contain any header files. The make doesn't install any header files at all. So I don't see a reason to even build a devel package. Sorry for the confusion related to the same. Currently there seem to be no value in building a devel package for sblim-sfcb. So, please related lines from spec file After fixing the above, sblim-sfcb is good for CVS access. Implemented the suggested changes. The modified spec and the SRPM available at the following location: http://linux.dell.com/files/fedora/sblim-sfcb/ Looks good now. Approved for CVS. New Package CVS Request ======================= Package Name: sblim-sfcb Short Description: CIM server for embedded enviroments. Owners: srini praveenp Branches: F-10 F-11 F-12 EL-4 EL-5 InitialCC: mdomsch I'm a little confused here... is Emily no longer the submitter here? Emily: Is it ok with you that srini and praveenp maintain the package? Kevin, Yes, it is ok with me with many thanks to Srini and Praveen for stepping up. Thanks! Emily Thanks! cvs done. |