https://bugzilla.redhat.com/show_bug.cgi?id=1780906 --- Comment #3 from Ankur Sinha (FranciscoD) <sanjay.ankur@xxxxxxxxx> --- Thanks for the review Jerry. I've made the required fixes: (In reply to Jerry James from comment #2) > Package Review > ============== > > Legend: > [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated > > Issues: > ======= > - Static libraries in -static or -devel subpackage, providing -devel if > present. > Note: Package has .a files: arbor-devel, arbor-mpich-devel, arbor- > openmpi-devel. Does not provide -static: arbor-devel, arbor-mpich-devel, > arbor-openmpi-devel. > See: https://docs.fedoraproject.org/en-US/packaging- > guidelines/#packaging-static-libraries > > That is an automatically generated issue. I think it is complaining that > the > 3 devel packages provide arbor{,-mpich,-openmpi}-static(%{arch}), but not > arbor{,-mpich,-openmpi}-static (i.e., without the "(%{arch})" part). I thought the arch specific Provides would be better, but I've removed that bit to please rpmlint. > > - Thank you for filing an upstream bug about static vs. shared libraries. I > wonder if we shouldn't build shared libraries for Fedora anyway, even > though > upstream doesn't support them. The issue is that once this package is > introduced, other packages will begin to depend on it by linking the static > libraries into their executables or libraries. That will require doing > some > transitioning in the future to move to shared libraries. It might be > better > to pay the cost now instead of later. What are your thoughts? Well, this isn't a general purpose library so we won't have tools linking to it as they generally do. Only users that run simulations using Arbor will link against it, and it is extremely unlikely that such simulation code will ever be packaged in Fedora. Since this package is related to science and correctness is paramount, I'm wary of providing any features that upstream does not support. I'm hoping upstream will start supporting shared libraries, and if/when they do, I can make them available. What do you think? > > - The test suite failed, but %check did not. Please arrange to have %check > fail the build if the tests do not pass. I've done this, but now I've had to disable tests completely to get the build to pass. Until upstream fixes the issue we've reported, we can't enable them. > > - The spec file contains: > > BuildRequires: python-devel > > Please change that to python3-devel. See > > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ > #_dependencies Done. > > - Please consider building the sphinx documentation. That will require that > you unbundle python3-sphinx_rtd_theme, however. Done. > > - Not all directories created by these packages are owned. In particular, > these directories are not owned by the arbor packages, nor by the openmpi > or mpich packages: %{_libdir}/openmpi/lib/cmake, > %{_libdir}/mpich/lib/cmake, > %{_libdir}/openmpi/include, %{_libdir}/mpich/include, > %{python3_sitearch}/mpich, %{python3_sitearch}/openmpi. > > Those last two look like they SHOULD be owned by python3-mpi4py-mpich and > python3-mpi4py-openmpi, but they are NOT. That is probably a bug in those > 2 > packages. They were owned by python3-{mpich,openmpi} so I've added those as Requires where required. > > - Can you do something about rpmlint's summary-not-capitalized warnings? For > example, "Arbor built with mpich" or "Mpich build of arbor" or > "Multi-compartment neural network simulation library (mpich build)"? Fixed. > > - This is not an issue. I just have to note that I laughed out loud when I > saw > the spell checker suggest this: neuroscience -> pseudoscience. > mpich -> chimp is pretty funny, too. > XD I know! Looks like the "enchant" library that rpmlint uses for spell checks does not include these terms. Updated spec/srpm: Spec: https://ankursinha.fedorapeople.org/arbor/arbor.spec SRPM: https://ankursinha.fedorapeople.org/arbor/arbor-0.2.2-2.fc32.src.rpm Changelog: * Mon Dec 09 2019 Ankur Sinha <ankursinha AT fedoraproject DOT org> - 0.2.2-2 - Remove arch info in provides for static - Temporarily disable tests - use python3-devel - Add documentation in separate sub-package - add python3-{mpich,openmpi} as requires that own MPI_PYTHON3_SITEARCH directories - Improve summaries for sub-packages to please rpmlint -- 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
