[Bug 251020] New: Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail],[e-mail]

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

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

------- Additional Comments From [e-mail]  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...

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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 [e-mail]  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

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]    |[e-mail]
             Status|NEW                         |ASSIGNED
               Flag|                            |fedora-review?

------- Additional Comments From [e-mail]  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.

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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 [e-mail]  2007-09-05 16:51 EST -------
I just realized that the broken debuginfo affects your rpm packages, too.

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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 [e-mail]  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

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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 [e-mail]  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
[...]

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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 [e-mail]  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

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

------- Additional Comments From [e-mail]  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

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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 [e-mail]  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

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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 [e-mail]  2007-09-07 05:10 EST -------
Package is APPROVED, you need to follow:
http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess
(step 8)

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

------- Additional Comments From [e-mail]  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

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

------- Additional Comments From [e-mail]  2007-09-07 14:33 EST -------
cvs done.

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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 [e-mail]  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.

[Bug 251020] Review Request: libflaim - Flaim Database Engine

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

------- Additional Comments From [e-mail]  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