Bug 438932 - Review Request: libotf - Library for handling OpenType Font
Review Request: libotf - Library for handling OpenType Font
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-25 21:03 EDT by Neal Becker
Modified: 2008-05-25 20:24 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-25 20:24:16 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Neal Becker 2008-03-25 21:03:43 EDT
Spec URL: http://nbecker.dyndns.org:8080/RPM/libotf.spec
SRPM URL: http://nbecker.dyndns.org:8080/RPM/libotf-0.9.7-1.fc8.src.rpm
Description: The library "libotf" provides the following facilites.
Read Open Type Layout Tables from OTF file. Currently these tables are
supported; head, name, cmap, GDEF, GSUB, and GPOS.  Convert a Unicode
character sequence to a glyph code sequence by using the above tables.
The combination of libotf and the FreeType library (Ver.2) realizes
CTL (complex text layout) by OpenType fonts. This library is currently
used by the m17n library. It seems that the probject Free Type Layout
provides the similar (or better) facility as this library, but
currently they have not yet released their library. So, we have
developed this one.

Useful for emacs-23.0.60.1
Mock build OK
Comment 1 Parag AN(पराग) 2008-03-26 00:01:48 EDT
1)No need of INSTALL as %doc
2)add Requires: pkgconfig to -devel package
3)Use following to preserve timestamps while installing
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
4) rpmlint gave me

libotf.i386: W: devel-file-in-non-devel-package /usr/bin/libotf-config
A development file (usually source code) is located in a non-devel
package. If you want to include source code in your package, be sure to
create a development package.
==> Good if you move file libotf-config to -devel package
Comment 2 Neal Becker 2008-03-26 07:38:37 EDT
I believe all the above have been fixed.

Please see:
http://nbecker.dyndns.org:8080/RPM/libotf.spec
http://nbecker.dyndns.org:8080/RPM/libotf-0.9.7-2.fc8.src.rpm
Comment 3 Parag AN(पराग) 2008-03-28 04:53:40 EDT
Review:
+ package builds in mock.
koji build => http://koji.fedoraproject.org/koji/taskinfo?taskID=535524
+ rpmlint is silent for SRPM and RPM.
+ source files match upstream.
017dc38925ffe33cc76d6d29c7757d36  libotf-0.9.7.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is small so no need of -doc subpackage.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ libotf.pc files are present.
+ -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ ldconfig scriptlets are used.
+ package libotf-0.9.7-2.fc9->
  Provides: libotf.so.0
  Requires: libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1)
libc.so.6(GLIBC_2.1.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4)
libfreetype.so.6 libotf.so.0 rtld(GNU_HASH)
+ package libotf-devel-0.9.7-2.fc9 ->
  Requires: /bin/sh libotf.so.0
+ Not a GUI app.

SHOULD:
 1) change license to LGPLv2+
 2) add examples directory to %doc of -devel package.  
 3) Source URL should be
   http://www.m17n.org/%{name}/%{name}-%{version}.tar.gz
Comment 4 Parag AN(पराग) 2008-03-28 04:55:34 EDT
Please submit updated SRPM here.
Comment 5 Neal Becker 2008-03-28 07:03:26 EDT
Updated SRPM:
http://nbecker.dyndns.org:8080/RPM/libotf-0.9.7-3.fc8.src.rpm

Minor: installing example also installs subdir .deps.  I recommend ignoring 
this because removing it will break the Makefile.
Comment 6 Parag AN(पराग) 2008-03-31 02:30:47 EDT
simple way then is to add to -devel 
%doc example/*.c
Comment 7 Parag AN(पराग) 2008-04-03 23:41:35 EDT
ping for updated SRPM
Comment 8 Neal Becker 2008-04-07 12:01:06 EDT
You mean only install example/*.c and not Makefile?  I think the original 
(including Makefile and .deps) is better - at least it has then a working 
Makefile.
Comment 9 Parag AN(पराग) 2008-04-14 07:02:04 EDT
I have not seen such case where .deps also got installed. Just search for .deps
under /usr/share/doc and afaics there is no .deps there.

so if you want .deps also then I will ask you to drop 
%doc example/*.c

Comment 10 Neal Becker 2008-04-14 08:26:36 EDT
OK, .deps removed

http://nbecker.fedorapeople.org/libotf-0.9.7-4.fc8.src.rpm
Comment 11 Parag AN(पराग) 2008-04-27 06:28:29 EDT
sorry I thought I posted review here but it seems I need to do that yet. Will
review this tomorrow.
Comment 12 Neal Becker 2008-05-04 12:58:30 EDT
(In reply to comment #11)
> sorry I thought I posted review here but it seems I need to do that yet.
Will
> review this tomorrow.

Ping?
Comment 13 Parag AN(पराग) 2008-05-04 23:53:12 EDT
sorry. I got so busy schedule at home and work also. will review now.
Comment 14 Parag AN(पराग) 2008-05-04 23:58:59 EDT
koji build http://koji.fedoraproject.org/koji/taskinfo?taskID=595436
Package looks Ok now.

APPROVED.
Comment 15 Neal Becker 2008-05-05 08:22:04 EDT
New Package CVS Request
=======================
Package Name: libotf
Short Description: Library for handling OpenType Font
Owners: nbecker
Branches: F-8 F-9
InitialCC:
Cvsextras Commits: yes
Comment 16 Kevin Fenzi 2008-05-05 11:47:01 EDT
cvs done.
Comment 17 Brian Pepple 2008-05-25 19:55:31 EDT
Neal, has this been built?  If so, you can close this bug.

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