Bug 247615 - Review Request: TECkit - Conversion library and mapping compiler
Summary: Review Request: TECkit - Conversion library and mapping compiler
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jochen Schmitt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-07-10 13:12 UTC by Jindrich Novy
Modified: 2013-07-02 23:21 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2007-07-17 07:16:52 UTC
Type: ---
Embargoed:
jochen: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Jindrich Novy 2007-07-10 13:12:32 UTC
Spec URL: http://people.redhat.com/jnovy/files/teckit.spec
SRPM URL: http://people.redhat.com/jnovy/files/teckit-2.2.1-0.1.fc7.src.rpm
Description:
TECkit is a low-level toolkit intended to be used by other
applications that need to perform encoding conversions (e.g., when
importing legacy data into a Unicode-based application). The
primary component of the TECkit package is therefore a library that
performs conversions; this is the "TECkit engine". The engine
relies on mapping tables in a specific binary format (for which
documentation is available); there is a compiler that creates such
tables from a human-readable mapping description (a simple text file).

---
The main reason to include this is that TeXLive is dependent on it. TeXLive uses its internal version of TECkit and we may want to include/maintain it separately.

Comment 1 Jochen Schmitt 2007-07-10 16:08:49 UTC
Good:
+ Package name meets naming guidelines
+ Name of SPEC file matches with package name.
+ SPEC file consitant macro usage.
+ SPEC file contains %{?dist} macro
+ License is LGPL
+ Package has the choice to use CPL or LGPL
+ SPEC in English
+ SPEC is legible
+ Sub packages are declared properly
+ Tar ball matches with upstream
  (md5sum: 6b7538aa53aa844db8bb95c1219d62d1)
+ Package has correct buildroot
+ BuildRequires isn't redundant
+ Local build works properly
+ Package contains proper %defattr and file permissions
+ Package has a proper clean section
+ Buildroot will be cleand in %clean section and on the beginning of the
%install section
+ Package and sub packages doc files don't affect runtine
+ File size on the %doc section is acceptable
+ Package doesn't contains static libraries
+ Package file list doesn't contains duplicates files
+ Package contains no files or directories owned by other packages


Bad:
- %{_smp_mflags] wasn't used during make step.
Pleae add it or add a comment which described, that the build doesn't works
with it
- Package doesn't contains verbatim copy of the license
  The file COPYING refers to a text file which is not included in the package
- Rpmlint complaints on source rpm:
rpmlint -i teckit-2.2.1-0.1.fc7.src.rpm
E: teckit configure-without-libdir-spec
Because package use %configure macro, this may be ignored
- Package contains zero length file 
rpmlint teckit-2.2.1-0.1.fc7.x86_64.rpm
E: teckit zero-length /usr/share/doc/teckit-2.2.1/ChangeLog
- Rpmlint complaints on installed package:
rpmlint teckit
E: teckit zero-length /usr/share/doc/teckit-2.2.1/ChangeLog
W: teckit unused-direct-shlib-dependency /usr/lib64/libTECkit_Compiler.so.0.0.0
/lib64/libexpat.so.0
W: teckit unused-direct-shlib-dependency /usr/lib64/libTECkit_Compiler.so.0.0.0
/lib64/libm.so.6
W: teckit unused-direct-shlib-dependency /usr/lib64/libTECkit.so.0.0.0
/lib64/libexpat.so.0
W: teckit unused-direct-shlib-dependency /usr/lib64/libTECkit.so.0.0.0
/lib64/libm.so.6
- Mock build fails:
*** Recreating libtool files
libtoolize
./autogen.sh: line 7: libtoolize: command not found
*** Recreating aclocal.m4
aclocal
./autogen.sh: line 12: aclocal: command not found
*** Recreating configure
./autogen.sh: line 17: autoheader: command not found
./autogen.sh: line 18: autoconf: command not found
*** Recreating the Makefile.in files
./autogen.sh: line 22: automake: command not found

You have to define the require automake/autoconf tools as BRs.

Optimizing Hints:

If a entry in the %file section and with a slash, all files and directories
beyond it will be included into the package, so you may write

%{_includedir}/teckit/

instead of

%dir %{_includedir}/teckit/
%{_includedir}/teckit/TECkit_Common.h
%{_includedir}/teckit/TECkit_Compiler.h
%{_includedir}/teckit/TECkit_Engine.h



Comment 2 Jindrich Novy 2007-07-11 06:52:22 UTC
Ok, here we go with the new fixed TECkit:

http://people.redhat.com/jnovy/files/teckit/

The only remaining issue is:

W: teckit unused-direct-shlib-dependency /usr/lib64/libTECkit_Compiler.so.0.0.0
/lib64/libexpat.so.0
W: teckit unused-direct-shlib-dependency /usr/lib64/libTECkit_Compiler.so.0.0.0
/lib64/libm.so.6
W: teckit unused-direct-shlib-dependency /usr/lib64/libTECkit.so.0.0.0
/lib64/libexpat.so.0
W: teckit unused-direct-shlib-dependency /usr/lib64/libTECkit.so.0.0.0
/lib64/libm.so.6

but according to:
http://www.redhat.com/archives/fedora-packaging/2007-January/msg00158.html

and the following discussion it could be safely neglected.

Comment 3 Jochen Schmitt 2007-07-11 15:20:54 UTC
Fixed:
+ %{_smp_mflags} added
+ Empty file was removed from the %doc section
+ Include whole directory %{_include]/teckit/
+ Mock build works fine for Devel (x86_64, i386, ppc64, ppc)

Need work:
- License.txt refers to two other files which contains the verbatim text of the
licenses

Accepted unfixed issues:
The ununsed-direct-shlib-dependency issue is not a blocker for approvement.
It may be nice, if you can notify upstream for fixing it.

Comment 4 Jindrich Novy 2007-07-11 15:38:21 UTC
The remaining licenses are now added in 0.3. I notified upstream about it.

Comment 5 Jochen Schmitt 2007-07-11 19:49:30 UTC
I think you understand me not in the right way. I saw, that the license.txt
doesn't contains the verbatim copy of ever the LGPL or the CPL. But the package
should comtains the verbatim text of the license if abailable.

So your statement make no sense from my point of view. If you say, that the
package should release under the term of the LGPL, you have to distribute the
file with the verbatim text of the LGPL with your package.

Comment 6 Jindrich Novy 2007-07-12 10:42:52 UTC
Please reread guidelines regarding the package licenses:

http://fedoraproject.org/wiki/Packaging/Guidelines#LicenseText

<cite>
If (and only if) the source package includes the text of the license(s) in its
own file, then that file, containing the text of the license(s) for the package,
must be included as documentation.
</cite>

That means it's not required to add, say LGPL, license text separately when it's
not present in the package. The license files that comes with TECkit contain
direct links to the web for both CPL and LGPL so I don't see any reason to add
any of these license texts to TECkit separately.

Comment 7 Jindrich Novy 2007-07-12 10:49:29 UTC
Btw. what is wrong with License_CPLv05.txt and License_LGPLv21.txt that are
packaged with TECkit since 0.3? Aren't they verbatim texts of the licenses?

Comment 8 Jochen Schmitt 2007-07-12 14:08:45 UTC
Yes how Jindrich wrote, the package contains the verbatim copy of the license 
texts, so why do you don't want to include this files into the package?

An ohter situation is, if you can't find a verbatim copy of the license text in 
the upstream tar ball. In this case, you don't need to include a license text 
from another source.

Comment 9 Jindrich Novy 2007-07-12 14:51:49 UTC
Please check the teckit-2.2.1-0.3:

http://people.redhat.com/jnovy/files/teckit/

I added all the licenses present in the package in this version as I wrote in
comment #4. Seems like the problems are all fixed now ;-)

Comment 10 Jochen Schmitt 2007-07-12 17:49:41 UTC
OK, your package looks nice.

You are APPROVED !!!

Comment 11 Jindrich Novy 2007-07-13 12:47:00 UTC
New Package CVS Request
=======================
Package Name: TECkit
Short Description: Conversion library and mapping compiler
Owners: jnovy
Branches: 
InitialCC: 

Comment 12 Kevin Fenzi 2007-07-13 16:29:27 UTC
The spec/package appears to be 'teckit' here, not 'TECkit'... 
Shouldn't the CVS package name be 'teckit' ? Or are you intending to import it
as "TECkit' for some reason?

Comment 13 Jindrich Novy 2007-07-16 10:19:22 UTC
Let's rather name it lowercase, even though the TEC is a short for "Text
Encoding Conversion":

http://scripts.sil.org/cms/scripts/page.php?site_id=nrsi&cat_id=TECkit

Comment 14 Jindrich Novy 2007-07-16 10:21:08 UTC
New Package CVS Request
=======================
Package Name: teckit
Short Description: Conversion library and mapping compiler
Owners: jnovy
Branches: 
InitialCC:

Comment 15 Kevin Fenzi 2007-07-16 18:23:58 UTC
cvs done.

Comment 16 Jindrich Novy 2007-07-17 07:16:52 UTC
Imported & built successfully.


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