Bug 222611 - libthai
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Clasen
Bill Nottingham
Depends On:
  Show dependency treegraph
Reported: 2007-01-15 02:44 EST by Behdad Esfahbod
Modified: 2014-03-16 23:04 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-01-27 17:10:29 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
full mock build log (88.75 KB, text/plain)
2007-01-16 13:36 EST, Matthias Clasen
no flags Details
spec file (3.42 KB, text/plain)
2007-01-16 21:04 EST, Matthias Clasen
no flags Details

  None (edit)
Description Behdad Esfahbod 2007-01-15 02:44:42 EST
Please add libthai to rawhide.  It's needed for proper thai support, and pango
rpm will depend on it.

Spec and source rpm at: http://www.cs.toronto.edu/~behdad/fedora/libthai/

I'll be maintaining it.

Note about magic in the spec file: libthai depends on this library called
libdatrie.  libdatrie is a data-structure implementaiton that the author of
libthai ripped out of it.  However, since libthai is the only user of that code,
there's no reason to 1) package it separately, 2) use it as a shared library. 
So, we compile it as a libtool convenience library and include in libthai.so,
and use symbol hiding to hide them (and other internal symbols).
Comment 1 Matthias Clasen 2007-01-16 11:44:42 EST
Initial impressions (not going through the review guidelines yet)

It would be good to have your comment about the libdatrie "magic" next
to the Patch: line in the spec file, or maybe even better in the %build
section where the magic happens.

Requires: %{name} = %{PACKAGE_VERSION}

I prefer to write %{version}-%{release} there, but this should be fine too.

The devel package should require pkgconfig since it installs a pc file

I think we have a semi-standard variation of the summary/description for 
-devel packages. Something like "Header files necessary to develop applications
using libthai" or something to that end. Seems better than to use identical

We don't package static libraries nowadays, unless there is a very good reason
for it.

Does it make any sense to put %{_docdir}/* in the file list ? normally packages
don't install docs there, it is where rpm puts the things that are marked as

Comment 2 Matthias Clasen 2007-01-16 13:23:05 EST
Trying to build in mock fails with:

gcc -shared  .libs/dummy.o -Wl,--whole-archive ../src/thctype/.libs/libthctype.a
../src/thstr/.libs/libthstr.a ../src/thcell/.libs/libthcell.a
../src/thinp/.libs/libthinp.a ../src/thrend/.libs/libthrend.a
../src/thcoll/.libs/libthcoll.a ../src/thbrk/.libs/libthbrk.a
../src/thwchar/.libs/libthwchar.a ../src/thwctype/.libs/libthwctype.a
../src/thwstr/.libs/libthwstr.a ../src/thwbrk/.libs/libthwbrk.a
-Wl,--no-whole-archive  -L/var/tmp/libthai-0.1.7-root-mockbuild/usr/lib
-L/usr/lib -ldatrie  -Wl,-soname -Wl,libthai.so.0 -o .libs/libthai.so.0.1.1
/usr/bin/ld: cannot find -ldatrie
Comment 3 Matthias Clasen 2007-01-16 13:36:57 EST
Created attachment 145715 [details]
full mock build log
Comment 4 Behdad Esfahbod 2007-01-16 14:13:15 EST
Uploaded updated spec file and libthai-0.1.7-2.fc7.src.rpm fixing all issues above.
Comment 5 Matthias Clasen 2007-01-16 21:02:17 EST
to make this build in mock with automake 1.10, I had to first
add aclocal calls before the automake calls, and then add a
BuildRequires: libtool

With these changes, the rpm builds fine, and rpmlint has no complaints
about the resulting base rpm, but for the -devel, it says:

W: libthai-devel incoherent-version-dependency-on libthai libthai-0.1.7 0.1.7
E: libthai-devel standard-dir-owned-by-package /usr/share/man/man3

Trying to install the -devel package fails with

error: Failed dependencies:
        libthai = libthai-0.1.7 is needed by libthai-devel-0.1.7-2.fc7.i386

after installing the base rpm.

One thing I wondered about is the format of the files that are installed in
/usr/share/libthai. They look binary, are they arch-independent ?
Comment 6 Matthias Clasen 2007-01-16 21:04:43 EST
Created attachment 145765 [details]
spec file 

the attached spec contains the fixes I needed to get libthai and libthai-devel
to build and install without rpmlint warnings.
Comment 7 Matthias Clasen 2007-01-17 12:22:13 EST
Ok, doing the review based upon the spec file attached in the previous comment.

+ rpmlint is silent
+ package name follows naming conventions
+ spec file name matches package name
+ package follows packaging guidelines
+ license is LGPL
- spec license tag says GPL --> THIS NEEDS FIXING
+ COPYING is in %doc
+ spec is in English (haven't checked carefully if it is 
  American or Canadian English...)
+ spec is legible and contains comments
+ md5sums of sources match upstream
+ packages built fine in mock on i386
+ complete BuildRequires listed
+ no locale data to handle
+ calls ldconfig in %post/%postun
+ no Prefix:
+ owns all non-standard directories
+ no dupes in file lists
+ file lists have %defattr
+ %clean is present
+ uses macros for standard paths
+ package contains code
+ no doc subpackage necessary
+ %doc has only docs
+ -devel package is there and contains the right stuff
+ -devel requires pkgconfig
+ libraries are correctly packaged
+ -devel requires base package
+ .la files are stripped
+ no desktop file necessary
+ package owns no directories owned by others
Comment 8 Matthias Clasen 2007-01-17 12:27:31 EST
btw, Behdad answered my question regarding the binary files in /usr/share: The
are arch-independent.
Comment 9 Matthias Clasen 2007-01-17 12:28:51 EST
Accepting the package with the provision that the License: tag is corrected for
the initial build.
Comment 10 Jesse Keating 2007-01-17 15:16:10 EST
Package added to brew.  Close this bug once built for rawhide.
Comment 11 Behdad Esfahbod 2007-01-27 17:10:29 EST
Built last week or so.

Note You need to log in before you can comment on or make changes to this bug.