Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Bug ID: 915337 Summary: Review Request: nmon - Nigel's performance MONitor for Linux Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: unspecified Reporter: ravnzon@gmail.com
Spec URL: https://docs.google.com/file/d/0Bws3TZd4V12pejI2QXdxa2NTUzA/edit?usp=sharing SRPM URL: https://docs.google.com/file/d/0Bws3TZd4V12pY2RJcDRiaFV6WkE/edit?usp=sharing
Description: This systems administrator, tuner, benchmark tool gives you a huge amount of performance information about CPU, memory, network, disks (mini graphs or numbers), file systems, NFS, top processes, resources (Linux version & processors), in one go. The information can be displayed either on screen or saved to a comma separated (csv) file.
rpmlint: nmon.src: W: file-size-mismatch Documentation.txt = 255, http://downloads.sf.net/project/nmon/Documentation.txt = 262 3 packages and 1 specfiles checked; 0 errors, 1 warnings.
Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5053091
Fedora Account System Username: paller
This is my first package and I am seeking a sponsor.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Palle Ravn ravnzon@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Clone Of| |524119 CC| |ravnzon@gmail.com
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Terje Røsten terjeros@phys.ntnu.no changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR) CC| |dylan.swift@gmail.com
--- Comment #1 from Terje Røsten terjeros@phys.ntnu.no --- *** Bug 524119 has been marked as a duplicate of this bug. ***
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Palle Ravn ravnzon@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Alias| |nmon, -, performance, | |monitor
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Palle Ravn ravnzon@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Alias|-, monitor, performance |
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #2 from Palle Ravn ravnzon@gmail.com --- Files moved to better download location.
Spec URL: http://nmon.zom.dk/nmon.spec SRPM URL: http://nmon.zom.dk/nmon-14g-1.fc18.src.rpm
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |volker27@gmx.at
--- Comment #3 from Volker Fröhlich volker27@gmx.at --- -O2 and -Wall are already set in optflags and can therefore be removed. To do the PPC guys a favor, you could include a conditional clause to add "-D POWER", if built on PPC.
Make it ...%{name}.1* to allow for changes possible changes in compression.
I think you should use the name macro on the gcc and install invocation.
The timestamp of Source1 should be preserved.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #4 from Palle Ravn ravnzon@gmail.com --- Package updated
Spec URL: http://nmon.zom.dk/nmon.spec SRPM URL: http://nmon.zom.dk/nmon-14g-2.fc18.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5069343
rpmlint: 3 packages and 1 specfiles checked; 0 errors, 0 warnings.
(In reply to comment #3)
-O2 and -Wall are already set in optflags and can therefore be removed. To do the PPC guys a favor, you could include a conditional clause to add "-D POWER", if built on PPC.
-O2 and -Wall removed. I have added the architecture conditional %ifarch for ppc and ppc64. I am using the %{power64} macro, but not sure if that is more correct than using ppc64? I have tested the build with ppc-koji and the "-D POWER" parameter is invoked for both ppc and ppc64, see http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=962445.
Make it ...%{name}.1* to allow for changes possible changes in compression.
Done.
I think you should use the name macro on the gcc and install invocation.
That seems reasonable, done.
The timestamp of Source1 should be preserved.
I didn't understand this at first, as the downloaded file had preserved the timestamp from the server. I assume that the timestamp should not change duo to the linebreak fix using sed? That is now corrected with touch, so Source1 is now dated correctly in the /usr/share/doc/... folder.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Hans de Goede hdegoede@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |hdegoede@redhat.com Assignee|nobody@fedoraproject.org |hdegoede@redhat.com
--- Comment #5 from Hans de Goede hdegoede@redhat.com --- Hi Palle, Volker,
Volker, thanks for your initial review of this packages.
Palle I think that it is great that you're working on the Fedora Design Packages WishList, as such I would like to Sponsor you.
The first step is me reviewing this and your other package submission, then you do a new revision (assuming I'll have some remarks on my first reviews), and I will review the new revision. Rince-repeat until both packages you've submitted are approved.
And then once I'm happy with both packages I'll sponsor you and you can continue with the next steps of: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers
I'll do a full review of the version you posted in comment 4 later tonight, and let you know what, if anything, needs to be fixed.
Regards,
Hans
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #6 from Hans de Goede hdegoede@redhat.com --- Hi,
Full review done:
Good: - rpmlint checks return: 3 packages and 1 specfiles checked; 0 errors, 0 warnings. - package meets naming guidelines (but not versioning, see below) - package meets packaging guidelines - license (GPLv3) OK, but text not in %doc, matches source - spec file legible, in am. english - source matches upstream - package compiles on devel (x86) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file
Needs work: -The Source0 and Source1 urls are wrong, we've a preferred down url form for sf.net, see: http://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net -The generated manpage is rubish, it contains the same bit twice, and then a reference to the non-existent texinfo documentation. Luckily Debian has already packaged nmon and provides a manpage for us :) See: ftp://ftp.nluug.nl/pub/os/Linux/distr/debian/pool/main/n/nmon/nmon_13g+debian-1.debian.tar.gz -Drop the now no longer needed "BuildRequires: help2man" -The lmon14g.c file does not contain a clear copyright-header, and upstream does not provide a copy of the GPLv3 (typically upstream provides a COPYING.txt), this is not a blocker, but please mail upstream asking them to do a new release with a proper copyright-header, and while they are at it they should add a COPYING.txt with the GPLv3 and put all the files together in a tarbal. As said this is not a blocker for getting this package into Fedora, but you *MUST* mail upstream asking them to do better for their next release. While mailing upstream, please attach the Debian manpage and ask them to add that to the tarbal too. -manpages should not be marked %doc
Regards,
Hans
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Hans de Goede hdegoede@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #7 from Palle Ravn ravnzon@gmail.com --- (In reply to comment #6)
Full review done:
Good:
- ...
- package meets naming guidelines (but not versioning, see below)
I'm looking below and I don't see more about the versioning, can you clarify this for me?
Needs work: -The Source0 and Source1 urls are wrong, we've a preferred down url form for sf.net, see:
Now following the guidelines.
http://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net -The generated manpage is rubish, it contains the same bit twice, and then a reference to the non-existent texinfo documentation. Luckily Debian has already packaged nmon and provides a manpage for us :) See: ftp://ftp.nluug.nl/pub/os/Linux/distr/debian/pool/main/n/nmon/ nmon_13g+debian-1.debian.tar.gz
The Debian manpage has also been committed on sourceforge under patches, so I downloaded that one. However, I'm not able to follow the sourceforge guidelines for this link.
There's a second manpage, which covers more of the documentation, and I would like to use that one, but I has some unwanted references to a screenshot list in its intro. I'm considering rewriting it a bit and then use it, what are your thoughts on that?
link: http://sourceforge.net/tracker/?func=detail&aid=3295760&group_id=271...
-Drop the now no longer needed "BuildRequires: help2man"
Now using install -D -p -m 0655 for the nmon.1 manpage.
... you *MUST* mail upstream asking them to do better for their next release. While mailing upstream, please attach the Debian manpage and ask them to add that to the tarbal too.
I have mailed Nigel using sourceforges mailing system, as I'm unable to find any other contact information, and asked for inclusion of license file and header, manpage, and tarball releases. The projects mailing list has a total of 2 messages, from the same person as he never received an answer, I'm guessing nobody reads it.
I could make feature requests for the above, but I'm not a fan of that, as it has nothing to do with the functionality of the program.
-manpages should not be marked %doc
Removed %doc.
New files Spec URL: http://nmon.zom.dk/nmon.spec SRPM URL: http://nmon.zom.dk/nmon-14g-3.fc18.src.rpm
rpmlint: nmon.x86_64: W: unstripped-binary-or-object /usr/bin/nmon 2 packages and 1 specfiles checked; 0 errors, 1 warnings.
koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5135115 PowerPC http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=998047
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #8 from Hans de Goede hdegoede@redhat.com --- Hi,
(In reply to comment #7)
(In reply to comment #6)
Full review done:
Good:
- ...
- package meets naming guidelines (but not versioning, see below)
I'm looking below and I don't see more about the versioning, can you clarify this for me?
My bad, usually we only allow numeric chars in Version:, so I was planning to point this out and point you to: http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Non-Numeric_Version...
But while reading that myself I noticed that there is one exception to the only numeric chars in the Version field, and this package matches that exception, so its use I fine,
Then I removed the comment on this, but not the referral to the comment you just quoted.
-The generated manpage is rubish, it contains the same bit twice, and then a reference to the non-existent texinfo documentation. Luckily Debian has already packaged nmon and provides a manpage for us :) See: ftp://ftp.nluug.nl/pub/os/Linux/distr/debian/pool/main/n/nmon/ nmon_13g+debian-1.debian.tar.gz
The Debian manpage has also been committed on sourceforge under patches, so I downloaded that one. However, I'm not able to follow the sourceforge guidelines for this link.
There's a second manpage, which covers more of the documentation, and I would like to use that one, but I has some unwanted references to a screenshot list in its intro. I'm considering rewriting it a bit and then use it, what are your thoughts on that?
link: http://sourceforge.net/tracker/ ?func=detail&aid=3295760&group_id=271307&atid=1153693
That is fine. But please change the LICENSE section from "under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3 of the License, or (at your option) any later version."
To:
"under the terms of the GNU General Public License as published by the Free Software Foundation; version 3"
IOW drop the "either" and the ", or (at your option) any later version.", as the only license statement we've is the one from Documentation.txt which does not contain the or later version language.
You should then also submit the updated manpage upstream (not sure how useful that is in this case, but as a rule we always try to send all changes upstream in Fedora).
-Drop the now no longer needed "BuildRequires: help2man"
Now using install -D -p -m 0655 for the nmon.1 manpage.
That should be -m 0644 which I see you've gotten right in the specfile :)
... you *MUST* mail upstream asking them to do better for their next release. While mailing upstream, please attach the Debian manpage and ask them to add that to the tarbal too.
I have mailed Nigel using sourceforges mailing system, as I'm unable to find any other contact information, and asked for inclusion of license file and header, manpage, and tarball releases.
Great, thanks. I know this rule may seem a bit stupid with upstreams which appear dead. But it is a good rule in general, and in my experience with dead upstreams sometimes they surprise you by not being dead after all.
The projects mailing list has a total of 2 messages, from the same person as he never received an answer, I'm guessing nobody reads it.
I could make feature requests for the above, but I'm not a fan of that, as it has nothing to do with the functionality of the program.
No need to file an RFE, if a direct mail won't do the trick usually nothing will.
New files Spec URL: http://nmon.zom.dk/nmon.spec SRPM URL: http://nmon.zom.dk/nmon-14g-3.fc18.src.rpm
Looks good: approved!
So as promised I've added you to the packager group and sponsored you, so you can now move forward with: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_P...
2 minor nitpicks to further improve the package:
1) In the ChangeLog you write: - Remove manpage from doc This may lead to users thinking the manpage was removed. If I were you I would've written this instead: - No longer mark manpage as %%doc
Note the %%, whenever you use % in the changelog (or in comments) you need to write %%
2) In %prep you do: cp %{SOURCE2} . and then in %install: install -D -p -m 0644 %{name}.1 %{buildroot}%{_mandir}/man1/%{name}.1
Instead you can just do the following in %install, without anything in %prep: install -D -p -m 0644 %{SOURCE2} %{buildroot}%{_mandir}/man1/%{name}.1
Regards,
Hans
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Hans de Goede hdegoede@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | Flags| |fedora-review+
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #9 from Palle Ravn ravnzon@gmail.com --- New Package SCM Request ======================= Package Name: nmon Short Description: Nigel's performance Monitor for Linux Owners: paller Branches: f17 f18 f19 InitialCC:
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Palle Ravn ravnzon@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) | Flags| |fedora-cvs?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #10 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #11 from Fedora Update System updates@fedoraproject.org --- nmon-14g-3.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/nmon-14g-3.fc18
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #12 from Fedora Update System updates@fedoraproject.org --- nmon-14g-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/nmon-14g-3.fc17
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- nmon-14g-3.fc18 has been pushed to the Fedora 18 testing repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915337
Palle Ravn ravnzon@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2013-04-01 04:03:07
https://bugzilla.redhat.com/show_bug.cgi?id=915337
Athmane Madjoudj athmanem@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |athmanem@gmail.com Flags|fedora-cvs+ |fedora-cvs?
--- Comment #14 from Athmane Madjoudj athmanem@gmail.com --- Package Change Request ====================== Package Name: nmon New Branches: el5 el6 epel7 Owners: athmane
https://bugzilla.redhat.com/show_bug.cgi?id=915337
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=915337
--- Comment #15 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=915337
Peter Oliver mavit@mavit.org.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mavit@mavit.org.uk Alias|nmon |
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla