[Bug 542760] New: Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

Summary: Review Request: mopac7 - Semi-empirical quantum mechanics suite

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Summary: Review Request: mopac7 - Semi-empirical quantum
                    mechanics suite
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora

Spec URL: http://www.five-ten-sg.com/mopac7.spec
SRPM URL: http://www.five-ten-sg.com/mopac7-1.15-1.fc12.src.rpm
Description: 
MOPAC7 is a semi-empirical quantum-mechanics code written by James J. P.
Stewart and co-workers. The purpose of this project is to maintain MOPAC7 as
a stand-alone program as well as a library that provides the functionality
of MOPAC7 to other programs.

no scratch build yet, since this depends on f2c

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

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Carl Byington  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |542740

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Carl Byington  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |542765

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Carl Byington  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |542767

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Jussi Lehtola  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Jussi Lehtola   2009-11-30 14:47:08 EDT ---
Why don't you package OpenMopac 7.1? It's still under maintenance, MOPAC7 is
not...

http://openmopac.net/

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #2 from Carl Byington   2009-11-30 15:30:32 EDT ---
http://openmopac.net/Downloads/Mopac_7.1source.zip only contains microsoft .dsp
style make files, nothing to help with building on linux.

http://www.uku.fi/~thassine/projects/download/current/mopac7-1.15.tar.gz is the
version used by the author of ghemical.

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #3 from Carl Byington   2009-12-23 16:29:32 EDT ---
http://www.five-ten-sg.com/mopac7.spec
http://www.five-ten-sg.com/mopac7-1.15-5.fc12.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=1888970

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #4 from Jussi Lehtola   2009-12-23 17:01:21 EDT ---
rpmlint output:

mopac7.x86_64: E: no-binary
mopac7-devel.x86_64: W: no-documentation
mopac7-libs.x86_64: W: no-documentation

What you are actually doing with
 sed "s/./src//usr/bin/" run_mopac7 > %{buildroot}%{_bindir}/run_mopac7
is replacing the binary with a temporary libtool wrapper. What's the actual
problem you want to solve?

The sed line looks like you could do with another separator than /, using e.g.
| would make the expressions a lot neater.

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #5 from Jussi Lehtola   2009-12-23 17:02:45 EDT ---
Besides, you are shipping makefiles in the documentation.

Try if
 %check
 make test
works, if it does then IMHO you don't need to ship the tests directory at all.

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #6 from Carl Byington   2009-12-23 18:24:18 EDT ---
fixed. The .dat files in ./test seem to be samples that could also be used for
testing. It seems reasonable to install those in %doc (without the Makefiles).
The sed expressions were from mandriva - cleaned up now, and installed the real
binary rather than the libtool wrapper. run_mopac7 is a convenience fortran
wrapper that needs to reference the installed mopac7 binary. Not caught earlier
since ghemical just needs the libraries, not the main mopac7 binary.

http://www.five-ten-sg.com/mopac7.spec
http://www.five-ten-sg.com/mopac7-1.15-6.fc12.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=1889228

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Jussi Lehtola  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #7 from Jussi Lehtola   2009-12-23 18:56:02 EDT ---
rpmlint output:
mopac7.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mopac7 ['/usr/lib64']
mopac7-devel.x86_64: W: no-documentation
mopac7-libs.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 1 errors, 2 warnings.

Get rid of the rpath.
http://fedoraproject.org/wiki/Packaging/Guidelines#Removing_Rpath

**

You have
 %{_bindir}/%{name}
 %{_bindir}/run_mopac7
in files, better change %{name} to mopac7.

**

Btw, the build process doesn't seem to use f2c at all - gfortran is used to
compile the Fortran stuff.

**

There is no CCOPTIONS variable in the makefiles, drop CCOPTIONS="%{optflags}"
from make. The used flags are already picked up by %configure.

**

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
- I think you could do with a few clarifying comments in the %install section.

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.
OK
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. N/A

MUST: All relevant items are included in %doc. Items in %doc do not affect
runtime of application.
- Place the %doc in -libs instead of the main package, as the main package
requires the libraries but not vice versa.

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'. OK
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. OK
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages andno general names. OK
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

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #8 from Carl Byington   2009-12-23 20:33:50 EDT ---
Fixed; only the 64 bit version has rpath? Needs chrpath to get rid of it.

http://www.five-ten-sg.com/mopac7.spec
http://www.five-ten-sg.com/mopac7-1.15-7.fc12.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=1889314

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #9 from Jussi Lehtola   2009-12-24 03:51:20 EDT ---
Doesn't

%configure
sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool

do the trick?

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #10 from Carl Byington   2009-12-24 11:34:31 EDT ---
possibly, but that seems much more fragile than using chrpath.

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #11 from Jussi Lehtola   2009-12-24 19:24:30 EDT ---
Anyway, it's the recommended way. Chrpath is a last resort and should be
treated as such.

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #12 from Carl Byington   2010-01-02 13:44:02 EDT ---
done.

http://www.five-ten-sg.com/mopac7.spec
http://www.five-ten-sg.com/mopac7-1.15-8.fc12.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=1898980

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Jussi Lehtola  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #13 from Jussi Lehtola   2010-01-03 03:04:41 EDT ---
All issues have been fixed, the package has been

APPROVED

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Carl Byington  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

--- Comment #14 from Carl Byington   2010-01-04 12:46:33 EDT ---
New Package CVS Request
=======================
Package Name: mopac7
Short Description: Semi-empirical quantum mechanics suite
Owners: carllibpst
Branches: F-11 F-12
InitialCC:

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #15 from Kevin Fenzi   2010-01-04 15:10:28 EDT ---
cvs done.

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |ON_QA

--- Comment #16 from Fedora Update System   2010-01-05 17:46:18 EDT ---
mopac7-1.15-8.fc11 has been pushed to the Fedora 11 testing repository.  If
problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update mopac7'.  You can provide
feedback for this update here:
http://admin.fedoraproject.org/updates/F11/FEDORA-2010-0173

[Bug 542760] Review Request: mopac7 - Semi-empirical quantum mechanics suite

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=542760

--- Comment #17 from Fedora Update System   2010-01-05 17:55:38 EDT ---
mopac7-1.15-8.fc12 has been pushed to the Fedora 12 testing repository.  If
problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update mopac7'.  You can provide
feedback for this update here:
http://admin.fedoraproject.org/updates/F12/FEDORA-2010-0200