Bug 681976

Hi ,
It looks like the MD5 sums don't match.

http://openfst.cs.nyu.edu/twiki/pub/FST/FstDownload/openfst-1.2.7.tar.gz  has
md5sum 97196a97d2a1ec88d612321e64dac2e4  

where as  openfst-1.2.7.tar.gz from the .src.rpm has 7e7f70a1cf22a107fb445b4de6af8537.

It looks like upstream replaced the 1.2.7 release tarball that originally had February 4 timestamps in it with one containing March 11 timestamps, without changing the version number.  The diff between the two shows a number of bug fixes, and an entirely new class.  That was naughty of them.  I've replaced the SRPM with one containing the new tarball, same URL as before.

Review openfst-1.2.7-1
https://bugzilla.redhat.com/show_bug.cgi?id=681976

Builds okay with Fedora 16 x86_64 mock.

*** Notes worth reading.

- Package meets naming and packaging guidelines
Yes matches tar ball name.
- Spec file matches base package name.
It does.
- Spec has consistant macro usage.
Yes.
- Meets Packaging Guidelines.
*** Add %{?_isa} tags to BuildRequirs and internal Requires.
- License
ASL 2.0
- License field in spec matches
Very clear licensing with a COPYING file and headers on each source file/
- License file included in package
Yes.
- Spec in American English
Yes
- Spec is legible.
Yes.
- Sources match upstream md5sum:
Yes.
$ md5sum openfst-1.2.7.tar.gz ../SOURCES/openfst-1.2.7.tar.gz 
97196a97d2a1ec88d612321e64dac2e4  openfst-1.2.7.tar.gz
97196a97d2a1ec88d612321e64dac2e4  ../SOURCES/openfst-1.2.7.tar.gz
- BuildRequires correct
*** Add %{?_isa}.
- Spec handles locales/find_lang
Not needed
- Package is relocatable and has a reason to be.
Not relocatable.
- Package has %defattr and permissions on files is good.
It does.
- Package has a correct %clean section.
It does.
- Package has correct buildroot
Not needed anymore.
- Package is code or permissible content.
It does.
- Doc subpackage needed/used.
Not needed.

- Headers/static libs in -devel subpackage.
They are,
- Spec has needed ldconfig in post and postun
It does.
- .pc files in -devel subpackage/requires pkgconfig
No .pc files
- .so files in -devel subpackage.
They are.
- -devel package Requires: %{name} = %{version}-%{release}
*** It does but it should be  %{name}%{?_isa} = %{version}-%{release} these days.
- .la files are removed.
None present.
- Package is a GUI app and has a .desktop file
No GUI.
- Package compiles and builds on at least one arch.
Mock.
- Package has no duplicate files in %files.
None
- Package doesn't own any directories other packages own.
It does not.
- Package owns all the directories it creates.
It does.
- No rpmlint output.

This is quite verbose so I'll put in a sepearte comment.


- final provides and requires are sane:
=== openfst-1.2.7-1.fc16.src.rpm provides

=== openfst-1.2.7-1.fc16.src.rpm requires
libicu-devel  
=== openfst-1.2.7-1.fc16.x86_64.rpm provides
arc_lookahead-fst.so.0()(64bit)  
compact16_acceptor-fst.so.0()(64bit)  
compact16_string-fst.so.0()(64bit)  
compact16_unweighted_acceptor-fst.so.0()(64bit)  
compact16_unweighted-fst.so.0()(64bit)  
compact16_weighted_string-fst.so.0()(64bit)  
compact64_acceptor-fst.so.0()(64bit)  
compact64_string-fst.so.0()(64bit)  
compact64_unweighted_acceptor-fst.so.0()(64bit)  
compact64_unweighted-fst.so.0()(64bit)  
compact64_weighted_string-fst.so.0()(64bit)  
compact8_acceptor-fst.so.0()(64bit)  
compact8_string-fst.so.0()(64bit)  
compact8_unweighted_acceptor-fst.so.0()(64bit)  
compact8_unweighted-fst.so.0()(64bit)  
compact8_weighted_string-fst.so.0()(64bit)  
const16-fst.so.0()(64bit)  
const64-fst.so.0()(64bit)  
const8-fst.so.0()(64bit)  
ilabel_lookahead-fst.so.0()(64bit)  
libfstcompact.so.0()(64bit)  
libfstconst.so.0()(64bit)  
libfstfarscript.so.0()(64bit)  
libfstfar.so.0()(64bit)  
libfstlookahead.so.0()(64bit)  
libfstpdtscript.so.0()(64bit)  
libfstscript.so.0()(64bit)  
libfst.so.0()(64bit)  
olabel_lookahead-fst.so.0()(64bit)  
openfst = 1.2.7-1.fc16
openfst(x86-64) = 1.2.7-1.fc16

=== openfst-1.2.7-1.fc16.x86_64.rpm requires
libdl.so.2()(64bit)  
libdl.so.2(GLIBC_2.2.5)(64bit)  
libfstfar.so.0()(64bit)  
libfstscript.so.0()(64bit)  
libfst.so.0()(64bit)  
libicuuc.so.46()(64bit)  
/sbin/ldconfig  
=== openfst-debuginfo-1.2.7-1.fc16.x86_64.rpm provides
openfst-debuginfo = 1.2.7-1.fc16
openfst-debuginfo(x86-64) = 1.2.7-1.fc16

=== openfst-debuginfo-1.2.7-1.fc16.x86_64.rpm requires
=== openfst-devel-1.2.7-1.fc16.x86_64.rpm provides
openfst-devel = 1.2.7-1.fc16
openfst-devel(x86-64) = 1.2.7-1.fc16

=== openfst-devel-1.2.7-1.fc16.x86_64.rpm requires
libfstcompact.so.0()(64bit)  
libfstconst.so.0()(64bit)  
libfstfarscript.so.0()(64bit)  
libfstfar.so.0()(64bit)  
libfstlookahead.so.0()(64bit)  
libfstpdtscript.so.0()(64bit)  
libfstscript.so.0()(64bit)  
libfst.so.0()(64bit)  
openfst = 1.2.7-1.fc16
=== openfst-tools-1.2.7-1.fc16.x86_64.rpm provides
config(openfst-tools) = 1.2.7-1.fc16
openfst-tools = 1.2.7-1.fc16
openfst-tools(x86-64) = 1.2.7-1.fc16

=== openfst-tools-1.2.7-1.fc16.x86_64.rpm requires
config(openfst-tools) = 1.2.7-1.fc16
libdl.so.2()(64bit)  
libfstfarscript.so.0()(64bit)  
libfstfar.so.0()(64bit)  
libfstpdtscript.so.0()(64bit)  
libfstscript.so.0()(64bit)  
libfst.so.0()(64bit)  
libicudata.so.46()(64bit)  
libicui18n.so.46()(64bit)  
libicuuc.so.46()(64bit)  
openfst = 1.2.7-1.fc16


Items to fix/change/comment on.

1) Fix Add %{?_isa} where appropriate.

2) Wait to see next comment about rpmlint output.

$ rpmlint  ./openfst.spec 
./openfst.spec:68: E: hardcoded-library-path in ../../lib/.libs
./openfst.spec:70: E: hardcoded-library-path in ../../lib/.libs
./openfst.spec:72: E: hardcoded-library-path in ../../lib/.libs
./openfst.spec:74: E: hardcoded-library-path in ../../lib/.libs
./openfst.spec:78: E: hardcoded-library-path in ../../lib/.libs
./openfst.spec: W: invalid-url Source2: openfst-man.tar.xz
0 packages and 1 specfiles checked; 5 errors, 1 warnings.

For the hardcoded-library-path it's not that obvious exactly what is going
on but by x86_64 builds end up in the correct location so probably fine.

$ rpmlint ./openfst-1.2.7-1.fc16.src.rpm 
openfst.src: W: spelling-error %description -l en_US automata -> automats, automat, automate
openfst.src: W: spelling-error %description -l en_US determinization -> determination, deterministically, deterministic


determinization is probably okay in american-english I would have 
thought even it makes me cringe, maybe determining is better?

automa seems to be included twice so is probably intentional.


$ rpmlint  ./openfst-1.2.7-1.fc16.x86_64.rpm 
openfst.x86_64: W: spelling-error %description -l en_US automata -> automats, automat, automate
openfst.x86_64: W: spelling-error %description -l en_US determinization -> determination, deterministically, deterministic
openfst.x86_64: W: shared-lib-calls-exit /usr/lib64/libfstfarscript.so.0.0.0 exit.5
openfst.x86_64: W: shared-lib-calls-exit /usr/lib64/libfstpdtscript.so.0.0.0 exit.5
openfst.x86_64: W: shared-lib-calls-exit /usr/lib64/libfstscript.so.0.0.0 exit.5
openfst.x86_64: W: shared-lib-calls-exit /usr/lib64/libfst.so.0.0.0 exit.5
1 packages and 0 specfiles checked; 0 errors, 6 warnings.


You may want to at least submit bugs upstream about those exits.

$ rpmlint ./openfst-tools-1.2.7-1.fc16.x86_64.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint ./openfst-devel-1.2.7-1.fc16.x86_64.rpm 
openfst-devel.x86_64: W: no-documentation
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/compact16_acceptor-fst.so compact16_acceptor-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/compact8_unweighted-fst.so compact8_unweighted-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/compact64_unweighted-fst.so compact64_unweighted-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/compact64_unweighted_acceptor-fst.so compact64_unweighted_acceptor-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/compact8_acceptor-fst.so compact8_acceptor-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/arc_lookahead-fst.so arc_lookahead-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/olabel_lookahead-fst.so olabel_lookahead-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/compact16_unweighted-fst.so compact16_unweighted-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/const8-fst.so const8-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/compact64_acceptor-fst.so compact64_acceptor-fst.so.0.0.0
openfst-devel.x86_64: W: dangling-relative-symlink /usr/lib64/fst/compact64_string-fst.so compact64_string-fst.so.0.0.0

....


These are links from -devel package to the main package so they are 
all satisfied. You can get rid of the warnings of course by making non relative
with 
pushd

popd.

which would make the rpmlint a lot cleaner.

(In reply to comment #4)
> For the hardcoded-library-path it's not that obvious exactly what is going
> on but by x86_64 builds end up in the correct location so probably fine.

Some of the libraries need to be linked against each other.  That is the way I came up with to accomplish the linking.  There may be a prettier way to do it.  In any case, as you say, once the libraries are actually installed, all is well.

> determinization is probably okay in american-english I would have 
> thought even it makes me cringe, maybe determining is better?

Determinization is the process of converting a nondeterministic automaton into a deterministic automaton.  That word is the one chosen by finite automata theorists, so it is correct, although it understandably is not in the standard dictionary.

> automa seems to be included twice so is probably intentional.

You mean "automata", right?  Yes, that is the plural of automaton.

> You may want to at least submit bugs upstream about those exits.

Will do.  I hate it when library authors do that.

> These are links from -devel package to the main package so they are 
> all satisfied. You can get rid of the warnings of course by making non relative
> with 
> pushd
> 
> popd.
> 
> which would make the rpmlint a lot cleaner.

But it wouldn't do any good, since ldconfig would just put them back the way they were.  Those links are correct; rpmlint just can't cope with a .so in a -devel package pointing to a .so.* in another package.

So other than 

1) Fix Add %{?_isa} where appropriate.

which can be done before import APPROVED.

Steve.

New Package SCM Request
=======================
Package Name: openfst
Short Description: Weighted finite-state transducer library
Owners: jjames
Branches: f14 f15
InitialCC:

Git done (by process-git-requests).

openfst-1.2.7-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/openfst-1.2.7-1.fc15

openfst-1.2.7-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/openfst-1.2.7-1.fc14

openfst-1.2.7-1.fc14 has been pushed to the Fedora 14 testing repository.

openfst-1.2.7-1.fc14 has been pushed to the Fedora 14 stable repository.

openfst-1.2.7-1.fc15 has been pushed to the Fedora 15 stable repository.