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 210824 - Review Request: harminv - Program and library for solving the harmonic inversion problem
Summary: Review Request: harminv - Program and library for solving the harmonic invers...
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-10-15 20:40 UTC by Ed Hill
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-10-19 14:12:07 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Ed Hill 2006-10-15 20:40:03 UTC
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
Description: 
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 15:30:56 UTC
Well,

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:

-----------------------------------------------
prefix=/usr
exec_prefix=/usr
libdir=/usr/lib
includedir=/usr/include

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.

A.
    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.
B.
    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 17:34:09 UTC
Hi Mamoru, thank you for the speedy feedback!  Here's a re-spin that 
hopefully fixes everything you listed:

  http://mitgcm.org/eh3/fedora_misc/harminv-1.3.1-5.src.rpm

Comment 3 Mamoru TASAKA 2006-10-17 18:03:07 UTC
(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-18 02:14:59 UTC
Hello:

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-18 03:15:42 UTC
Good catch!  Its fixed.  I'll close after the FC-5 build succeeds.

Comment 6 Ed Hill 2006-10-19 14:12:07 UTC
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.