https://bugzilla.redhat.com/show_bug.cgi?id=1167175 Susi Lehtola <susi.lehtola@xxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@xxxxxxxxxxxxxxxxx |susi.lehtola@xxxxxx Flags| |fedora-review? --- Comment #3 from Susi Lehtola <susi.lehtola@xxxxxx> --- Next time, please post separate links to the spec and the srpm, so that the reviewer doesn't have to download the potentially huge srpm and decompress it to have a look at the spec file. Please clean up the spec file from commented out junk. E.g. #BuildRequires: atlas-devel atlas texlive-braket cmake hdf5-devel coreutils gsl gsl-devel doxygen python python-devel numpy Cython BuildRequires: atlas-devel atlas cmake hdf5-devel python python-devel numpy Cython and #python build Also, the buildrequires should be split up one per line. ** Please note that it is not necessary to BR the -devel and the base package. https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package ** Please note that explicit requires as in Requires: atlas are forbidden https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires ** Please don't replicate hardcoded values in the spec. Source0: https://github.com/SebWouters/CheMPS2/archive/v1.7.zip should be written as Source0: https://github.com/SebWouters/CheMPS2/archive/v%{version}.zip Also, you should use .tar.gz instead of .zip ** export CMAKE_INCLUDE_PATH=%{_includedir}/atlas-x86_64-base is incorrect on most architectures, and should be replaced by export CMAKE_INCLUDE_PATH=%{_includedir}/atlas ** You're mixing macro styles, ${RPM_BUILD_ROOT} vs %{buildroot}. Please pick one and stick with it. [Personally, I prefer the latter.] https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS ** gzip %{_builddir}/%{name}-%{version}/chemps2.1 mkdir -p %{buildroot}%{_mandir}/man1/ cp -p %{_builddir}/%{name}-%{version}/chemps2.1.gz %{buildroot}%{_mandir}/man1/ You don't need to compress the man pages, rpmbuild will do it for you. Also, referring to %{_builddir} is somewhat bad style. You could rewrite the %install section as make -C build install DESTDIR=$RPM_BUILD_ROOT install -D -p -m 644 chemps2.1 %{buildroot}%{_mandir}/man1/chemps2.1 cd PyCheMPS2 %{__python2} setup.py install -O1 --skip-build --root %{buildroot} find %{buildroot} -name '*.la' -exec rm -f {} ';' find %{buildroot} -name '*.a' -exec rm -f {} ';' Also, instead of deleting the static libraries after install you might consider fixing the cmake files to not build them at all, after which you could send the patch upstream. I'd recommend adding a ENABLE_SHARED flag toggling build of a shared library, and linking everything against it if it's enabled. Correspondingly, a ENABLE_STATIC flag would just toggle if a static library is built or not. ** for i in test*[0-9].py; do LD_LIBRARY_PATH=../../build/CheMPS2 PYTHONPATH=`find .. -name lib.*` %{__python2} $i || { echo 'tests failed' ; exit 1; }; done You're allowed to use indentation in spec files. This would be much more readable as for i in test*[0-9].py; do export LD_LIBRARY_PATH=../../build/CheMPS2 export PYTHONPATH=`find .. -name lib.*` %{__python2} $i || { echo 'tests failed' ; exit 1; } done Also, the tests take too much time. Please reduce the set of tests, or otherwise building the package will take ages on the ARM architecture. ** Please don't use wildcards so blatantly in %files %{_libdir}/*.so.* %{_bindir}/chemps2 %{_mandir}/man1/*.gz %files devel %{_includedir}/* %{_libdir}/*.so %files python %{python2_sitearch}/* could be written as %files %{_libdir}/libchemps2.so.* %{_bindir}/chemps2 %{_mandir}/man1/chemps2.1.* %files devel %{_includedir}/chemps2/ %{_libdir}/libchemps2.so %files python %{python2_sitearch}/CheMPS2-python-%{version}-py*.egg-info %{python2_sitearch}/PyCheMPS2.so -- 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://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx