https://bugzilla.redhat.com/show_bug.cgi?id=1064656 --- Comment #3 from marcindulak <Marcin.Dulak@xxxxxxxxx> --- (In reply to Will Benton from comment #1) > Thanks for packaging this, Marcin. I've successfully built locally and am > waiting for a koji build to complete now. Here are some initial notes on > the package, though: thanks > > * The most serious problem is bundling. The source directory contains > (apparently) bundled BLAS and LAPACK implementations, a bundled libxc file, > and (as far as I can tell) Jack Dongarra's modified version of a public > domain implementation of the error function from the NSWC Library of > Mathematical Subroutines. This kind of bundling seems to be cultural in the > Fortran world, but you will need to remove these in %prep and build against > versions available in Fedora. in fact there was only one clearly "bundled" library (apart from erf) used by elk: fftw. BLAS and LAPACK were compiled but not used - i removed the source dirs in %prep and modified the makefile to show this explicitly. libxc was not bundled (in a sense being included in the elk source an compiled and linked as such), but elk included some fortran modules from libxc for interface purposes. I have switched to libxc-devel provided fortran modules, at the cost of having -I%{_fmoddir} in gfortran compilation options. I switched also to fftw3. As for erf.f90 - it looks like erf is used only in src/stheta_mp.f90 so i just removed the "external erf" from there + rm src/erf.f90. > > * The %if here appears vestigial; is that the case? > > %if 0%{?fedora} >= 21 > %global BLAS -L%{_libdir} -lopenblas > %global LAPACK -L%{_libdir} -llapack > %else > %global BLAS -L%{_libdir} -lopenblas > %global LAPACK -L%{_libdir} -llapack > %endif > this part is cleaned, and superfluous -llapack removed > * The %build section is awfully hairy, but it's not obvious that you could > make it a lot simpler without involving upstream. It looks like you've been > using this spec for some time; do you have a sense for how robust your > macros are here? i think elk build instructions have not changed since the introduction of libxc interface to elk (~3 years ago), but the module-load-dependent parts relevant for Fedora are new - in the past i was building just one openmpi version. The length of those build/install/check parts is a consequence of requirements (http://fedoraproject.org/wiki/Packaging:MPI#Packaging_of_MPI_software) that one should try to build all supported mpi (i don't build with mvapich on el6 anyway - as this would introduce even more logic into the spec). > > * In %check, I think it would make more sense to use an environment variable > for NPROC (I had to get a clarification on when this macro body would be > evaluated): > > %global NPROC %(cat /proc/cpuinfo | grep processor | wc -l) i do now: export NPROC=2 > > Please let me know once you have a chance to address these and I'll continue > with the review. Spec URL: http://marcindulak.fedorapeople.org/packages/elk/r03/elk.spec SRPM URL: http://marcindulak.fedorapeople.org/packages/elk/r03/elk-2.2.10-3.fc21.src.rpm koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=6543991 Additional comment: it looks like most test failures are due to OMP_NUM_THREADS!=1, but i leave the tests as is for now - elk is supposed to handle openmp. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review