[Bug 464424] Review Request: GROMACS - a Molecular Dynamics package

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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=464424





--- Comment #8 from Jussi Lehtola <jussi.lehtola@xxxxxx>  2008-09-30 10:01:31 EDT ---
(In reply to comment #7)
> Full review below. OK'd items omitted.
> 
> - MUST: rpmlint must be run on every package. The output should be posted in
> the review.
> NEEDSFIX

Fixed. Now rpmlint output is:

gromacs.x86_64: W: no-documentation
gromacs-bash.x86_64: W: no-documentation
gromacs-bash.x86_64: W: non-conffile-in-etc /etc/bash_completion.d/gromacs
gromacs-devel.x86_64: W: no-documentation
gromacs-libs.x86_64: W: no-documentation
gromacs-mpi.x86_64: W: no-documentation
gromacs-mpi-devel.x86_64: W: no-documentation
gromacs-mpi-libs.x86_64: W: no-documentation
gromacs-mpi-libs.x86_64: E: postun-without-ldconfig
/usr/lib64/libmd_mpi_d.so.4.0.0
gromacs-mpi-libs.x86_64: E: postun-without-ldconfig
/usr/lib64/libmd_mpi.so.4.0.0
gromacs-mpi-libs.x86_64: E: postun-without-ldconfig
/usr/lib64/libgmx_mpi.so.4.0.0
gromacs-mpi-libs.x86_64: E: postun-without-ldconfig
/usr/lib64/libgmx_mpi_d.so.4.0.0
gromacs-mpi-libs.x86_64: E: non-empty-%postun /sbin/ldconfig
gromacs-zsh.x86_64: W: no-documentation
11 packages and 1 specfiles checked; 5 errors, 9 warnings.

gromacs-mpi-libs *does* run ldconfig in post and postun. 
Is this a bug in rpmbuild / rpmlint??

> - MUST: The package must meet the Packaging Guidelines.
> NEEDSFIX
> 
> Please rename the specfile to have the same name as the main package, i.e.
> gromacs.spec.
> 
> Either make four devel subpackages, one for each set of libraries
> or group them together, for example
> single+double -> gromacs
> mpi single+double -> gromacs-mpi
> and then only two -devel packages
> You should also put libraries into a separate package to allow for multilib
> installs.

OK, done.


> autoreconf
> 
> Is it necessary? Calling autoreconf makes the build fail on rawhide/i386. If
> you remove
> that, remove BR: autoconf/automake, too.

Yes, since the configure script distributed with GROMACS is so old that it uses
rpath. Works fine on F9 and RHEL5.

> Maybe add --enable-pthreads? Or a separate pthread-enabled version? Not a
> blocker, just FYI.

GROMACS doesn't have thread support yet, since the performance is so bad on
NUMA systems, the speedup is about 75%. MPI results in far better results: the
speedups are >99%.

> completion.bash could be put in %{_sysconfdir}/bash_completion.d/ (needs
> Requires: bash-completion then, maybe a separate package
> bash-completion-gromacs?).
> Check for similar possibilities for other shells. This would help loosen the
> dependencies on various shells. As it is, -common depends on all of them:
> Requires: /bin/bash /bin/csh /bin/sh /bin/zsh /usr/bin/perl
> This is not reasonable. At least the csh and zsh dependencies must be removed.

Made packages gromacs-bash, -zsh and -csh.

> - MUST: The package must be licensed with a Fedora approved license and meet
> the  Licensing Guidelines .
> NEEDSFIX
> 
> Some files (especially src/gmxlib/gmx_{lapack,blas}/*) have no licensing
> information,
> although they look like private copies of lapack and blas, so please verify
> that.
> If so, their license is BSD, which is GPLv2+ compatible.

(clip)

> Please ask upstream to add appropriate license headers to these files.

Filed upstream bug, http://bugzilla.gromacs.org/show_bug.cgi?id=217 .
The files are C-versions of BLAS & LAPACK made with f2c.
RPMs are now built with distribution libraries instead of the ones distributed
with GROMACS.

> - MUST: Packages containing GUI applications must include a %{name}.desktop
> file, and that file must be properly installed with desktop-file-install in the
> %install section. This is described in detail in the desktop files section of
> the Packaging Guidelines . If you feel that your packaged GUI application does
> not need a .desktop file, you must put a comment in the spec file with your
> explanation.
> NEEDSFIX(?)
> I see libX11 in rpm-generated Requires:. Does this package display a GUI? If
> so, it needs an appropriate desktop file and desktop-file-install call (BR:
> desktop-file-utils).

The only program which requires X is ngmx, which has no desktop interface - it
must be run from the command line with file parameters.

> SHOULD Items:
> 
> - SHOULD: The reviewer should test that the package builds in mock. See
> MockTricks for details on how to do this.
> 
> Doesn't build in mock/devel/i386 due to autoreconf failure.
> Builds fine in koji/dist-f10 (with autoreconf invocation removed).
> http://koji.fedoraproject.org/koji/taskinfo?taskID=851228

If you run rpmlint on these you'll fine rpaths in the libraries.

> - SHOULD: The package should compile and build into binary rpms on all
> supported architectures.
> - SHOULD: The reviewer should test that the package functions as described. A
> package should not segfault instead of running, for example.
> 
> I haven't actually tested it, but I think you must use --disable-ia32-* for
> i386 builds, otherwise gromacs will crash with SIGILL when run on a CPU without
> 3dnow or sse instructions. You can also build separate versions of gromacs
> libraries with these enabled and put them in %{_libdir}/3dnow and
> %{_libdir}/sse, respectively. The linker will then select the best version
> automatically (see atlas package for inspiration).
> Similarly --disable-ppc-altivec for ppc builds (not all ppc have altivec), but
> you can build separate altivec-enabled libraries and put them in
> %{_libdir}/altivec/

Disabling assembly hurt the speed of GROMACS *a lot*. People would probably
still end up compiling locally GROMACS if the instructions were disabled.

Also, I think GROMACS handles this automatically: when I run it on an x86_64 it
reports

Configuring nonbonded kernels...
Testing x86_64 SSE support... present.

so I think there should be no problems.

SPEC:
http://theory.physics.helsinki.fi/~jzlehtol/rpms/gromacs.spec

SRPM:
http://theory.physics.helsinki.fi/~jzlehtol/rpms/gromacs-4.0-4.rc2.fc9.src.rpm

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

_______________________________________________
Fedora-package-review mailing list
Fedora-package-review@xxxxxxxxxx
http://www.redhat.com/mailman/listinfo/fedora-package-review

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]