Bug 542759 - Review Request: mpqc - Ab-initio chemistry program
Summary: Review Request: mpqc - Ab-initio chemistry program
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Susi Lehtola
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 542765 542767
TreeView+ depends on / blocked
 
Reported: 2009-11-30 18:06 UTC by Carl Byington
Modified: 2011-09-18 18:22 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-03-29 22:10:47 UTC
Type: ---
Embargoed:
susi.lehtola: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Carl Byington 2009-11-30 18:06:35 UTC
Spec URL: http://www.five-ten-sg.com/mpqc.spec
SRPM URL: http://www.five-ten-sg.com/mpqc-2.3.1-11.fc12.src.rpm
Description: 
MPQC is the Massively Parallel Quantum Chemistry Program. It computes
properties of atoms and molecules from first principles using the time
independent Schrödinger equation. It runs on a wide range of architectures
ranging from individual workstations to symmetric multiprocessors to
massively parallel computers. Its design is object oriented, using the C++
programming language.

koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1838765


This is part of my project to get ghemical and its dependencies into Fedora.

ghemical buildrequires:
   f2c
   libSC-devel
   mopac7-devel
   oglappth-devel
   libghemical-data
libghemical buildrequires:
   f2c
   libSC-devel
   mopac7-devel
mopac7 buildrequires:
   f2c
mpqc provides libSC7, libSC-devel
oglappth provides oglappth-devel
f2c provides f2c

Comment 1 Susi Lehtola 2009-11-30 19:43:44 UTC
Well, this is a parallel program so MPI versions should be packaged as well. Currently the MPI stuff in Fedora is still in a bit of a state of flux, as the last tweaks are done to conform to the MPI guidelines

http://www.fedoraproject.org/wiki/PackagingDrafts/MPI

(these were approved by FESCo months ago).

Comment 2 Carl Byington 2009-11-30 20:36:38 UTC
For the purposes of ghemical, we just need the libSC stuff from mpqc. Those libraries are then used by ghemical. The only executable is a command line 'molrender' and a gui tkmolrender frontend. 

Do you have any suggested .spec changes to enable MPI versions?

Comment 3 Susi Lehtola 2009-11-30 21:45:54 UTC
I'll have to take a look later on. I remember I had a look at mpqc some time ago, but decided not to package it myself.

- Create the desktop file in %prep, not in %install.

- Is there any reason why you want to use macros for make and rm...?

- I think it would be better if the library package was just -libs.

- IMO the development package should be -devel, not libSC-devel.

- -data and -html should be BuildArch: noarch.

- -html should be called -doc.

- Drop Mandriva's obsoletes and provides. No need for them in Fedora.

- It's the Schrödinger equation, not Schrödinger...

Comment 4 Carl Byington 2009-11-30 22:11:53 UTC
desktop file - if we create it in prep, we still need a line in %install to install it. Does that buy us any clarity?

macros for make,rm - no, just the mandriva spec file had it that way. I presume all the ${__xxx} instances should be simply xxx - fixed in the next version.

library package name - that contains a whole bunch of libSC*.so objects, which is the primary purpose of that library. Same for the name of the devel package.

Schrödinger - an artifact of the web server sending the wrong charset. curl can properly fetch the spec file, which is utf-8.

data, html, obsoletes - fixed in the next version.

Comment 5 Susi Lehtola 2009-11-30 23:13:30 UTC
(In reply to comment #4)
> desktop file - if we create it in prep, we still need a line in %install to
> install it. Does that buy us any clarity?

Yes, since I find it a bit odd that you still create contents in %install.

> library package name - that contains a whole bunch of libSC*.so objects, which
> is the primary purpose of that library. Same for the name of the devel package.

.. but normally Fedora packages aren't named that way.

Comment 6 Carl Byington 2009-11-30 23:23:32 UTC
Is it acceptable to have package mpqc...src.rpm which builds mpqc-libs...rpm which contains (mostly) libSC*.so libraries?

Comment 7 Susi Lehtola 2009-11-30 23:58:27 UTC
(In reply to comment #6)
> Is it acceptable to have package mpqc...src.rpm which builds mpqc-libs...rpm
> which contains (mostly) libSC*.so libraries?  

Yes.

Comment 8 Carl Byington 2009-12-01 01:45:31 UTC
All the requires are fully versioned. %{libname} and %{develname} removed from all the packages. All the subpackages renamed to basename{-doc,-libs,-devel}. Destop files created in %prep. Patching moved from %build to %prep.

The one exception is the molrender subpackage in mpqc, but it seems that could be moved into the main mpqc package with the other mpqc binaries.

http://www.five-ten-sg.com/mpqc.spec
http://www.five-ten-sg.com/mpqc-2.3.1-11.fc12.src.rpm

Comment 9 Susi Lehtola 2009-12-01 09:08:18 UTC
- Are the .la files necessary for something? Get rid of them e.g. by running
 find %{buildroot}%{_libdir} -name *.la -exec rm -rf {} \;
at the end of %install.

- Try if --disable-static in %configure gets rid of the static library.

- Molrender needs to own
 %{_datadir}/molrender/
not just
 %{_datadir}/molrender/molrender.in

Comment 10 Carl Byington 2009-12-02 04:10:59 UTC
molrender subpackage merged into the main package, --disable-static removed the .a files, use find to remove the .la files. It seems those .la files were used in the subsequent ghemical.spec build. For now I added the full list of libSC*so files to the ghemical link step.

http://www.five-ten-sg.com/mpqc.spec
http://www.five-ten-sg.com/mpqc-2.3.1-12.fc12.src.rpm  

scratch build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1842643

Comment 11 Susi Lehtola 2009-12-02 14:57:51 UTC
rpmlint output:

mpqc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/molrender ['/usr/lib64']
mpqc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mpqc ['/usr/lib64']
mpqc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/scls ['/usr/lib64']
mpqc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/scpr ['/usr/lib64']
mpqc.x86_64: E: non-standard-executable-perm /usr/bin/chkmpqcout 0555
mpqc.x86_64: E: script-without-shebang /usr/bin/chkmpqcout
mpqc-data.noarch: W: no-documentation
mpqc-debuginfo.x86_64: E: debuginfo-without-sources
mpqc-devel.x86_64: E: description-line-too-long This package contains the header files and static libraries needed to build programs linked
mpqc-devel.x86_64: E: rpath-in-buildconfig /usr/bin/sc-config lines ['30']
mpqc-devel.x86_64: E: script-without-shebang /usr/bin/sc-mkf77sym
mpqc-libs.x86_64: E: description-line-too-long This package contains the shared libraries needed to run programs dynamically linked
mpqc-libs.x86_64: W: macro-in-%description %{bname}
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCmolecule.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCmolecule.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCmolecule.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libmpqc.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libmpqc.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libmpqc.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCisosurf.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCclass.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCclass.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCclass.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCmisc.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCmisc.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCmisc.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCdft.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCwfn.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCmbpt.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCoptions.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCsymmetry.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCsymmetry.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCsymmetry.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCpsi.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCscf.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCoint3.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCkeyval.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCkeyval.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCkeyval.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCbasis.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCbasis.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCbasis.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCscmat.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCintv3.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCintv3.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCintv3.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCsolvent.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCref.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCcontainer.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCoptimize.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCgroup.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCstate.so.7.1.0
mpqc-libs.x86_64: W: unstripped-binary-or-object /usr/lib64/libSCrender.so.7.1.0
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCrender.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCrender.so.7.1.0 exit@@GLIBC_2.2.5
mpqc-libs.x86_64: W: no-documentation
7 packages and 0 specfiles checked; 11 errors, 45 warnings.

Fix these (you don't need to mind about the shared-libs-calls-exit warnings). The unstripped-binary-or-object warnings probably are caused by wrong permissions of the libraries. The libraries need to be installed as 755.

Comment 12 Susi Lehtola 2009-12-02 14:59:31 UTC
Notice that the description of the -libs package uses a macro %{bname} that doesn't exist.

Also, the -devel package has a wrong summary, it should be something along the lines of "Development headers and libraries for %{name}".

Comment 13 Susi Lehtola 2009-12-02 15:02:09 UTC
Please don't mix %{name} and "mpqc" in %files, it makes the list a bit hard to read. And, you might consider shortening the changelog, just keep the latest Mandriva entry.

Comment 14 Susi Lehtola 2009-12-02 15:06:37 UTC
Furthermore: drop the BuildConflicts, since that package does not exist in Fedora. Instead add
 export F77=gfortran
to the beginning of %build to make the build process use gfortran instead of g77.

Since for now we are building a serial version, add --disable-parallel to %configure.

Comment 15 Susi Lehtola 2009-12-02 15:15:50 UTC
I'd change the description to "The Massively Parallel Quantum Chemistry Program".

Comment 16 Carl Byington 2009-12-03 00:14:13 UTC
Change description or summary?

Other fixes done - export F77, drop BuildConflicts, shorter changelog, typos in description and summaries fixed, permissions on installed .so files fixed (strange that an autoconf install installed those non-executable).

The interesting one was 'script-without-shebang /usr/bin/chkmpqcout' and also sc-mkf77sym. I am not sure how bash figured out that it should use Perl on a script with a first line of '#', but I patched those two in the .spec file.

scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1844954

Your rpmlint finds stuff that mine did not find. How are you invoking it?

Comment 17 Susi Lehtola 2009-12-03 04:51:36 UTC
(In reply to comment #16)
> Change description or summary?

Whoops; summary.

> Your rpmlint finds stuff that mine did not find. How are you invoking it?  

I'm just running it on the packages created by a mock build in F-12. No magic involved.

Some warnings you get only with installed packages, but I haven't got to that part, yet. (Besides, they aren't that important, normally.)

Comment 18 Susi Lehtola 2009-12-03 07:56:48 UTC
Please attach links to the new spec and srpm every time you make updates.

Comment 19 Carl Byington 2009-12-04 05:09:20 UTC
- re-enable parallel to try to use multiple cpus. ghemical only used 1 cpu, but perhaps that has nothing to do with the --disable-parallel here. 

http://www.five-ten-sg.com/mpqc.spec
http://www.five-ten-sg.com/mpqc-2.3.1-13.fc12.src.rpm  
http://koji.fedoraproject.org/koji/taskinfo?taskID=1848542

Comment 20 Susi Lehtola 2009-12-04 10:20:19 UTC
Umm. The main package contains the serial version, which MUST be compiled with --disable-parallel, especially since the mpich2 package does not currently obey the guidelines.

However, even though it is a serial version, it can still use threads to parallelize on shared-memory computers. So, please add --disable-parallel to %configure.

And, scrap the note about libmpich in the description. I'll try to have a look at the MPI stuff later on.

Comment 21 Susi Lehtola 2009-12-05 04:11:19 UTC
.. and since there is heavy linear algebra involved in the calculations, it might be worthwhile to compile this against ATLAS instead of plain BLAS. To do this, add Requires: atlas-devel, and configure
 --with-blas="-L%{_libdir}/atlas -lf77blas -latlas"

The same goes with the other ghemical-related packages.

Comment 22 Carl Byington 2009-12-05 19:00:33 UTC
parallel disabled again, did not help with ghemical on multi-core anyway.

atlas changes unnecessary, since something is already picking that up. 

ldd $(which mpqc) | grep atlas
        liblapack.so.3 => /usr/lib/atlas/liblapack.so.3 (0x021e7000)
        libf77blas.so.3 => /usr/lib/atlas/libf77blas.so.3 (0x076b1000)
        libcblas.so.3 => /usr/lib/atlas/libcblas.so.3 (0x0036a000)
        libatlas.so.3 => /usr/lib/atlas/libatlas.so.3 (0x077e4000)

ldd /usr/lib/libghemical.so.5 | grep atlas
        liblapack.so.3 => /usr/lib/atlas/liblapack.so.3 (0x00b6d000)
        libf77blas.so.3 => /usr/lib/atlas/libf77blas.so.3 (0x008c3000)
        libcblas.so.3 => /usr/lib/atlas/libcblas.so.3 (0x005c4000)
        libatlas.so.3 => /usr/lib/atlas/libatlas.so.3 (0x010fe000)

http://www.five-ten-sg.com/mpqc.spec
http://www.five-ten-sg.com/mpqc-2.3.1-14.fc12.src.rpm  
http://koji.fedoraproject.org/koji/taskinfo?taskID=1851625

Comment 23 Susi Lehtola 2009-12-08 18:53:39 UTC
By the way, you can remove the offset from the release. You have started numbering from release 11; it really should be started from 1. So the next spec should be revision 5.

Comment 24 Fabien Archambault 2009-12-08 19:26:55 UTC
For information I also tried to make a spec for mpqc at the same time as you and did not succeed as you did but here is my spec file : ftp://dedimarbo.ath.cx/pub/repo/specs/mpqc.spec

more information available (in French) at : http://forums.fedora-fr.org/viewtopic.php?id=44709
in particular the rpmlint errors and warning.

Comment 25 Carl Byington 2009-12-09 15:58:09 UTC
(In reply to comment #23)
It looks like the -11 was a typo that should have been -1. Can we renumber those at this point, or is there any data in koji that will get confused by eventually getting a -11 release that is different than the current -11?

Comment 26 Susi Lehtola 2009-12-09 17:06:16 UTC
Yes, normally that would be the case since upgrade paths would be broken; however the package is not yet in Fedora so we don't really need to care about such things.

Once you correct the release numbers you will probably get rpmlint warnings from the Mandriva stuff; at that stage I'd just scrap them and add a note in the first entry that the spec file is based on that of Mandriva (and maybe add a few names for the contributions).

Comment 29 Susi Lehtola 2009-12-29 17:12:00 UTC
Oh, you may want to add
 BuildRequires: libint-devel
so that you get libint support that speeds up the computations.

The build scripts don't seem to support off-root builds, which makes building MPI parallellized versions a bit difficult. We might want to leave those for later.

You still need to drop
 BuildRequires: mpich2-devel
which isn't necessary. And I suggest building against atlas instead of the reference BLAS & LAPACK.

Comment 30 Carl Byington 2010-01-02 19:28:48 UTC
done. I just added atlas-devel, since it won't build without blas-devel. It seems that removing blas-devel would require extensive patches to ./configure or ./configure.in, which is probably not what you meant.

http://www.five-ten-sg.com/mpqc.spec
http://www.five-ten-sg.com/mpqc-2.3.1-7.fc12.src.rpm  
http://koji.fedoraproject.org/koji/taskinfo?taskID=1898999

Comment 31 Susi Lehtola 2010-01-02 20:25:35 UTC
(In reply to comment #30)
> done. I just added atlas-devel, since it won't build without blas-devel. It
> seems that removing blas-devel would require extensive patches to ./configure
> or ./configure.in, which is probably not what you meant.
> 
> http://www.five-ten-sg.com/mpqc.spec
> http://www.five-ten-sg.com/mpqc-2.3.1-7.fc12.src.rpm  
> http://koji.fedoraproject.org/koji/taskinfo?taskID=1898999  

The spec file is still revision 6.

I haven't had a look at the spec file yet, but the magic trick is to define the blas library as
 --with-blas="-L%{_libdir}/atlas -lf77blas -latlas"
instead of
 --with-blas="-lblas".
in %configure. Anyway, you should get the idea.

Comment 33 Susi Lehtola 2010-01-05 08:09:54 UTC
You still have to fix the rpath issue. Use the same method as in mopac7.

$ rpmlint mpqc-*
mpqc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/molrender ['/usr/lib64']
mpqc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mpqc ['/usr/lib64']
mpqc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/scls ['/usr/lib64']
mpqc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/scpr ['/usr/lib64']
mpqc-data.noarch: W: no-documentation
mpqc-devel.x86_64: E: rpath-in-buildconfig /usr/bin/sc-config lines ['30']
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCmolecule.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libmpqc.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCclass.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCmisc.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCsymmetry.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCkeyval.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCbasis.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCintv3.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCrender.so.7.1.0 exit.5
7 packages and 0 specfiles checked; 5 errors, 10 warnings.

Comment 34 Susi Lehtola 2010-01-05 08:13:28 UTC
What's more, Fedora optimization flags aren't used in the build. You might want to ask upstream to fix this - the flags should be picked up by configure from the environment variables set by the %configure macro.

Comment 36 Susi Lehtola 2010-01-06 11:13:39 UTC
mpqc-data.noarch: W: no-documentation
mpqc-devel.x86_64: E: rpath-in-buildconfig /usr/bin/sc-config lines ['30']
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCmolecule.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libmpqc.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCclass.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCmisc.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCsymmetry.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCkeyval.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCbasis.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCintv3.so.7.1.0 exit.5
mpqc-libs.x86_64: W: shared-lib-calls-exit /usr/lib64/libSCrender.so.7.1.0 exit.5
7 packages and 0 specfiles checked; 1 errors, 10 warnings.

You still have the rpath..

Comment 37 Susi Lehtola 2010-01-06 12:21:07 UTC
MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK
MUST: The spec file for the package is legible and macros are used consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK

MUST: The License field in the package spec file must match the actual license. NEEDSWORK
- I can't find any file that is licensed only under GPLv2.
- I think the license should be just "GPLv2+ and LGPLv2+".

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. OK
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. OK

MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. NEEDSWORK
- Add COPYING.LIB to %doc.
- Some files in the library are under GPLv2+, so both COPYING and COPYING.LIB have to be in the package.

MUST: Header files must be in a -devel package. OK
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. OK
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. OK
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. OK

MUST: No file conflicts with other packages and no general names. NEEDSWORK
- There's a *lot* of stuff in the includedir, for instance
 /usr/include/math/
 /usr/include/chemistry/
 /usr/include/util/
I would suggest placing all of the include files in a mpqc-specific directory. You can probably do this with a configure option.

MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK

Comment 38 Carl Byington 2010-01-06 20:40:17 UTC
fixed. 
include files moved to /usr/include/mpqc/*
remove rpath from sc-config script

http://www.five-ten-sg.com/mpqc.spec
http://www.five-ten-sg.com/mpqc-2.3.1-10.fc12.src.rpm  
http://koji.fedoraproject.org/koji/taskinfo?taskID=1905748

Comment 39 Susi Lehtola 2010-01-06 22:48:47 UTC
Oh, there's still something a bit odd:

/usr/share/man/man3/LocalHSOSContribution.3.gz
/usr/share/man/man3/LocalHSOSEnergyContribution.3.gz
/usr/share/man/man3/LocalHSOSGradContribution.3.gz
/usr/share/man/man3/MPQC_CartesianIterCCA.3.gz
/usr/share/man/man3/TCPClientConnection.3.gz
/usr/share/man/man3/TCPIOSocket.3.gz
/usr/share/man/man3/TCPServerConnection.3.gz
/usr/share/man/man3/TCPServerSocket.3.gz
/usr/share/man/man3/TCPSocket.3.gz
/usr/share/man/man3/Taylor_Fjt_Eval.3.gz
/usr/share/man/man3/YYSTYPE.3.gz
/usr/share/man/man3/errno_exception.3.gz
/usr/share/man/man3/point.3.gz

and

/usr/share/man/man3/vertex.3.gz
/usr/share/man/man3/vertices.3.gz

that are in -devel. These are way too generic for my taste. The other man3 files are prefixed with sc_, so maybe those could be renamed. You could contact upstream about this..

Comment 41 Susi Lehtola 2010-01-07 07:05:19 UTC
OK. One final comment: use -p in
 cp -r doc/man %{buildroot}%{_datadir}
to preserve time stamps.

APPROVED

Comment 43 Carl Byington 2010-01-08 17:26:33 UTC
New Package CVS Request
=======================
Package Name: mpqc
Short Description: Ab-inito chemistry program
Owners: carllibpst
Branches: F-11 F-12
InitialCC:

Comment 44 Kevin Fenzi 2010-01-09 04:32:13 UTC
cvs done.

Comment 45 Susi Lehtola 2010-03-29 22:26:51 UTC
You should remember to mark the review bug as closed when you do the first updates (first release), so you won't have to remember to close it yourself :)


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