https://bugzilla.redhat.com/show_bug.cgi?id=1427341 --- Comment #15 from VincentS <vincent@xxxxxxxxxxxxxxxxxx> --- (In reply to Charalampos Stratakis from comment #11) > As this package is quite complex I will post my findings so far and after we > get these out of the way, we can proceed with the rest. > > All the BuildRequires are fine. > > You should remove the comment after %install about the python2 and python3 > install, since the package is python2 and only uses the %py2_install macro All right. > The desktop file installation should happen after the %py2_install. Done > Also what is the reason for this line? > chmod 755 %{buildroot}/%{python2_sitearch}/%{srcname}/gendoc.py > Same for: %doc %attr(644,-,-) README > And: %defattr(644,root,root,755) As guidelines say, Permissions on files must be set properly. I saw with rmplint : - python-gamera.x86_64: E: wrong-script-interpreter /usr/lib64/python2.7/site-packages/gamera/gendoc.py /usr/bin/env python - python-gamera.x86_64: E: non-executable-script /usr/lib64/python2.7/site-packages/gamera/gendoc.py 644 /usr/bin/env python - python-gamera.x86_64: W: spurious-executable-perm /usr/share/doc/python-gamera/README I thought permissions hadn't set properly, so I added chmod and %defattr to solve these problems. What do you think about ? > The devel subpackage should have a Requires tag for the main package. > See: > https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package Yes I forgot. > This comment is not required: # For noarch packages: sitelib Done > Also while it is good to keep some level of abstraction when specifying the > directories and binaries at the files section, I would recommend to be more > specific about some things, since it helps while working with the package. > e.g. > > Remove: '%{python2_sitearch}/%{srcname}*' > Add '%{python2_sitearch}/%{srcname}' and > '%{python2_sitearch}/%{srcname}-%{version}-py?.?.egg-info' Ok I understand. > Also the %{_includedir}/python2.7/%{srcname}/ should be changed to > %{_includedir}/python%{python2_version}/%{srcname}/ All right. > Now as for the binaries. The package provides two of them for which it would > be good practice to list them individually at the respective files sections, > so basically instead of %{_bindir}/*, use %{_bindir}/gamera_gui and > %{_bindir}/gamera_post_install.py Great, for few binaries I need to be precise. > However what is the purpose of gamera_post_install.py binary? I kinda think > that it might be required to be included at the devel subpackage. In fact, this file is only needed for Windows installation. So I removed it. > Also the package ships a lot of .so libraries so you should read the > respective section at the packaging guidelines: > https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries Thanks Miro for your reply and Charalampos for your comments. -- 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