Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 204954 - Review Request: libofa - Open Fingerprint Architecture library
Summary: Review Request: libofa - Open Fingerprint Architecture library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-09-01 17:30 UTC by Rex Dieter
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-12 17:00:25 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Mock build log of libofa with debian gcc patch applied (52.93 KB, text/plain)
2006-09-11 16:40 UTC, Mamoru TASAKA
no flags Details
diff of libofa.spec (946 bytes, patch)
2006-09-11 16:43 UTC, Mamoru TASAKA
no flags Details | Diff
Mock build log of libofa-0.9.3-6 (failed) (51.45 KB, text/plain)
2006-09-12 16:21 UTC, Mamoru TASAKA
no flags Details

Description Rex Dieter 2006-09-01 17:30:34 UTC
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/libofa.spec
SRPM URL: http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.stable/libofa-0.9.3-1.src.rpm
Description:
Currently, MusicDNS and the Open Fingerprint Architecture are being used to:
* identify duplicate tracks, even when the metadata is different, MusicIP
  identifies the master recording.
* fix metadata
* find out more about tracks by connecting to MusicBrainz

Comment 1 Rex Dieter 2006-09-01 17:44:18 UTC
This is a (new) dependency for libtunepimp-0.5.x

Comment 2 Parag AN(पराग) 2006-09-05 10:23:26 UTC
{Not Official Reviewer}
packaging looks ok in SPEC.
- Mockbuild is FAILED for i386 FC6
JAMA/tnt_math_utils.h:33: error: call of overloaded 'abs(const float&)' is ambiguous
/usr/include/stdlib.h:786: note: candidates are: int abs(int)
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../../include/c++/4.1.1/cstdlib:172:
note:                 long long int __gnu_cxx::abs(long long int)
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../../include/c++/4.1.1/cstdlib:142:
note:                 long int std::abs(long int)
make[3]: *** [mainprint.lo] Error 1
make[3]: Leaving directory `/builddir/build/BUILD/libofa-0.9.3/lib'
make[2]: *** [all-recursive] Error 1

+ dist tag is present
+ Buildroot is correct
- source URL is NOT working
+ License used is GPL
+ License file COPYING is included
+ Devel package is handled correclty in SPEC
- No upstream tarball available to verify its MD5



Comment 3 Rex Dieter 2006-09-06 14:46:09 UTC
Crud, I had only previously tested this on rhel4.  Must be a gcc4 thing.  I'll
report it upstream.

Comment 4 Mamoru TASAKA 2006-09-11 12:06:36 UTC
Perhaps debian's patch for lib/JAMA/tnt_math_utils.h works
( I have not yet checked debian's patch)

http://ftp.debian.org/debian/pool/main/libo/libofa/libofa_0.9.3-1.diff.gz

Comment 5 Rex Dieter 2006-09-11 14:00:22 UTC
Thanks, hidden within all the debian-isms in in there, is what appears to be a
gcc41 patch.

Comment 6 Rex Dieter 2006-09-11 16:08:12 UTC
Turns out the patch doesn't seem to help any, build dies similarly as before: 
...
 g++ -DHAVE_CONFIG_H -I. -I. -I.. -I../include -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4
-m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -Wall -g -MT
mainprint.lo -MD -MP -MF .deps/mainprint.Tpo -c mainprint.cpp  -fPIC -DPIC -o
.libs/mainprint.o
JAMA/tnt_math_utils.h: In function 'Real TNT::hypot(const Real&, const Real&)
[with Real = float]':
JAMA/jama_svd.h:73:   instantiated from 'JAMA::SVD<Real>::SVD(const
TNT::Array2D<T>&) [with Real = float]'
mainprint.cpp:151:   instantiated from here
JAMA/tnt_math_utils.h:33: error: call of overloaded 'abs(const float&)' is ambiguous
/usr/include/stdlib.h:786: note: candidates are: int abs(int)
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../../include/c++/4.1.1/cstdlib:172:
note:                 long long int __gnu_cxx::abs(long long int)
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../../include/c++/4.1.1/cstdlib:142:
note:                 long int std::abs(long int)


Comment 7 Rex Dieter 2006-09-11 16:18:35 UTC
build failure reported upstream:
http://forums.predixis.com/index.php?showtopic=2196

Comment 8 Mamoru TASAKA 2006-09-11 16:40:26 UTC
Created attachment 136017 [details]
Mock build log of libofa with debian gcc patch applied

Umm?

For me (gcc-4.1.1-20.i386), mock build succeeded.

Comment 9 Mamoru TASAKA 2006-09-11 16:43:38 UTC
Created attachment 136018 [details]
diff of libofa.spec

Diff between original 0.9.3-1 spec file and the one I used.

Comment 11 Mamoru TASAKA 2006-09-12 02:18:49 UTC
(In reply to comment #10)
> Here's what I used that still doesn't build (for me):
> Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/libofa.spec

Yes, surely by your spec file, mock build fails.

When you apply debian's patch by %patch1 -p1, it creates debian/ directory
and debian-original files are made under the directory. You can see 
the "real patch" is created under debian/patches directory.

So next time you have to apply the "real patch" under debian/patches
created by %patch1 -p1. The spec file I used includes:

-----------------------------------
%patch0 -p1 -b .deb
patch -p1 -b --suffix .gcc41 < debian/patches/01_gcc41.diff
-----------------------------------



Comment 12 Rex Dieter 2006-09-12 02:21:22 UTC
Arg, didn't realize that the patch simply created another patch!

Comment 13 Rex Dieter 2006-09-12 02:37:36 UTC
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/libofa.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.testing/libofa-0.9.3-3.src.rpm

%changelog
* Mon Sep 11 2006 Rex Dieter <rexdieter[AT]users.sf.net> 0.9.3-3
- use gcc41 patch extracted from debian's patchset



Comment 14 Mamoru TASAKA 2006-09-12 06:36:43 UTC
Okay. I will review this package.

First review:

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* rpmlint
  - is not silent.
    W: libofa wrong-file-end-of-line-encoding /usr/share/doc/libofa-0.9.3/README
    - The following files have wrong (Windows-like) end-of-line
      encoding.

libofa-0.9.3-3.fc6/libofa-0.9.3-gcc41.patch
libofa-0.9.3-3.fc6/usr/share/doc/libofa-0.9.3/README
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/AFLIB/aflibConverter.cpp
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/AFLIB/aflibConverter.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/AFLIB/aflibConverterLargeFilter.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/AFLIB/aflibConverterSmallFilter.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/JAMA/jama_svd.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/JAMA/tnt_array1d.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/JAMA/tnt_array2d.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/JAMA/tnt_math_utils.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/fft_op.cpp
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/fftlibw3_op.cpp
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/frametracker_op.cpp
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/frametracker_op.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/trackdata_op.cpp
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/trackdata_op.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/trackframe_op.cpp
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/trackframe_op.h
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/tracklist_op.cpp
libofa-debuginfo-0.9.3-3.fc6/usr/src/debug/libofa-0.9.3/lib/tracklist_op.h
libofa-devel-0.9.3-3.fc6/usr/include/ofa1/ofa.h

    W: libofa mixed-use-of-spaces-and-tabs (this is srpm)
    W: libofa-devel no-documentation (ignored)

* Requires:
  - Requires for -devel package cannot be supplied automatically, which must be
found by
  manual check.
  libofa-devel-0.9.3-3.fc6/usr/lib/pkgconfig/libofa.pc says:

Requires: fftw3
Libs: -L${libdir} -lofa -lexpat -lm
Cflags: -I${includedir}

  This means that libofa-devel requires fftw3-devel, expat-devel (and
  libofa).

 2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
* MUST: The sources used to build the package....
  Umm? I cannot find Source0. I can find .tar.gz.

3. Other things I have noticed:
   = Nothing.

Comment 15 Rex Dieter 2006-09-12 13:06:49 UTC
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/libofa.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.testing/libofa-0.9.3-4.src.rpm

%changelog
* Tue Sep 12 2006 Rex Dieter <rexdieter[AT]users.sf.net> 0.9.3-4
- remove extrenous entries from libofa.pc
- dos2unix README
- fix url in Source0
- -devel: %doc examples/

NOTE: can't change encoding of gcc41.patch, since the file it patches is a DOS
text file.

Comment 16 Paul Howarth 2006-09-12 13:58:17 UTC
(In reply to comment #15)
> %changelog
> * Tue Sep 12 2006 Rex Dieter <rexdieter[AT]users.sf.net> 0.9.3-4
> - remove extrenous entries from libofa.pc
> - dos2unix README
> - fix url in Source0
> - -devel: %doc examples/
> 
> NOTE: can't change encoding of gcc41.patch, since the file it patches is a DOS
> text file.

Actually you could if you did a dos2unix on the file to be patched before
applying the patch.


Comment 17 Rex Dieter 2006-09-12 14:00:19 UTC
> Actually you could if you did a dos2unix on the file to be patched before
> applying the patch.

For no other purpose than to make rpmlint happy?  No thanks.

Comment 18 Mamoru TASAKA 2006-09-12 14:22:59 UTC
Second review:

* rpmlint
  - is not yet silent:

W: libofa macro-in-%changelog doc
W: libofa mixed-use-of-spaces-and-tabs (this is for src.rpm)
W: libofa-devel hidden-file-or-dir /usr/share/doc/libofa-devel-0.9.3/examples/.deps
W: libofa-devel hidden-file-or-dir /usr/share/doc/libofa-devel-0.9.3/examples/.deps

    - Use %%doc, not %doc in %changelog entry.
    - Remove spaces or tabs in spec file and unify spacing.
    - examples/.deps is needed?
  - Well, this wiki page discourages to use unix2dos and actually, unix2dos
    is not needed. Also, changing the end-of-file encodings of souce files
    (i.e. .h or .cpp files) is done by:

    for f in `find . -name README -or -name \*.cpp -or -name \*.h` ; do \
        sed -i -e 's|\r||' $f ; done

    in %prep stage. Then removing DOS-like encoding of patch file (Patch1) can be
    done. Changing the end-of file encoding of \*.cpp, \*.h files is needed
    because they are finally included in debuginfo rpm.

* Well, removing external dependency from .pc file is correct?

Comment 19 Rex Dieter 2006-09-12 15:30:54 UTC
> * Well, removing external dependency from .pc file is correct?

Yeah, I'm pretty sure.  If problems arise, we can always add them back.

Comment 20 Rex Dieter 2006-09-12 15:33:04 UTC
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/libofa.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.testing/libofa-0.9.3-5.src.rpm

%changelog
* Tue Sep 12 2006 Rex Dieter <rexdieter[AT]users.sf.net> 0.9.3-5
- use sed instead of dos2unix
- omit examples/.deps


Comment 21 Rex Dieter 2006-09-12 15:45:16 UTC
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/libofa.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.testing/libofa-0.9.3-6.src.rpm

%changelog
* Tue Sep 12 2006 Rex Dieter <rexdieter[AT]users.sf.net> 0.9.3-6
- de-DOS'ify .cpp, .h files too


Comment 22 Mamoru TASAKA 2006-09-12 16:21:28 UTC
Created attachment 136085 [details]
Mock build log of libofa-0.9.3-6 (failed)

I cannot rebuild -6 src.rpm

+ rm -rf rpmdocs
+ mkdir -p rpmdocs
+ cp -a examples rpmdocs/
+ make -C rpmdocs/examples clean
make: Entering directory `/builddir/build/BUILD/libofa-0.9.3/rpmdocs/examples'
cd .. && make  am--refresh
make[1]: Entering directory `/builddir/build/BUILD/libofa-0.9.3/rpmdocs'
make[1]: *** No rule to make target `am--refresh'.  Stop.
make[1]: Leaving directory `/builddir/build/BUILD/libofa-0.9.3/rpmdocs'
make: *** [../configure] Error 2
make: Leaving directory `/builddir/build/BUILD/libofa-0.9.3/rpmdocs/examples'
error: Bad exit status from /var/tmp/rpm-tmp.25351 (%install)

Comment 23 Rex Dieter 2006-09-12 16:24:51 UTC
Spec URL: http://kde-redhat.unl.edu/apt/kde-redhat/SPECS/libofa.spec
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.testing/libofa-0.9.3-6.src.rpm

%changelog
* Tue Sep 12 2006 Rex Dieter <rexdieter[AT]users.sf.net> 0.9.3-7
- fix rpmdoc handling

Serves me right for not actually testing the build first. (:

Comment 25 Mamoru TASAKA 2006-09-12 16:46:12 UTC
(In reply to comment #24)
> Make that SRPM URL:
> http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.testing/libofa-0.9.3-7.src.rpm

Okay.

Well, rpmlint complaint for src.rpm still exists.
W: libofa mixed-use-of-spaces-and-tabs
This can be fixed by using only spaces, not tabs. Consider
to use only spaces or tabs.

Aside for it, no problems is left.

--------------------------------------------------------------
       This package (libofa) is APPROVED by me.


Comment 26 Rex Dieter 2006-09-12 17:00:25 UTC
Thanks for the excellent review, importing.


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