Bug 204954

This is a (new) dependency for libtunepimp-0.5.x

{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

Crud, I had only previously tested this on rhel4.  Must be a gcc4 thing.  I'll
report it upstream.

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

Thanks, hidden within all the debian-isms in in there, is what appears to be a
gcc41 patch.

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)

build failure reported upstream:
http://forums.predixis.com/index.php?showtopic=2196

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.

Created attachment 136018 [details]
diff of libofa.spec

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

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
SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.testing/libofa-0.9.3-2.src.rpm

mock result(s):
http://kde-redhat.unl.edu/apt/kde-redhat/mock/fedora-6-i386-core-kde/libofa/

(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
-----------------------------------

Arg, didn't realize that the patch simply created another patch!

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

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.

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.

(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.

> 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.

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?

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

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

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

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

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)

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. (:

Make that SRPM URL:
http://kde-redhat.unl.edu/apt/kde-redhat/all/SRPMS.testing/libofa-0.9.3-7.src.rpm

(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.

Thanks for the excellent review, importing.