Some initial comments:

MUST FIX:
* Please explain in detail, why you want to include a static lib 
or disable it (--disable-static). Feel strongly encouraged to do the latter.

* /usr/bin/itpp-config is pretty much bugged and needs further treatment:
At minimum, please remove all traces of /usr/include, /usr/lib and (!) 
-L/usr/lib/gcc* in return values.

* Something is broken with the *-doc package:
# rpm -qlp itpp-doc-3.10.5-1.i386.rpm
/usr/share/doc/itpp-3.10.5
/usr/share/doc/itpp-3.10.5/AUTHORS
/usr/share/doc/itpp-3.10.5/ChangeLog
/usr/share/doc/itpp-3.10.5/INSTALL
/usr/share/doc/itpp-3.10.5/NEWS
/usr/share/doc/itpp-3.10.5/README
/usr/share/doc/itpp-3.10.5/TODO

I haven't tried to investigate, yet, but would guess on some issue related
_docdir and/or doxygen.

COULD FIX:
* rpmlint complains:
E: itpp-debuginfo script-without-shellbang
/usr/src/debug/itpp-3.10.5/itpp/base/itpp_version.h

[Header is executable. Not a real issue, can be ignored]


CONSIDER:
* --disable-dependency-tracking. This package is large, so it should have a
measureable impact on building ;)

MUST FIX:
* *-devel contains a *.pc
=> According to the Packaging Guidelines, *-devel MUST Require: pkgconfig

Hi Ralf, heres an update where I've tried to address all of the items in 
the two above comments:

  http://mitgcm.org/eh3/fedora_misc/itpp-3.10.5-3.src.rpm

Also, I found that the --disable-dependency-tracking shaved the build
time by 5% on my otherwise inactive laptop (6.33min vs. 6.02min).  I 
didn't add it to the spec file this time but probably will soon.

(In reply to comment #3)

Two minor issues:

1. 
# rpm -q --requires -p itpp-doc-3.10.5-3.fc5.i386.rpm | grep itpp
itpp = 3.10.5-3.fc5

AFAICT, itpp-doc contains devel-docs. Therefore, it probably should better
"require: itpp-devel" instead of itpp.

2. Something seems fishy with the configure script wrt. atlas:
On one hand it uses /usr/lib/atlas/lib* and /usr/include/atlas, on the other
hand, it reports:
...
External libs:
  - BLAS ........... : yes
    * MKL .......... : no
    * ACML ......... : no
    * ATLAS ........ : no
...

... I must be missing something ;)


and a major one:

IMO, the *.pc is still bugged. It contains this:
...
Libs: -L${libdir} -litpp -lfftw3 -llapack -latlas -lblas   -L/usr/lib/atlas
-L/usr/lib/gcc/i386-redhat-linux/4.1.1
-L/usr/lib/gcc/i386-redhat-linux/4.1.1/../../.. -lgfortranbegin -lgfortran -lm
-lgcc_s
...

The references to -L/usr/lib/gcc/i386-redhat-linux and lgcc_s definitely are
bugs. They must not be present inside of a *.pc.

I am not sufficiently familiar with gfortran (My last real encounter with
fortran dates back more than 15 years) to be able to judge on -lgfortranbegin
and -lgfortran, but I am inclined to think the same apply to them.

Well, what is the status of this review?

Hi Mamoru, the status is currently FE-NEW.

Ralf posted some helpful comments (thanks, Ralf!) and I'm working on 
them.  I intend to push an updated SRPM this weekend that fixes the 
*.pc library bits per comment #4.  And I'm hoping someone will sign on 
as a reviewer and help finish the process...  :-)

(In reply to comment #6)
> Hi Mamoru, the status is currently FE-NEW.
> 
> I intend to push an updated SRPM this weekend that fixes the 
> *.pc library bits per comment #4.  

Okay, I understand.

Hi folks, here's an update:

  http://mitgcm.org/eh3/fedora_misc/itpp-3.10.5-4.src.rpm

It addresses the following things:

 + sanitizes itpp.pc
 + --disable-dependency-tracking for ~5% faster builds (woohoo!)
 + fixes the macro-in-changelog rpmlint error

And I'm not going to make -doc depend on -devel (as per comment #4) 
since (1) there is no actual dependency (that is, its a perfectly 
viable devel package without the docs and vice-versa), (2) it causes 
rpmlint errors, and (3) I think it violates the principle of least 
surprise for end users.

Well, from my viewpoint:

1. From http://fedoraproject.org/wiki/Packaging/Guidelines :

* rpmlint
  - rpmlint is not silent.

E: itpp-debuginfo script-without-shebang
/usr/src/debug/itpp-3.10.5/itpp/base/itpp_version.h
    - This is because the permission of this file is incorrect.

W: itpp-devel no-documentation
    - I think this can be ignored.

W: itpp undefined-non-weak-symbol /usr/lib/libitpp.so.2.2.0
_ZTVN10__cxxabiv117__class_type_infoE
W: itpp undefined-non-weak-symbol /usr/lib/libitpp.so.2.2.0
_ZTISt13basic_fstreamIcSt11char_traitsIcEE
W: itpp undefined-non-weak-symbol /usr/lib/libitpp.so.2.2.0
_ZNSt13basic_fstreamIcSt11char_traitsIcEED1Ev
W: itpp undefined-non-weak-symbol /usr/lib/libitpp.so.2.2.0
_ZNSt13basic_fstreamIcSt11char_traitsIcEED0Ev
W: itpp undefined-non-weak-symbol /usr/lib/libitpp.so.2.2.0
_ZThn8_NSt13basic_fstreamIcSt11char_traitsIcEED1Ev
W: itpp undefined-non-weak-symbol /usr/lib/libitpp.so.2.2.0
_ZThn8_NSt13basic_fstreamIcSt11char_traitsIcEED0Ev
W: itpp undefined-non-weak-symbol /usr/lib/libitpp.so.2.2.0
_ZTv0_n12_NSt13basic_fstreamIcSt11char_traitsIcEED1Ev
W: itpp undefined-non-weak-symbol /usr/lib/libitpp.so.2.2.0
_ZTv0_n12_NSt13basic_fstreamIcSt11char_traitsIcEED0Ev
       ..... (continued)
     - Linkage is incorrect. You can check this by:
       $ ldd -r /usr/lib/libitpp.so.2.2.0
       Some people say that this is not a blocker, while
       other perple say this is a blocker. My opinion is, since this is a library
       and is thought to be used by other package, this warning IS a blocker for
       this package.

* Requires:
  - Check the Requires for -devel packages (see the section Requires of
    http://fedoraproject.org/wiki/Packaging/Guidelines: For -devel package,
    dependency for the package should be checked manually).

* BuildRequies:
  - redundant BuildRequires is found.
    * perl (included in mimimal buildroot)
    * tetex, tetex-dvips <- required by tetex-latex

* Timestamps
  - cp AUTHORS ChangeLog NEWS README TODO \
      $RPM_BUILD_ROOT/%{_docdir}/%{name}-%{version}
    * Use "cp -p"

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :
* MUST: If (and only if) the source package includes the text of the license(s)...
  - Include "COPYTING" in main package. This is a MUST item.

Hi Mamoru, thanks for the feedback.  I'll fix the license inclusion, the 
redundant BRs and the time-stamp issue and will post another SRPM.

So how did you get rpmlint to produce all the undefined-non-weak-symbol
warnings?  When I run rpmlint against all of the rpms generated here on 
my system, I only get these two lines:

  W: itpp-devel no-documentation
  E: itpp-debuginfo script-without-shebang 
     /usr/src/debug/itpp-3.10.5/itpp/base/itpp_version.h

which, in my opinion, are both safe to ignore.

(In reply to comment #10)
> Hi Mamoru, thanks for the feedback.  I'll fix the license inclusion, the 
> redundant BRs and the time-stamp issue and will post another SRPM.
> 
> So how did you get rpmlint to produce all the undefined-non-weak-symbol
> warnings?  

Try: "rpmlint itpp (with itpp installed)". This rpmlint can be gained
only when used for installed rpms. See:
https://www.redhat.com/archives/fedora-extras-list/2006-September/msg00825.html

Well, for now these warning can be ignored because I tried to link
against libitpp.so and it succeeded so this is NOT a blocker.

[tasaka1@localhost PROGRAM]$ cat itpp-check.cpp 
int main(){
  return 0;
}
[tasaka1@localhost PROGRAM]$ g++ -Wall -O2 -o itpp-check itpp-check.cpp
`itpp-config --libs` -L/usr/lib/atlas

However, I recommend that you report this to upstream.
Anyway, fix the dependency for -devel package.

> When I run rpmlint against all of the rpms generated here on 
> my system, I only get these two lines:
> 
>   W: itpp-devel no-documentation
>   E: itpp-debuginfo script-without-shebang 
>      /usr/src/debug/itpp-3.10.5/itpp/base/itpp_version.h
> 
> which, in my opinion, are both safe to ignore.

The first one (no-documentation issue) can be ignored, I think too.
The latter one is because:
-rwxr-xr-x    1 root    root  1542 Jul 12 18:33
/usr/src/debug/itpp-3.10.5/itpp/base/itpp_version.h
The permission should be 0644, not 0755.

Yes, I agree that the "ldd -r ..." business is not a blocker because I'm 
building custom software that uses the itpp header and libraries and it 
doesn't just compile and link -- it actually runs.  ;-)

And please be more explicit about your "fix the dependency for -devel 
package" comment.  The SRPM already has the line:

  Requires:       %{name} = %{version}-%{release}

for the -devel sub-package so what exactly needs to be fixed?

Some remark:

> [tasaka1@localhost PROGRAM]$ cat itpp-check.cpp 
> int main(){
>   return 0;
> }
> [tasaka1@localhost PROGRAM]$ g++ -Wall -O2 -o itpp-check itpp-check.cpp
> `itpp-config --libs` -L/usr/lib/atlas

The reason I had to add "-L/usr/lib/atlas" is that liblapack.so is
under /usr/lib/atlas but you deleted "-L/usr/lib/atlas" from
itpp-config and itpp.pc. "-L/usr/lib/atlas" is actually needed so
re-add this.

> Anyway, fix the dependency for -devel package.
What I mean is:
$ itpp-config --libs
-litpp -lfftw3 -llapack -latlas -lblas -lgfortranbegin -lgfortran -lm -lgcc_s
(as said in above, -L/usr/lib/atlas should be added).
This means that itpp-devel should also require
fftw-devel atlas-devel and gcc-gfortran .

Re: comment #12
> Yes, I agree that the "ldd -r ..." business is not a blocker

Means there are undefined symbols, preventing prelink from functioning.  This
really should be fixed.

Re: comment #13
> deps for -devel
Related to shared lib undefined symbols, the *library* ought to link against all
those things, not itpp-using apps, they ought need only:
-litpp

Here's a re-spin:

  http://mitgcm.org/eh3/fedora_misc/itpp-3.10.5-5.src.rpm

It should address everything mentioned in this review except the unresolved 
symbols.  I just don't have the time this weekend to delve into them.  If 
someone (anyone?) wants to submit patches to do the linking then by all 
means please do so!  :-)

(In reply to comment #14)
> Means there are undefined symbols, preventing prelink from functioning.  This
> really should be fixed.

Well, I have found that undefined non-weak symbols are in libstdc++.so.
Actually, unless this is fixed, I cannot compile the following:

[tasaka1@localhost PROGRAM]$ g++ -O2 -o itpp-check itpp-check.cpp -litpp -nostdlib 
[tasaka1@localhost PROGRAM]$ cat itpp-check.cpp
int main(){
  return 0;
}
[tasaka1@localhost PROGRAM]$ g++ -O2 -o itpp-check itpp-check.cpp -litpp -nostdlib
/usr/bin/ld: warning: cannot find entry symbol _start; defaulting to 080481c0
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../libitpp.so: undefined reference to
`std::runtime_error::runtime_error(std::basic_string<char,
std::char_traits<char>, std::allocator<char> > const&)'
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../libitpp.so: undefined reference to
`std::basic_ostringstream<char, std::char_traits<char>, std::allocator<char>
>::~basic_ostringstream()'
/usr/lib/gcc/i386-redhat-linux/4.1.1/../../../libitpp.so: undefined reference to
`virtual thunk to std::basic_iostream<char, std::char_traits<char>
>::~basic_iostream()'
..........

> Re: comment #13
> > deps for -devel
> Related to shared lib undefined symbols, the *library* ought to link against all
> those things, not itpp-using apps, they ought need only:
> -litpp

Explicit linking against external libraries are always needed when
header files in the library use some types, 
structure or so which are defined by other packages then the
header files include other external header files.
( I have not checked if this package falls under this case).

(In reply to comment #16)
> (In reply to comment #14)
> > Means there are undefined symbols, preventing prelink from functioning.  This
> > really should be fixed.
> 
> Well, I have found that undefined non-weak symbols are in libstdc++.so.
> Actually, unless this is fixed, I cannot compile the following:
> 
> [tasaka1@localhost PROGRAM]$ g++ -O2 -o itpp-check itpp-check.cpp -litpp
-nostdlib 

1. Why do you expect this to work? -nostdlib disables GCC's internal libs (such
a gcc_s, stdc++), so this is a non-issue.

2. There is nothing wrong in using undefined non-weak symbols. If this causes
prelink to fail, then prelink is broken ...

(In reply to comment #17)
> > [tasaka1@localhost PROGRAM]$ g++ -O2 -o itpp-check itpp-check.cpp -litpp
> -nostdlib 
> 
> 1. Why do you expect this to work? -nostdlib disables GCC's internal libs (such
> a gcc_s, stdc++), so this is a non-issue.
As you said, I _EXPLICITLY_ disabled default linkage provided by gcc and
this should success if libitpp.so is linked against libstdc++.so.6 properly.

> 2. There is nothing wrong in using undefined non-weak symbols. If this causes
> prelink to fail, then prelink is broken ...

I am not familiar with prelink mechanism, however explanation by Jakub:
http://sources.redhat.com/ml/libc-alpha/2003-05/msg00034.html
perhaps means that undefined non-weak symbols can cause some problems
with prelink.

(In reply to comment #18)
> (In reply to comment #17)
> > > [tasaka1@localhost PROGRAM]$ g++ -O2 -o itpp-check itpp-check.cpp -litpp
> > -nostdlib 
> > 
> > 1. Why do you expect this to work? -nostdlib disables GCC's internal libs (such
> > a gcc_s, stdc++), so this is a non-issue.
> As you said, I _EXPLICITLY_ disabled default linkage provided by gcc and
> this should success if libitpp.so is linked against libstdc++.so.6 properly.

Oops.. This breaks start up symbol, however, linkage aganst libstdc++.so.6
is broken, anyway.

(In reply to comment #19)
> (In reply to comment #18)
> > (In reply to comment #17)
> > > > [tasaka1@localhost PROGRAM]$ g++ -O2 -o itpp-check itpp-check.cpp -litpp
> > > -nostdlib 
> > > 
> > > 1. Why do you expect this to work? -nostdlib disables GCC's internal libs
(such
> > > a gcc_s, stdc++), so this is a non-issue.
> > As you said, I _EXPLICITLY_ disabled default linkage provided by gcc and
> > this should success if libitpp.so is linked against libstdc++.so.6 properly.
> 
> Oops.. This breaks start up symbol, however, linkage aganst libstdc++.so.6
> is broken, anyway.
Nope, you're simply trying to overengineer a non-issue.

(In reply to comment #20)
> (In reply to comment #19)
> > Oops.. This breaks start up symbol, however, linkage aganst libstdc++.so.6
> > is broken, anyway.
> Nope, you're simply trying to overengineer a non-issue.

Well, I don't this is a non-issue. At least, this is unwilling
problem. And this undefined non-weak symbols problem disappeard 
by linking against libstdc++.so.6 ( I confirmed ) , so the fix should be
done as such.

[tasaka1@localhost ~]$ ldd -r /usr/lib/libitpp.so
        linux-gate.so.1 =>  (0x004ba000)
        libfftw3.so.3 => /usr/lib/libfftw3.so.3 (0x00eb7000)
        liblapack.so.3 => /usr/lib/atlas/liblapack.so.3 (0x0064a000)
        libatlas.so.3 => /usr/lib/atlas/libatlas.so.3 (0x00f49000)
        libblas.so.3 => /usr/lib/atlas/libblas.so.3 (0x07673000)
        libgfortran.so.1 => /usr/lib/libgfortran.so.1 (0x003da000)
        libm.so.6 => /lib/libm.so.6 (0x00b1c000)
        libgcc_s.so.1 => /lib/libgcc_s.so.1 (0x00459000)
        libstdc++.so.6 => /usr/lib/libstdc++.so.6 (0x004bb000)
        libc.so.6 => /lib/libc.so.6 (0x00b98000)
        libpthread.so.0 => /lib/libpthread.so.0 (0x00465000)
        /lib/ld-linux.so.2 (0x00b7d000)

( after fix, no undefined non-weak symbol appears any more )

I agree with Ralf in comment #20.

If you (Mamoru) really want to straighten out this linkage issue
then please work with upstream to get it into their regular build 
system.

So, getting back to the actual review, are there any blockers left 
here?

(In reply to comment #22)
> I agree with Ralf in comment #20.
> 
> If you (Mamoru) really want to straighten out this linkage issue
> then please work with upstream to get it into their regular build 
> system.

?? You want to become the maintainer of this package, don't you?
Then it is you who should report this problem (undefined non-weak
problem) to upsteam because this problem is surely a _BUG_ of this
package and _YOU_ must fix this when someone opens bugzilla 
entry about this issue. Indeed in other review request I asked the
submitter to contact upstream to fix some problem.

Quick note on undefined-non-weak-symbols: these warnings can be hard to get rid
of sometimes. If no easy fix is found, i think it's better to ignore the
warnings rather than introduce traumatic patches that might cause other
problems. You'd want to at least verify that the package works on all arches,
i.e. link test examples to make sure... And yes, package maintainer should
report the problem upstream :-)

I am going to reiterate myself: There is no real problem in this case.

Ralf, thanks for the opinion, but many (including me and the reviewer) disagree.

(In reply to comment #22)
> So, getting back to the actual review, are there any blockers left 
> here?

Well,
* %{_datadir}/%{name}/ is not owned by any package. Main package
  should own this.

(In reply to comment #24)
> these warnings can be hard to get rid of sometimes.
For this package, this can be resolved by following:
------------------------------------
.................
%configure --with-blas="-latlas -lblas" --disable-dependency-tracking  \
  --with-docdir=%{_docdir}/%{name}-%{version} --disable-static

sed -i -e 's|-lgcc_s|-lgcc_s -lstdc++|' config.status
./config.status

make %{?_smp_mflags}
.................
------------------------------------
  ... and as this is actually a bug and can be fixed as above, this bug
  should be fixed before releasing in FE.

* Well, 
  
(In reply to comment #16)
> 
> > Re: comment #13
> > > deps for -devel
> > Related to shared lib undefined symbols, the *library* ought to link against all
> > those things, not itpp-using apps, they ought need only:
> > -litpp
> 
> Explicit linking against external libraries are always needed when
> header files in the library use some types, 
> structure or so which are defined by other packages then the
> header files include other external header files.
> ( I have not checked if this package falls under this case).

As I said, explicit linking is needed when 
header files includes other header files which are in other packages.
I checked this package by:

 ( for f in `rpm -ql itpp-devel` ; do cat $f 2>/dev/null | grep '^#[
\t]*include' | sed -e 's|^[ \t][ \t]*||' | sed -n -e 's|.*<\(.*\)>.*|\1|p' ;
done ) | sort | uniq 

and it seems it is not a case for this package.
Then as Rex said in the comment #14, I  doubt that 
itpp.pc and itpp-config should have:
"-lfftw3 -llapack -latlas -lblas   -L/usr/lib/atlas -lgfortranbegin -lgfortran
-lm -lgcc_s -lstdc++" .
Please check if this linkage is truly required.

(In reply to comment #26)
> Ralf, thanks for the opinion, but many (including me and the reviewer) disagree.

OK, Rex, elaborate which problems this issue causes in this particular case and
what it breaks.

(In reply to comment #27)
> (In reply to comment #22)
>
> (In reply to comment #24)
> > these warnings can be hard to get rid of sometimes.
> For this package, this can be resolved by following:
> ------------------------------------
> .................
> %configure --with-blas="-latlas -lblas" --disable-dependency-tracking  \
>   --with-docdir=%{_docdir}/%{name}-%{version} --disable-static
> 
> sed -i -e 's|-lgcc_s|-lgcc_s -lstdc++|' config.status
> ./config.status
Dead wrong - Never ever patch config.status.

> Dead wrong - Never ever patch config.status.
Agreed.  There needs to be a better/cleaner solution.

> OK, Rex, elaborate which problems this issue causes in this particular 
> case and what it breaks.

Undefined symbols in shared libraries is bad, period, especially if easily
avoidable (which it apparently is in this case).

(In reply to comment #31)
> > OK, Rex, elaborate which problems this issue causes in this particular 
> > case and what it breaks.
> 
> Undefined symbols in shared libraries is bad, period, especially if easily
> avoidable (which it apparently is in this case).
This is your personal preference on a particular design, but this doesn't cause
any malfunction.

glibc (c.f. man 3 getopt), libstdc++ and many other libs are heavily using such
constructs.
 

(In reply to comment #28)
> (In reply to comment #26)
> > Ralf, thanks for the opinion, but many (including me and the reviewer) disagree.
> 
> OK, Rex, elaborate which problems this issue causes in this particular case and
> what it breaks.
> 

The main problem for undefined non-weak symbols is that this may produce
symbols conflict. 
As you can see by "rpm -q --requires itpp-3.10.5-5.fc6", this package
(without this problem fixed) does NOT require libstdc++.so.6 (because
libitpp.so.2.2.0 is not linked against it) although it surely require it.
So this package can be installed which uses libstdc++.so.5, for example,
which surely cause symbol conflicts.

> As you can see by "rpm -q --requires itpp-3.10.5-5.fc6", this package
> (without this problem fixed) does NOT require libstdc++.so.6 (because
> libitpp.so.2.2.0 is not linked against it) although it surely require it.
> So this package can be installed which uses libstdc++.so.5, for example,
> which surely cause symbol conflicts.

Assuming a system that only has compat-libstdc++-33 installed but not libstdc++.
However nothing prevents you from manually adding the Requires: in the package.

(In reply to comment #34)
> Assuming a system that only has compat-libstdc++-33 installed but not libstdc++.
> However nothing prevents you from manually adding the Requires: in the package.

That I said "libstdc++.so.5" is only a example. The conflict can also occur
when soname of libstdc++ changes.

(In reply to comment #29)
> (In reply to comment #27)
> > (In reply to comment #22)
> >
> > (In reply to comment #24)
> Dead wrong - Never ever patch config.status.

If you say so, another example is

%build
chmod 644 itpp/base/itpp_version.h

for f in `find . -name Makefile.in` ; do
  sed -i -e '/^LIBS/s|^\(.*\)$|\1\nLIBS += -lstdc++|' $f
done

export LDFLAGS="-L/usr/lib/atlas"
export CPPFLAGS="-I/usr/include/atlas"
export F77=gfortran
...........

(In reply to comment #36)

> for f in `find . -name Makefile.in` ; do
>   sed -i -e '/^LIBS/s|^\(.*\)$|\1\nLIBS += -lstdc++|' $f
> done

The same applies to Makefile.in: Never ever patch them.

Sorry, forgot: Never ever explicitly link against libstdc++.

(In reply to comment #38)
> Sorry, forgot: Never ever explicitly link against libstdc++.

Explicit linking is needed because libtool (in this package) uses
-nostdlib.

(In reply to comment #38)
> Sorry, forgot: Never ever explicitly link against libstdc++.

Note: explicit linking against libgcc_s.so is done already (as -nostdlib
is used).

Hi Mamoru, thank you for pointing out the unowned dir in comment #27 and 
here's an update that fixes it:

  http://mitgcm.org/eh3/fedora_misc/itpp-3.10.5-6.src.rpm


In regards to the undefined symbols, I tried using:

  export LDFLAGS="-L/usr/lib/atlas -stdc++"

and it appears to do the trick.  However, it violates Ralf's assertion in 
comment #38 so I did not (for the above SRPM) add it to the spec.  Of 
course, its something that we can revisit.

So, are there any blockers left here?

Theres a missing character in the above comment.  It should have been:

  export LDFLAGS="-L/usr/lib/atlas -lstdc++"

(In reply to comment #41)

> In regards to the undefined symbols, I tried using:
> 
>   export LDFLAGS="-L/usr/lib/atlas -stdc++"
> 
> and it appears to do the trick.  However, it violates Ralf's assertion in 
> comment #38 so I did not (for the above SRPM) add it to the spec.  

This case explicit linking by "-lstdc++" is actually needed
( as I said in comment #39 and #40 ) because this package used "-nostdlic"
when linking (this is not unusual). So your linking

export LDFLAGS="-L/usr/lib/atlas -lstdc++"

is acceptable and must be done. Please add "-lstdc++".

-------

Then as I said in comment #27 ( and Rex said in comment #14 ), would you check
if "-lfftw3 -llapack -latlas -lblas   -L/usr/lib/atlas   -lgfortranbegin
-lgfortran -lm -lgcc_s -lstdc++" is really needed for
/usr/lib/itpp.pc and /usr/bin/itpp-config ?
I checked all header files in -devel package and no files are included from
external packages. If linkage is fixed by "-lstdc++", I think the above
("-lfftw3 ........" ) is  not necessary ( and should be deleted ).

(In reply to comment #43)
 
> Then as I said in comment #27 ( and Rex said in comment #14 ), would you check
> if "-lfftw3 -llapack -latlas -lblas   -L/usr/lib/atlas   -lgfortranbegin
> -lgfortran -lm -lgcc_s -lstdc++" is really needed for
> /usr/lib/itpp.pc and /usr/bin/itpp-config ?

Of course  -L${libdir} -litpp is needed anyway.

OK, this adds "-lstc++":

  http://mitgcm.org/eh3/fedora_misc/itpp-3.10.5-7.src.rpm

and both itpp-config and /usr/lib/pkgconfig/itpp.pc appear to work correctly
for me.

(In reply to comment #45)
> and both itpp-config and /usr/lib/pkgconfig/itpp.pc appear to work correctly
> for me.

Yes, it should work correctly. I just wonder whether
"-lfftw3 -llapack -latlas -lblas   -L/usr/lib/atlas   -lstdc++ -lgfortranbegin
-lgfortran -lm -lgcc_s" is REALLY needed for the two file (itpp-config &
itpp.pc) .

"-lfftw3....." is really needed when compiling libitpp.so (i.e., in
building this package).  However, once
linking is corectly done (as in -7 src.rpm) and since no header files include
header files in other packages, including "-lfftw3".... to itpp-config &
itpp.pc is perhaps redundant.

In short, I think that itpp.pc can be:
----------------------------------------------------------
prefix=/usr
exec_prefix=/usr
libdir=/usr/lib
includedir=/usr/include

Name: IT++
Description: IT++ is a C++ library of mathematical, signal processing, speech
processing, and communications classes and fu
nctions
Version: 3.10.5
URL: http://itpp.sourceforge.net/
Libs: -L${libdir} -litpp
Cflags: 
--------------------------------------------------------------------------

and itpp-config can be:
--------------------------------------------------------------------------
...............................
    --libs)
      echo -L${libdir} -litpp 
      ;;
    --libs-opt)
      echo -L${libdir} -litpp 
      ;;
    --libs-debug)
      echo -L${libdir} -litpp 
      ;;
........................................................
------------------------------------------------------------------------------

Please check if this is possible. Other things are okay.

Yes, but is it a _blocker_ for this review?

The points I'm trying to make are that (1) I'm rather confident that it 
works safely and correctly as it is currently packaged in -7, (2) I'm 
not 100% certain that what you suggest will work in all cases [how can 
we be certain?] and (3) what you suggest is a further deviation from 
the way upstream does things and therefore it should be done cautiously.

So, I prefer to leave it as-is.

(In reply to comment #47)
> Yes, but is it a _blocker_ for this review?
> 
> The points I'm trying to make are that 
> (1) I'm rather confident that it 
> works safely and correctly as it is currently packaged in -7, 

It is true.

> (2) I'm 
> not 100% certain that what you suggest will work in all cases [how can 
> we be certain?] 

As I explained, since linkage is now correct and header files are
"consistent", the external linkage should not be necessary.

> (3) what you suggest is a further deviation from 
> the way upstream does things and therefore it should be done cautiously.

Well, please report this argument to upstream. Anyway it is recommended
(I think) that the maintainer in Fedora and upstream has good connection.

> So, I prefer to leave it as-is.

Okay. Then for now I don't block this any longer.

----------------------------------------------------------------------------------------
   This package (itpp) is now APPROVED by me.

Hi Mamoru, thank you for the review!

The -7 version built successfully for devel, an FC-5 branch has been 
requested, and the undefined non-weak symbols issue has been reported 
to upstream:

http://sourceforge.net/tracker/index.php?func=detail&aid=1576333&group_id=37044&atid=418758

so I think that covers everything.

Thank you for reporting to upstream.

Well, I see you successfully imported itpp to FE-devel.
Now I checked that CVS has itpp FE-5 entry. So please build this against
FE-5. If it is done, you can close this bug.

Hey Ed: 

I don't see this package in owners.list. Can you please add it?
See: 

http://fedoraproject.org/wiki/Extras/Contributors#head-f6f080b4c48fe519c98a29364a740953f90179e7

Hi Kevin, thank you for pointing out the omission!