Red Hat Bugzilla – Bug 247615
Review Request: TECkit - Conversion library and mapping compiler
Last modified: 2013-07-02 19:21:30 EDT
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.
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
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.
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.
The remaining licenses are now added in 0.3. I notified upstream about it.
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.
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.
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?
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.
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 ;-)
OK, your package looks nice. You are APPROVED !!!
New Package CVS Request ======================= Package Name: TECkit Short Description: Conversion library and mapping compiler Owners: jnovy@redhat.com Branches: InitialCC:
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?
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
New Package CVS Request ======================= Package Name: teckit Short Description: Conversion library and mapping compiler Owners: jnovy@redhat.com Branches: InitialCC:
cvs done.
Imported & built successfully.