Bug 178263

Ed - 

  I posted this just a bit too soon and it doesn't build yet.  Note that this
only builds on devel due to a bug fixed in gfortran-4.1.0 but not it 4.0.1. 
I'll post an update when I've got a working version.

Hi Orion, no worries!  I'll review when you say its ready.

Hey orion,

>   I posted this just a bit too soon and it doesn't build yet.  Note that this
> only builds on devel due to a bug fixed in gfortran-4.1.0 but not it 4.0.1. 
> I'll post an update when I've got a working version.

Why not just get it in for devel tree only at this tree. If the build issues are
really due to bugs in the compiler, those gfortran fixes may never get
backported to 4.0.1; unless I don't quite get what you mean.

A usefull speci file trick is the %exclude tag, with it you can probably
simplify your %files section down to:

%files
%defattr(-,root,root,-)
%doc COPYING Copyright README
%{_sysconfdir}/profile.d/ncarg.*sh
%{_bindir}/*
%{_libdir}/ncarg
%{_mandir}/man*/*.gz
%exclude %{_bindir}/ncargcc
%exclude %{_bindir}/ncargf90
%exclude %{_libdir}/*.a
%exclude %{_libdir}/ncarg/examples
%exclude %{_libdir}/ncarg/tests

%files devel
%{_bindir}/ncargcc
%{_bindir}/ncargf90
%{_includedir}/ncarg
%{_libdir}/*.a
%{_libdir}/ncarg/examples
%{_libdir}/ncarg/tests

Saves a few pages of listing binaries and takes care of all directory ownership
etc properly too

Okay, please download again and we'll start from there.  Subsequent versions
will get their revision updated.  Probably still need to shuffle files around.

Hi Orion, I don't have an FC5 system running at the moment to test
whether the ncarg binaries segfault or not.  So I'll have to skip that
part of the review for now.  In any case, here are the parts that I
could do:

good:
 + source matches upstream
 + specfile is legible
 + license is ok and correctly included
 + builds in mock FC5 i386
 + code not content -- although it apparently contains some basic 
     map, font (?), etc. data for creating the graphics
 + dir ownership and permissions looks OK
 + no shared libs and no *.la files

needswork:
 - please add an "unset NCARG" at the end of %build
 - please consider using Chris Chabot's suggestion in comment #4
 - rpmlint emits a few warnings/errors:

     OK to ignore:
     E: ncarg script-without-shellbang /etc/profile.d/ncarg.csh
     E: ncarg script-without-shellbang /etc/profile.d/ncarg.sh
     W: ncarg-devel no-documentation

     Please put all the tutorial bits (/usr/lib/ncarg/tutorial/)
     in the -devel package or perhaps in a -doc package:
     W: ncarg devel-file-in-non-devel-package 
        /usr/lib/ncarg/tutorial/c_colcon.c

     Probably, this should go in the devel package since I don't 
     think it gets used via any run-time (eg. plug-in) methods. 
     Or does it?
     W: ncarg devel-file-in-non-devel-package 
        /usr/lib/ncarg/graphcaps/aed.a

 - please put all the /usr/lib/lib*.a files in /usr/lib/ncarg/
   so that they don't pollute the /usr/lib/ namespace with 
   generic names
 - please have the devel package require the main package:
     "Requires: %{name} = %{version}-%{release}"

(In reply to comment #6)
> Hi Orion, I don't have an FC5 system running at the moment to test
> whether the ncarg binaries segfault or not.

I was able to verify that some of the examples from ncargex in X11 mode (-W 8)
run on FC5 without crashing.

> needswork:
>  - please add an "unset NCARG" at the end of %build

Why bother?  The %build and %install scripts are run as separate shell scripts,
so environment settings don't carry from one to the other AFAIK.

>      Please put all the tutorial bits (/usr/lib/ncarg/tutorial/)
>      in the -devel package or perhaps in a -doc package:

On a related note, it appears that ncargex, in the base package, requires the
example files in the ncarg-devel package.  Perhaps the ncargex package could be
moved to the -devel package, or as Ed suggests, move it all to a -doc package.

Thanks for the reviews:

- I prefer to list binaries explicitly - it has alerted me to problems before.
- Moved tutorial to -devel
- Moved .a to %{_libdir}/ncarg/ (and other stuff to %{_libdir}/ncarg/ncarg/
- Added devel Requires.
- Moveg ncargex to -devel
- aed.a is a text graphcad file, not a library.

http://www.cora.nwra.com/~orion/fedora/ncarg-4.4.1-2.src.rpm

Hi Orion, you've addressed all the comments and I just got a clean build 
in mock (fc5, i386) with the 4.4.1-2 SRPM.  Good!  And thanks to Wart 
(#7 above) for checking that the binaries don't segfault.

APPROVED.

Checked in and built on devel.