https://bugzilla.redhat.com/show_bug.cgi?id=1402164
Bug ID: 1402164 Summary: Review Request: pydf - Fully colorized df clone written in python Product: Fedora Version: rawhide Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: devin@six19.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://devhen.org/rpm/pydf/f25/pydf.spec SRPM URL: https://devhen.org/rpm/pydf/f25/pydf-12-1.fc25.src.rpm Description: pydf displays the amount of used and available space on your file systems, just like df, but in colors. The output format is completely customizable. Fedora Account System Username: devhen
This is my first package and I need a sponsor.
pydf is in Fedora (for <= f24) and in EPEL (for el5 & el6) but is very outdated. The latest is version 12 and Fedora/EPEL have version 9.
I submitted a bug report asking for the package to be upgraded to which the current maintainer replied that he doesn't have the time. So I would like to take over maintenance duties for the this package. I've created updated packages for f24, f25, f26, el6, and el7.
https://devhen.org/rpm/pydf/f24/pydf-12-1.fc24.src.rpm https://devhen.org/rpm/pydf/f25/pydf-12-1.fc25.src.rpm https://devhen.org/rpm/pydf/f26/pydf-12-1.fc26.src.rpm https://devhen.org/rpm/pydf/el6/pydf-12-1.el6.src.rpm https://devhen.org/rpm/pydf/el7/pydf-12-1.el7.src.rpm
I have made as little changes to the original rpms as possible. pydf version 12 supports python 3 so I've changed the spec to require python 3 instead of python 2. I've also added a sed command to %prep that changes the shebang in the pydf executable from /usr/bin/python to /usr/bin/python3. Other than that, and version number & changelog changes, everything else in the rpms & spec files has stayed the same aside from the new pydf version 12 source code (upstream: http://kassiopeia.juls.savba.sk/~garabik/software/pydf/pydf_12.tar.gz).
I have tested these on f24, f25, el6, and el7 and they work well. I have also run koji scratch builds and they have all finished successfully.
Successful f24 koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=16776736
Successful f25 koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=16776559
Successful f26 koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=16776763
Successful el6 koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=16776894
Successful el7 koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=16776834
Please let me know if there is someone who would be willing to sponsor me and help me with my first package submission. I hope to be able to contribute this package as well as more packages in the future!
Thanks,
FAS User: devhen Devin Henderson devin@six19.com
https://bugzilla.redhat.com/show_bug.cgi?id=1402164
--- Comment #1 from Devin Henderson devin@six19.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1402164
Raphael Groner projects.rg@smart.ms changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1402164
Dhanesh B. Sabane dhanesh95@disroot.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dhanesh95@disroot.org
--- Comment #2 from Dhanesh B. Sabane dhanesh95@disroot.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1402164
--- Comment #3 from Michael Schwendt bugs.michael@gmx.net ---
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*
https://bugzilla.redhat.com/show_bug.cgi?id=1402164
--- Comment #4 from Devin Henderson devin@six19.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1402164
Artem artem.goncharov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |artem.goncharov@gmail.com
--- Comment #5 from Artem artem.goncharov@gmail.com --- 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?
package-review@lists.fedoraproject.org