Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=251020
Summary: Review Request: libflaim - Flaim Database Engine Product: Fedora Version: devel Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: snecklifter@gmail.com QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
Spec URL: http://snecker.fedorapeople.org/libflaim/libflaim.spec SRPM URL: http://snecker.fedorapeople.org/libflaim/libflaim-4.9.1046-9.1.fc7.src.rpm Description: FLAIM is an embeddable cross-platform database engine that provides a rich, powerful, easy-to-use feature set. It is the database engine used by Novell eDirectory. It has proven to be highly scalable, reliable, and robust. It is available on a wide variety of 32 bit and 64 bit platforms.
Notes: This is the first of a number of packages in order to provide iFolder[1] for Fedora. I am already in ACL and familiar with Koji, plague etc. Builds cleanly in mock.
[1] http://www.ifolder.com/index.php/Main_Page
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |opensource@till.name
------- Additional Comments From opensource@till.name 2007-08-30 09:50 EST ------- - static libraries should be in a -static package or not packaged at all: %{_libdir}/libflaim.a - see: http://fedoraproject.org/wiki/Packaging/Guidelines#head-2302ec1e1f44202c9cc4...
- - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). -> Add Reuires: pkgconfig to %package devel
- you should use $RPM_OPT_FLAGS instead of %optflags: http://fedoraproject.org/wiki/Packaging/Guidelines#head-f3d77b27a5d29dfc1f56...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
------- Additional Comments From snecklifter@gmail.com 2007-08-30 10:51 EST ------- (In reply to comment #1)
Thanks for the review Till.
- static libraries should be in a -static package or not packaged at all:
%{_libdir}/libflaim.a - see:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-2302ec1e1f44202c9cc4...
Done.
- MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
(for directory ownership and usability). -> Add Reuires: pkgconfig to %package devel
Done.
- you should use $RPM_OPT_FLAGS instead of %optflags:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-f3d77b27a5d29dfc1f56...
Done.
Updated pkgs at:
http://snecker.fedorapeople.org/libflaim/
I am still working on iFolder and simias which are the other two packages that form the reqs necessary to get iFolder working on Fedora.
Regards Chris
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |opensource@till.name Status|NEW |ASSIGNED Flag| |fedora-review?
------- Additional Comments From opensource@till.name 2007-09-05 16:49 EST ------- The release should be an integer, imho it is in the Naming Guidelines (http://fedoraproject.org/wiki/Packaging/NamingGuidelines)
So change Release: 9.2%{?dist} to Release: 10%{?dist}
You can later use Release: 10%{?dist}.1 If you need to increment only in one Fedora Collection that is not Rawhide.
Source0 is not valid (anymore): $ curl -I http://forgeftp.novell.com/flaim/development/flaim/downloads/source/libflaim... HTTP/1.1 404 Not Found
According to the URL in the spec file the latest tarball is: libflaim-4.9.989.tar.gz
Is this older version intentional?
The buildroot is not ok, see http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885...
You should use %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) or some other example from the wiki.
Please use rm -rf %{buildroot} instead of rm -Rf %{buildroot}
Did you build your packages with mock? When I build it and run rpmlint on the rpms, I get: $ rpmlint libflaim-* W: libflaim unstripped-binary-or-object /usr/lib/libflaim.so.5.2 E: libflaim-debuginfo empty-debuginfo-package W: libflaim-devel no-documentation
The third warning can be ignored. The build.log shows: ldconfig /var/tmp/libflaim-4.9.1046-build/usr/lib Installation complete. + /usr/lib/rpm/find-debuginfo.sh /builddir/build/BUILD/libflaim-4.9.1046 0 blocks find: /var/tmp/libflaim-4.9.1046-build/usr/lib/debug: No such file or directory
But I do not know, why.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
------- Additional Comments From opensource@till.name 2007-09-05 16:51 EST ------- I just realized that the broken debuginfo affects your rpm packages, too.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
------- Additional Comments From snecklifter@gmail.com 2007-09-06 10:55 EST ------- Hi Till,
Thanks for assigning yourself to this.
(In reply to comment #3)
The release should be an integer, imho it is in the Naming Guidelines (http://fedoraproject.org/wiki/Packaging/NamingGuidelines)
So change Release: 9.2%{?dist} to Release: 10%{?dist}
You can later use Release: 10%{?dist}.1 If you need to increment only in one Fedora Collection that is not Rawhide.
Source0 is not valid (anymore): $ curl -I
http://forgeftp.novell.com/flaim/development/flaim/downloads/source/libflaim...
HTTP/1.1 404 Not Found
According to the URL in the spec file the latest tarball is:
libflaim-4.9.989.tar.gz
Is this older version intentional?
The version being built is newer, essentially a subversion checkout therefore I have made the necessary changes to reflect this.
The buildroot is not ok, see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885...
You should use %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) or some other example from the wiki.
Okay, done.
Please use rm -rf %{buildroot} instead of rm -Rf %{buildroot}
Okay, done.
Did you build your packages with mock? When I build it and run rpmlint on the rpms, I get: $ rpmlint libflaim-* W: libflaim unstripped-binary-or-object /usr/lib/libflaim.so.5.2 E: libflaim-debuginfo empty-debuginfo-package W: libflaim-devel no-documentation
The third warning can be ignored. The build.log shows: ldconfig /var/tmp/libflaim-4.9.1046-build/usr/lib Installation complete.
- /usr/lib/rpm/find-debuginfo.sh /builddir/build/BUILD/libflaim-4.9.1046
0 blocks find: /var/tmp/libflaim-4.9.1046-build/usr/lib/debug: No such file or directory
But I do not know, why.
This was because the shared object was not installed executable - I have attached a small patch to resolve this and should hopefully get this upstream quickly.
Updated RPMS as usual at:
http://snecker.fedorapeople.org/libflaim/
Regards Chris
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
------- Additional Comments From opensource@till.name 2007-09-06 11:31 EST ------- (In reply to comment #5)
http://forgeftp.novell.com/flaim/development/flaim/downloads/source/libflaim...
HTTP/1.1 404 Not Found
According to the URL in the spec file the latest tarball is:
libflaim-4.9.989.tar.gz
Is this older version intentional?
The version being built is newer, essentially a subversion checkout therefore I have made the necessary changes to reflect this.
This still needs some work. I do not understand where the 1046 comes from in the Version tag. Is this the svn revision? Did you built the svn snapshot yourself? In this case, you need to add instructions to the spec how to rebuild the snapshot, see: http://fedoraproject.org/wiki/Packaging/SourceURL#head-615f6271efb394ab340a9...
The URL in Source0 does not work here: $ curl --insecure -I https://forgesvn1.novell.com/svn/flaim/libflaim-4.9.1046.tar.gz HTTP/1.1 404 Not Found [...]
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
------- Additional Comments From snecklifter@gmail.com 2007-09-06 16:30 EST ------- (In reply to comment #6)
This still needs some work. I do not understand where the 1046 comes from in the Version tag. Is this the svn revision?
It was, kinda, sorta. I had used the some RPMS from a SUSE developers repo as a starting point and that tarball was included as source. I can't figure out how it was generated as pulling from svn also brings down a number of other header files and such which cause the build to fail. Therefore I've downgraded the spec a little to an official release which builds fine using the patch. The patch was accepted upstream today btw.
I have uploaded new builds. Rpmlint is quiet on these now.
Cheers Chris
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
------- Additional Comments From opensource@till.name 2007-09-06 17:22 EST ------- - rpmlint: ok W: libflaim-devel no-documentation
- naming: ok - versioning: ok - license: BAD/TODO License Tag ist LGPLv2 but license is GPLv2 license text (GPLv2) is included and mentioned in source files - rpm legible: ok - source: ok url points to correct file: cbd0caf6239cffb7640391eda7551d4a libflaim-4.9.989.tar.gz cbd0caf6239cffb7640391eda7551d4a libflaim-4.9.989.tar.gz.1 - builds in mock for F7, i386 - ldconfig run: ok - directory ownage: ok - buildroot: ok - %files: no dupes and correct defattr: ok - no -static needed: ok - no libtool files: ok - .so file without suffix in devel: ok - %install and %clean with rm: ok - -devel requires of mainpackage: ok
- todo before import: * add a libflaim prefix to the patch, i.e. rename it to: libflaim-permissions.patch
* fix license tag
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
------- Additional Comments From snecklifter@gmail.com 2007-09-07 04:56 EST ------- (In reply to comment #8)
- todo before import:
- add a libflaim prefix to the patch, i.e. rename it to:
libflaim-permissions.patch
Okay, done.
- fix license tag
Okay, done.
Cheers Chris
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
------- Additional Comments From opensource@till.name 2007-09-07 05:10 EST ------- Package is APPROVED, you need to follow: http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess (step 8)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
snecklifter@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
------- Additional Comments From snecklifter@gmail.com 2007-09-07 05:25 EST ------- New Package CVS Request ======================= Package Name: libflaim Short Description: Flaim Database Engine Owners: snecker Branches: F-7 InitialCC: Cvsextras Commits: yes
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
------- Additional Comments From kevin@tummy.com 2007-09-07 14:33 EST ------- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-09-08 12:26 EST ------- libflaim.pc contains the line: ---------------------------------------------------- Libs: -lpthread -lrt -lstdc++ -ldl -lncurses -lflaim -L${libdir} ---------------------------------------------------- "-lncurses" means that libflaim-devel should have "Requires: ncurses-devel".
But make check if "-lncurses" is really needed. For libflaim.so the linkage against libncurses.so MUST be done in advance and "-lncurses" is usually not needed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: libflaim - Flaim Database Engine
https://bugzilla.redhat.com/show_bug.cgi?id=251020
snecklifter@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
------- Additional Comments From snecklifter@gmail.com 2007-09-09 07:00 EST ------- (In reply to comment #13)
libflaim.pc contains the line:
Libs: -lpthread -lrt -lstdc++ -ldl -lncurses -lflaim -L${libdir}
"-lncurses" means that libflaim-devel should have "Requires: ncurses-devel".
But make check if "-lncurses" is really needed. For libflaim.so the linkage against libncurses.so MUST be done in advance and "-lncurses" is usually not needed.
Thanks for the additional info Mamoru. I'll add this on the next build. Buildsys seems to be having problems at the moment. :(
Closing now as per package process - thanks everyone.
Regards Chris
package-review@lists.fedoraproject.org