Bug 210824 - Review Request: harminv - Program and library for solving the harmonic inversion problem
Review Request: harminv - Program and library for solving the harmonic invers...
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
Depends On:
  Show dependency treegraph
Reported: 2006-10-15 16:40 EDT by Ed Hill
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-10-19 10:12:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Ed Hill 2006-10-15 16:40:03 EDT
Spec URL: http://mitgcm.org/eh3/fedora_misc/harminv.spec
SRPM URL: http://mitgcm.org/eh3/fedora_misc/harminv-1.3.1-4.fc5.src.rpm
Harminv is a free program (and accompanying library) to solve the
problem of "harmonic inversion."  Given a discrete, finite-length
signal that consists of a sum of finitely-many sinusoids (possibly
exponentially decaying), it determines the frequencies, decay
constants, amplitudes, and phases of those sinusoids.
Comment 1 Mamoru TASAKA 2006-10-16 11:30:56 EDT

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* BuildRequires:
  blas-devel, lapack-devel is not needed. All the libraries needed
  to rebuild harminv are in atlas-devel, removing blas-devel, lapack-devel
  from BuildRequires causes no problem for mockbuild.

* Requires :
  - /usr/lib/pkgconfig/harminv.pc says:


Name: Harminv
Description: harmonic inversion of time series by filter diagonalization
Version: 1.3.1
Libs: -L${libdir} -lharminv -llapack -lcblas -lf77blas -latlas -lm  
-L/usr/lib/atlas   -lgfortranbegin -lgfortran 
-lm -lgcc_s
Cflags: -I${includedir}
    This means -devel package should require atlas-devel, gcc-gfortran.
    ..... But is this really? It seems that -devel package should require
    only main package.

    I think this harminv.pc should be regarded as useless and just be removed
    from -devel package. Then "Requires: pkgconfig" can be removed from
    -devel package.
    If you want to leave harminv.pc, then fix the content or
    add atlas-devel to Requires for -devel package.

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :

3. Other things I have noticed :
* I think man page should be in main package.

* %configure F77=gfortran --enable-shared --disable-static --with-cxx

F77 is not the option of configure but the environment.
So This should be:

export F77=gfortran
%configure --enable-shared --disable-static --with-cxx
Comment 2 Ed Hill 2006-10-17 13:34:09 EDT
Hi Mamoru, thank you for the speedy feedback!  Here's a re-spin that 
hopefully fixes everything you listed:

Comment 3 Mamoru TASAKA 2006-10-17 14:03:07 EDT
(In reply to comment #2)
>   http://mitgcm.org/eh3/fedora_misc/harminv-1.3.1-5.src.rpm

This is okay.

 This package (harminv) is APPROVED by me.
Comment 4 Mamoru TASAKA 2006-10-17 22:14:59 EDT

I saw that you added blas-devel, lapack-devel to BuildRequires to fix
x86_64 compilation problem.
However this is incorrect because:

on i386:
Requires: libatlas.so.3 libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1.3)
libcblas.so.3 libf77blas.so.3 libgcc_s.so.1 libgfortran.so.1 libharminv.so.2
liblapack.so.3 libm.so.6 libm.so.6(GLIBC_2.0) libm.so.6(GLIBC_2.1)
libstdc++.so.6 libstdc++.so.6(CXXABI_1.3) rtld(GNU_HASH)
Processing files: harminv-devel-1.3.1-7.fc6

on x86_64:
Requires: libblas.so.3()(64bit) libc.so.6()(64bit) libc.so.6(GLIBC_2.2.5)(64bit)
libgcc_s.so.1()(64bit) libgfortran.so.1()(64bit) libharminv.so.2()(64bit)
liblapack.so.3()(64bit) libm.so.6()(64bit) libm.so.6(GLIBC_2.2.5)(64bit)
libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) rtld(GNU_HASH)

... linkage is different between i386/ppc and x86_64.

The main problem is not BuildRequires. It is that on x86/64 the needed
libraries are under /usr/lib64/atlas, not under /usr/lib/atlas .

Please fix the linkage so that harminv is linked against the same
libraries on i386/ppc & x86_64. Then I think adding blas-devel, lapack-devel
is not needed (and should be avoided for this case).
Comment 5 Ed Hill 2006-10-17 23:15:42 EDT
Good catch!  Its fixed.  I'll close after the FC-5 build succeeds.
Comment 6 Ed Hill 2006-10-19 10:12:07 EDT
Hi Mamoru, thanks again for the (speedy!) review and for pointing out the 
%{_libdir} issue.  The FC-5 build was successful.

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