[Bug 1662526] Review Request: neuron - A flexible and powerful simulator of neurons and networks

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

 



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




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux