Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=913152 --- Comment #15 from Paulo Andrade <paulo.cesar.pereira.de.andrade@xxxxxxxxx> --- You should bump the release for every review request update, that is, the last one should be Release 4. Things that still should be done include: 1. You added sed -e 's|@@CFLAGS@@|%{optflags}|g' -i Makefile.inc to the spec as I suggested, but it is pointless, because SOURCE1 (Makefile.seq.inc) and SOURCE2 (Makefile.par.inc) still hardcode CFLAGS, that is, you should change those to the pseudo patch: -OPTC = -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 +OPTC = -@@CFLAGS@@ 2. Following above, the OPTF variable should also use %{optflags}, and not the hardcoded "-O -Dintel_ -DALLOW_NON_INIT" that is, should replace the -O with %{optflags}. 3. The variable OPTL appears to be the best place to use -Wl,-as-needed, that is, the pseudo patch: -OPTL = -O -OPTL = -Wl,-as-needed About the sequential vs parallel versions, it may not be a runtime choice, so, should also build a version of the examples using it the "seq" libraries, and test them. Should be something like: cp -ar examples examples.seq sed -e 's|libdir = $(topdir)/lib|libdir = $(topdir)/libmums_seq' -i examples.seq/Makefile This would also need some extra patching to add a examples.seq target to the toplevel Makefile. Oterhwise, you cannot know if the sequential packages and libraries are functional. I can see the "seq" libraries as an option when there is no functional mpi, otherwise, I think it would be easier to just build the parallel version. Either way, I think it is doing a bogus build in the libmumps_seq directory, because, if I understand it correctly, libmpiseq-4.10.0.so should be a wrapper to not need to link to the mpi libraries, but every other library in that directory is linked to the mpi libraries. And these chunks are duplicated in the spec for the "seq" and "par" builds: MUMPS_MPI=openmpi MUMPS_INCDIR=-I/usr/include/openmpi-%{_arch} MUMPS_LIBF77="\ -L%{_libdir}/openmpi -L%{_libdir}/openmpi/lib \ -lmpi_f77 -lmpi -lopen-rte \ -lopen-pal -lscalapack -lmpiblacs \ -lmpiblacsF77init -lmpiblacsCinit -llapack" make MUMPS_MPI="$MUMPS_MPI" \ MUMPS_INCDIR="$MUMPS_INCDIR" \ MUMPS_LIBF77="$MUMPS_LIBF77" \ all Maybe the libraries should not be explicitly linked to the mpi libraries? And only actual binaries linked to; hmm, sanity tells it should be other way around to allow runtime choice, so, the MUMPS-seq package should include the libraries in the libmumps_seq directory (and those cannot be linked to mpi, but to libmpiseq-4.10.0.so), and then, have the 2 packages conflict with each other, or use alternatives or environment-modules to allow both to be installed at the same time. Long history short, for the sake of review, I would be satisfied if you do the simpler approach of dropping the "seq" build, and only do the "par" build. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=thaVhR2hIP&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review