https://bugzilla.redhat.com/show_bug.cgi?id=1662526 --- Comment #7 from Dominik 'Rathann' Mierzejewski <dominik@xxxxxxxxxxxxxx> --- (In reply to Ankur Sinha (FranciscoD) from comment #6) > (In reply to Dominik 'Rathann' Mierzejewski from comment #5) [...] > > 1. I read your comment at the top of the spec: > > > # This is a serial build of NEURON without Python or other bindings. > > > # Both the MPI builds and Python bindings require NEURON to be already > > > # installed in the system---they are build as post-installation hooks. So, we > > > # first package a serial version of NEURON and then package those separately > > > # after using this package as a BR > > > > Are you sure this is really the case? Looking at src/nrnmpi/Makefile.am, > > it seems that specifying the path to the just-built libraries should suffice > > to build it after the serial version is built. > > I did try that, but I wasn't able to get it to work. nrnmpi depends on other > bits that are not in subdirs of the nrnmpi directory. So, the dep chain > isn't set up correctly. From what I could find, there's a way to force the > dependent targets that are not in subdirs. (The other alternative is to drop > the subdir method of using Autotools and re-do the project.) So, it seems > that this why upstream build nrnmpi in the post-install hook (and not > post-build). Alright. Could you open a ticket with upstream about this, too? It would be better if MPI variants could be built in the same build process. [...] > > 3. The header file in -devel subpackage: > > # should this be here?! > > %{_libdir}/nrnconf.h > > > > It shouldn't. Please put it in %{_includedir} . > > Moved. You seem to have left the comment which is no longer relevant. > > 4. It looks like there's more bundled code: > > src/gnu - libstdc++-devel (?) > > src/readline - readline-devel > > Unfortunately it seems to be a version from 1988(!), and libstdc++ has > changed so much since then that I cannot even find the bundled headers. I've > filed a ticket upstream. For the time being, though, I've included it. > Upstream changed the soname, so it won't conflict with the upstream version > we provide in Fedora. OK, thanks for opening the upstream ticket. [...] > > 6. README.md isn't very useful for the installed package as it talks only > > about installing from source. > > Removed. Good, but the changelog entry doesn't explain why you did it, which might be useful. > Updated spec/srpm: > https://ankursinha.fedorapeople.org/neuron/neuron.spec # Do we need the static libraries? %files static %license Copyright %doc README.md %{_libdir}/libivoc.la %{_libdir}/libmemacs.la %{_libdir}/libmeschach.la %{_libdir}/libneuron_gnu.la %{_libdir}/libnrniv.la %{_libdir}/libnrnmpi.la %{_libdir}/libnrnoc.la %{_libdir}/liboc.la %{_libdir}/libocxt.la %{_libdir}/libsparse13.la %{_libdir}/libscopmath.la %{_libdir}/libivos.la If it's just .la (libtool archive), then these are not enough, you should have .a files as well. So either ship those, too or don't ship .la files at all unless they're dlopened by neuron consumers. -- 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 To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
