Bug 1402164

I forgot to link to the original bug report where the current maintainer stated that he doesn't have time to upgrade the package -->

https://bugzilla.redhat.com/show_bug.cgi?id=1338050

Disclaimer: This is an unofficial review.

The SPEC file that you are using is fairly old.  The last update performed by the package maintainer was in 2011.  There have been a host of changes in the packaging guidelines since then.  I would recommend following the current guidelines to write the SPEC file.

* https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Guidelines

* https://fedoraproject.org/wiki/Packaging:Python

> I have made as little changes to the original rpms as possible.

Why that? What changes would you have liked to make?


> Summary:        Fully colorized df clone written in python

s/python/Python/


> %install
> rm -rf $RPM_BUILD_ROOT

https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections


> install -p pydf   $RPM_BUILD_ROOT%{_bindir}
> install -p pydfrc $RPM_BUILD_ROOT%{_sysconfdir}
> install -p pydf.1 $RPM_BUILD_ROOT%{_mandir}/man1

These commands are the obvious place where to use the -m argument to set the mode of the files.


> gzip -9nf $RPM_BUILD_ROOT%{_mandir}/man1/pydf.1 README

rpm-builds compresses manual pages on-the-fly, choosing whatever compression technique is configured on the build machine. Don't compress the file manually.

And compressing the 1992 bytes small README into a renamed file README.gz serves no purpose. It doesn't even save a KiB.


> %clean
> rm -rf $RPM_BUILD_ROOT

https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections


> %defattr(644,root,root,755)

https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions


> %doc README.gz INSTALL COPYING

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text


> %attr(755,root,root) %{_bindir}/pydf

If installed correctly during %install (either via an upstream Makefile or manually with the "install" command), it would not be necessary to fiddle with %attr here.  Sure, using %attr works, but imagine you need to maintain a larger package with many more files. Overusing %attr reduces readability a lot. You want to reduce the usage to really special permission/owner change scenarios.


> %{_mandir}/man1/pydf.1.gz

As above. The following would be able to handle uncompressed man pages as well as system-wide rpmbuild configuration changes:

  %{_mandir}/man1/pydf.1*

Thank you Dhanesh and Michael! I will read through your notes and links and see if I can come up with a better spec file.

what about:
srpm: https://github.com/gtema/pydf/raw/master/pydf-12-1.fc25.src.rpm
spec: https://github.com/gtema/pydf/raw/master/pydf.spec

spec is created brand new taking into account all found packaging recommendations.

Can anyone, please, review the spec?

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.