Bug 635256 - Review Request: qtop - tool for monitoring PBS systems
Review Request: qtop - tool for monitoring PBS systems
Status: ASSIGNED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Haïkel Guémar
Fedora Extras Quality Assurance
http://cern.ch/fotis/QTOP
: Reopened
: 635257 (view as bug list)
Depends On:
Blocks: FE-DEADREVIEW
  Show dependency treegraph
 
Reported: 2010-09-18 08:37 EDT by Fotis Georgatos
Modified: 2015-07-21 11:01 EDT (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Requires torque package (ie. pbsnodes & qstat), at run-time.
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-07-21 09:05:16 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
karlthered: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Fotis Georgatos 2010-09-18 08:37:37 EDT
Spec URL: http://cern.ch/fotis/qtop.spec
SRPM URL: http://cern.ch/fotis/qtop-v40-1.src.rpm
Description: qtop is the ultimate ascii tool for monitoring PBS systems, esp. torque.
Comment 1 Martin Gieseking 2010-09-19 07:39:16 EDT
Hi Fotis,

as I can't find you in FAS, this is probably your first package submission. If so, please add FE-NEEDSPONSOR to the Blocks field above, and see http://fedoraproject.org/wiki/PackageMaintainers/Join for further details.

Here are some initial notes on your package:
- the version number should not contain any letters
- add %{?dist} to the Release number
- shorten the Summary to "Monitoring tool for PBS-based cluster systems"
  (the package name and leading articles should be avoided in the summary)
- the license seems to be GPLv2 (update the License field accordingly)
  http://fedoraproject.org/wiki/Licensing#SoftwareLicenses
- the SF project URL is invalid
- the %description lines should not exceed 80 characters
- add an empty %build section with a comment "nothing to build"
- drop the commented/disabled shell commands
- use macros to refernce system directories in %install and %files 
  /etc -> %{_sysconfdir}
- Unlike stated in the header comment of qtop, the source tarball doesn't 
  contain the GPLv2 license text. Since you're the upstream developer, you 
  should add it. :)
- the %changelog headers should contain the email address of the packager
  (see http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs)


$ rpmlint /var/lib/mock/fedora-13-x86_64/result/*.rpm
qtop.noarch: W: summary-not-capitalized C qtop, a monitoring tool for PBS-based cluster systems
qtop.noarch: W: name-repeated-in-summary C qtop
qtop.noarch: W: spelling-error %description -l en_US ascii -> ASCII, asci, asci i
qtop.noarch: W: spelling-error %description -l en_US ansi -> ANSI, anis, ans
qtop.noarch: W: spelling-error %description -l en_US devided -> decided, devised, divided
qtop.noarch: W: spelling-error %description -l en_US qstat -> stat, q stat, States
qtop.noarch: W: spelling-error %description -l en_US eg -> Eg, eh, e
qtop.noarch: E: description-line-too-long C It tries to fit as much as possible information in a single screen's real estate.
qtop.noarch: E: description-line-too-long C The screen is devided in three sections, reporting a) Summary b) Cores matrix c) Users
qtop.noarch: E: description-line-too-long C Each user gets mapped to a unique letter, according to their number of jobs in qstat.
qtop.noarch: E: description-line-too-long C Character 0 is always the user with the most R+Q+other jobs, 1 is next in # of jobs etc.
qtop.noarch: E: description-line-too-long C qtop should suppress color mode automatically, when needed, eg. try: watch -d qtop
qtop.noarch: E: description-line-too-long C It is very configurable, read first part of source code for how to make your qtop.conf
qtop.noarch: W: incoherent-version-in-changelog v40 ['v40-1', 'v40-1']
qtop.noarch: W: invalid-license GPL
qtop.noarch: W: invalid-url URL: http://qtop.sf.net HTTP Error 404: Not Found
qtop.noarch: W: no-documentation
qtop.noarch: W: non-conffile-in-etc /etc/qtop.colormap
qtop.noarch: W: no-manual-page-for-binary qtop
qtop.src: W: summary-not-capitalized C qtop, a monitoring tool for PBS-based cluster systems
qtop.src: W: name-repeated-in-summary C qtop
qtop.src: W: spelling-error %description -l en_US ascii -> ASCII, asci, asci i
qtop.src: W: spelling-error %description -l en_US ansi -> ANSI, anis, ans
qtop.src: W: spelling-error %description -l en_US devided -> decided, devised, divided
qtop.src: W: spelling-error %description -l en_US qstat -> stat, q stat, States
qtop.src: W: spelling-error %description -l en_US eg -> Eg, eh, e
qtop.src: E: description-line-too-long C It tries to fit as much as possible information in a single screen's real estate.
qtop.src: E: description-line-too-long C The screen is devided in three sections, reporting a) Summary b) Cores matrix c) Users
qtop.src: E: description-line-too-long C Each user gets mapped to a unique letter, according to their number of jobs in qstat.
qtop.src: E: description-line-too-long C Character 0 is always the user with the most R+Q+other jobs, 1 is next in # of jobs etc.
qtop.src: E: description-line-too-long C qtop should suppress color mode automatically, when needed, eg. try: watch -d qtop
qtop.src: E: description-line-too-long C It is very configurable, read first part of source code for how to make your qtop.conf
qtop.src: W: invalid-license GPL
qtop.src: W: invalid-url URL: http://qtop.sf.net HTTP Error 404: Not Found
qtop.src:11: W: macro-in-comment %{_tmppath}
qtop.src:11: W: macro-in-comment %{name}
qtop.src:11: W: macro-in-comment %{version}
qtop.src:11: W: macro-in-comment %{release}
qtop.src:11: W: macro-in-comment %{__id_u}
qtop.src:32: W: macro-in-comment %_sourcedir
qtop.src:32: W: macro-in-comment %_sourcedir
qtop.src:33: W: macro-in-comment %{_sourcedir}
qtop.src:46: W: macro-in-comment %{buildroot}
qtop.src: W: no-%build-section
2 packages and 0 specfiles checked; 12 errors, 32 warnings.
Comment 2 Fotis Georgatos 2010-09-19 10:45:14 EDT
Hi,

thanks for all the comments! (as opposed to thanks for all the fish :-).

I just managed to recreate the RPMs, please retry:

Spec URL: http://cern.ch/fotis/qtop.spec
SRPM URL: http://cern.ch/fotis/qtop-40-2948.src.rpm
Description: Monitoring tool for PBS-based cluster systems, incl. torque.

fyi. what I did wrong earlier was that I applied rpmlint only to the .spec
This looks normal:

[fotis@ui10 ~]$ rpmlint /home/fotis/rpmbuild/RPMS/noarch/qtop-40-2948.noarch.rpm /home/fotis/rpmbuild/SRPMS/qtop-40-2948.src.rpm
qtop.noarch: W: no-documentation
2 packages and 0 specfiles checked; 0 errors, 1 warnings.
[fotis@ui10 ~]$ 

Going in standby for further feedback...

tia,
Fotis
Comment 3 Martin Gieseking 2010-09-19 11:45:05 EDT
OK, triggering wakeup again. :)

Here are some more comments:

- The Release increment is a bit too large. :) This field reflects the number 
  of spec file revisions for a given version. So, just increase it by 1 every 
  time you provide a new spec/srpm with identical version number. Also, add a
  %changelog entry for every revision. The release number of your next revision
  should be 3, and the %changlog should contain 3 entries listing the changes 
  made for each release.
  If 2948 indicates your upstream revision or something similar, you might 
  want to add it to the version number, e,g. 40.2948 or the like.

- Drop the double BuildRoot tag commented out.

- Add file LICENSE to the %files section (with %doc)
  https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text

- Add blank lines between the various sections (%prep, %install, etc.) to 
  increase legibility

- Remove the "echo" lines to avoid redundant output during the build. The 
  %build section should contain a comment (starting with #) rather than an 
  echo statement. 

- Drop "version" from the %changelog header
  http://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs

- If possible, please add a link to the tarball to the project website.
  Otherwise, it's a bit difficult to verify that the packaged archive is the
  original tarball belonging to the project. Maybe you could also add the
  version number to the filename, e.g. qtop-40.tar.gz or qtop-40.2948.tar.gz.
Comment 4 Susi Lehtola 2010-09-19 13:56:42 EDT
A few comments from an everyday batch system user.

The
 Requires: torque
should read
 Requires: torque-client
as the queue tools are in that package.

I'm not totally sure if the Requires is necessary or if a meta-require should be placed, but on the other hand I can't come up with any other PBS queue system than torque, so it's probably OK.

Furthermore, you need to do some patching for the colormap file. There seems to be a bug in qtop, since it downloads the colormap to the home directory before checking if there is a colormap in /etc.

It's probably best to keep the option for the user to have his/her own colormap, so you'll probably want to rearrange lines 163-170 in qtop. You might want to report this upstream. 

Third, you need to place lines 32-60 in /etc/qtop.conf (mark this as config(noreplace)), as those lines contain configuration options that users might want to change. First of all having them in the "binary" is kind of sick, and second if users change the contents the changes will be lost whenever an upgrade is performed. Be sure to make a comment line in qtop indicating the correct location to make any changes in.
Comment 5 Fotis Georgatos 2010-09-19 15:10:05 EDT
Hi,

one more round, it is looking better now.
I fixed as much as I could, including feedback by Jussi Lehtola.
(I am the upstream and thanks btw)

Spec URL: http://cern.ch/fotis/qtop.spec
SRPM URL: http://cern.ch/fotis/qtop-40-3.src.rpm
Description: Monitoring tool for PBS-based cluster systems, incl. torque.

can you please verify it once more?

cheerio,
Fotis
Comment 6 Martin Gieseking 2010-09-19 15:19:42 EDT
Hi Fotis, 

please don't change the Product and Component fields of this ticket. Review requests should belong to Fedora/Package Review even if you plan to maintain the package for EPEL too.
Comment 7 Mark Chappell 2010-09-23 08:29:07 EDT
*** Bug 635257 has been marked as a duplicate of this bug. ***
Comment 8 Fotis Georgatos 2010-10-02 19:32:07 EDT
Here is the latest and greatest version, including nice command line options.

Spec URL: http://cern.ch/fotis/QTOP/qtop.spec
SRPM URL: http://cern.ch/fotis/QTOP/qtop-42-1.src.rpm
Description: Monitoring tool for PBS-based cluster systems, incl. torque.

I wonder why it takes so long for this ticket to progress...
is there perhaps any action pending on this side?

tia,
Fotis
Comment 9 Susi Lehtola 2010-10-03 05:38:25 EDT
Did you have a look at
 http://fedoraproject.org/wiki/PackageMaintainers/Join
as suggested by Martin in comment #1 ?

There's currently more than a hundred people waiting in queue for a sponsor (I counted something of the order of 160 packages that are blocked by FE-NEEDSPONSOR).

The idea of the sponsorship process is to check that anyone doesn't do anything stupid with his/her package(s), we have a lot of packaging guidelines for this. When you become a Fedora packager, you'll get the right to perform package reviews of your own; this also requires that you know the guidelines.

If you want to help your getting sponsored, I suggest you make another submission (check that it isn't in Fedora or in the review queue already), and that you do a couple of informal reviews of other packages that have been submitted. It's best to stick to bugs that are not tagged with FE-NEEDSPONSOR, as your sponsor will have to check your informal review (and possibly approve the package).
Comment 10 Steve Traylen 2010-12-15 14:38:19 EST
Hi Fotis, 

Did you manage to review any other packages informally.

http://fedoraproject.org/PackageReviewStatus/
Comment 11 Michael Schwendt 2011-02-05 08:30:38 EST
Please enter your real name in your bugzilla account preferences. That will also improve the current http://fedoraproject.org/PackageReviewStatus/NEEDSPONSOR.html list.
Comment 12 Steve Traylen 2011-11-22 16:20:52 EST
Marking as stalled on whiteboard, be sure to remove if anything changes.

http://fedoraproject.org/wiki/Package_Review_Process#The_Whiteboard
Comment 13 Fotis Georgatos 2011-11-23 02:53:00 EST
Hi,

I just don't know what is missing from the package to improve it.

tia for any bit of info,
Fotis
Comment 14 Fotis Georgatos 2012-01-29 19:17:18 EST
Latest and greatest version, including nice command line options.

Spec URL: http://cern.ch/fotis/QTOP/qtop.spec
SRPM URL: http://cern.ch/fotis/QTOP/qtop-45-1.src.rpm
Description: Monitoring tool for PBS-based cluster systems, incl. torque.

The build is proven to be working, reports of produced packages for all distros seen over here:
https://build.opensuse.org/package/show?package=qtop&project=home%3Ageorgatos

enjoy, ;-)
Fotis
Comment 15 Fotis Georgatos 2012-01-29 19:17:18 EST
    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
Requires torque package (ie. pbsnodes & qstat)
Comment 16 Jason Tibbitts 2012-04-25 22:50:02 EDT
I have no way at all to test this, but it does build and install fine.  Some comments and questions:

Do you intend to build this for RHEL5?  There is a whole bunch of stuff you can remove from the spec if you are targeting Fedora and RHEL6:  BuildRoot:, the entire %clean section and the cleaning of the buildroot in %install.

Your %description less a description of the software than it is documentation.  It should describe what the software does, so that an interested party can decide whether they wish to install it.  They can look to included documentation once the package is installed.

There's no need for the echo at the start of %prep.

You don't need a %build section at all if there's nothing to put there.

It is not necessary to include the %defattr line in %files.

Your %changelog entries must follow one of the formats from http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs

When I run rpmlint I get:
  qtop.noarch: W: non-standard-group Productivity/Clustering/Computing
Which is OK, I guess; nothing in Fedora cares about Group: and you're welcome to leave it out entirely.

  qtop.noarch: W: no-manual-page-for-binary qtop
This is fine; manpages are nice but not mandatory.

I'm not sure what you meant in comment 15; there's no process in Fedora that, and certainly nothing in the review process, that makes use of the Technical Notes field.

This is all pretty much trivial.  If you're interested in a full review, just say so and I'll get into it.
Comment 17 Fotis Georgatos 2012-06-06 16:43:02 EDT
Hello Jason et al,

On 26/04/2012 04:50, bugzilla@redhat.com wrote:
> Please do not reply directly to this email. All additional
> comments should be made in the comments box of this bug.
> https://bugzilla.redhat.com/show_bug.cgi?id=635256

> --- Comment #16 from Jason Tibbitts <tibbs@math.uh.edu> 2012-04-25 22:50:02 EDT ---
> Do you intend to build this for RHEL5?  There is a whole bunch of stuff you can
> remove from the spec if you are targeting Fedora and RHEL6:  BuildRoot:, the
> entire %clean section and the cleaning of the buildroot in %install.

The %clean section I'd rather leave it because this spec is multi-purpose:
It later gets used inside OpenSUSE Build Service to build packages
for other distributions (incl. mandrake/suse/debian/ubuntu).

> Your %description less a description of the software than it is documentation. 
> It should describe what the software does, so that an interested party can
> decide whether they wish to install it.  They can look to included
> documentation once the package is installed.

OK, I reduced it just a bit.

> There's no need for the echo at the start of %prep.

OK, I put it under comment so that it can be restored in the future if wanted.

> You don't need a %build section at all if there's nothing to put there.

OK, commented out.

> It is not necessary to include the %defattr line in %files.

Well, when I tried that I got this error:

qtop.src:50: E: files-attr-not-set
qtop.src:51: E: files-attr-not-set
qtop.src:52: E: files-attr-not-set
qtop.src:53: E: files-attr-not-set

> Your %changelog entries must follow one of the formats from
> http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs

OK, done.

> When I run rpmlint I get:
>   qtop.noarch: W: non-standard-group Productivity/Clustering/Computing
> Which is OK, I guess; nothing in Fedora cares about Group: and you're welcome
> to leave it out entirely.
> 
>   qtop.noarch: W: no-manual-page-for-binary qtop
> This is fine; manpages are nice but not mandatory.

Both the above, relate to the multi-distribution aspect of the spec file.
(I recall I tried to fix the man page issue but then something else breaks)

> I'm not sure what you meant in comment 15; there's no process in Fedora that,
> and certainly nothing in the review process, that makes use of the Technical
> Notes field.

I can't recall the context of writing that message at all; I'd say ignore it.

> This is all pretty much trivial.  If you're interested in a full review, just
> say so and I'll get into it.

Yes, let's go for a full review now!

Kindly check: http://fotis.web.cern.ch/fotis/QTOP/qtop.spec
(in it, you see the references for latest sources etc)

thanks,
Fotis
Comment 18 Fotis Georgatos 2012-07-14 19:54:54 EDT
Hi there,

this has been addressed some month ago so, let's give it another spin.

In short, it should be simply ready for release.

cheers,
Fotis
Comment 19 Fotis Georgatos 2012-07-31 06:53:49 EDT
Hi,

this is ready for release; what is pending?

[lxplus244] /afs/cern.ch/user/f/fotis > rpmlint ./rpmbuild/RPMS/noarch/qtop-45-1.noarch.rpm
qtop.noarch: W: non-standard-group Productivity/Clustering/Computing
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
[lxplus244] /afs/cern.ch/user/f/fotis > rpmlint ./rpmbuild/RPMS/noarch/qtop-45-1.noarch.rpm
qtop.noarch: W: non-standard-group Productivity/Clustering/Computing
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
Comment 20 Fotis Georgatos 2012-08-01 09:28:58 EDT
Hi,

just set Whiteboard field to "Trivial"; this should be ready for review/release.

tia,
F.
Comment 21 Fotis Georgatos 2012-08-07 05:25:13 EDT
Hi,

after 2 years I think this arrives to excaustion... 
I wonder what more can be done.
Comment 22 Fotis Georgatos 2012-08-07 06:24:19 EDT
Ah, btw, and here's the latest and greatest version:

Spec URL: http://cern.ch/fotis/QTOP/qtop.spec
SRPM URL: http://cern.ch/fotis/QTOP/qtop-46-1.src.rpm

and the rpmlint output in case you wonder:

[lxplus417] /afs/cern.ch/user/f/fotis/rpmbuild > rpmlint ../www/QTOP/qtop-46-1.*.rpm
qtop.noarch: W: non-standard-group Productivity/Clustering/Computing
qtop.src: W: non-standard-group Productivity/Clustering/Computing
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

ie. the groupname is non-standard (I need this, to have 1 rpm for all distros)

thanks,
Fotis
Comment 23 Fotis Georgatos 2012-08-07 07:02:36 EDT
    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -1 +1 @@
-Requires torque package (ie. pbsnodes & qstat)+Requires torque package (ie. pbsnodes & qstat), at run-time.
Comment 24 Fedora End Of Life 2013-04-03 14:51:52 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19
Comment 25 Christopher Meng 2013-08-17 01:17:43 EDT
Don't know why cern guys love el only... 

Change to rawhide to avoid CLOSE WONTFIX
Comment 26 Fotis Georgatos 2013-08-17 01:49:24 EDT
OK, thanks for looking into this request guys... fyi. 
Upstream (me :) has promoted the .spec file a few times since last touching this.
Comment 27 Fotis Georgatos 2013-08-17 01:54:35 EDT
OK, thanks for looking into this request guys... fyi. 
Upstream (me :) has promoted the .spec file a few times since last touching this.

the latest spec file lives here:
http://fotis.web.cern.ch/fotis/QTOP/qtop.spec
and it includes the pointer for the tarball...
Comment 28 Christopher Meng 2013-08-17 02:04:28 EDT
(In reply to Fotis Georgatos from comment #26)
> OK, thanks for looking into this request guys... fyi. 
> Upstream (me :) has promoted the .spec file a few times since last touching
> this.

Hi Fotis, I'm sure that cern has people been sponsored. 

So if you still cannot find a sponsor, take it easy, just let the approved guy handle this. Because you are upstream, should pay more attention to code but not packaging in this case. 

Just my suggestion. ;)
Comment 29 Fotis Georgatos 2013-08-17 02:52:21 EDT
btw. about:
> Don't know why cern guys love el only... 

just for the record:
The LCG (LHC Computing Grid) has been depending on SL5/6 for many many years now; as you may be aware, SL is a derivative of RHEL line, really => target is EPEL.

What I come to realize is, that you guys expect that at first things end up in the Fedora/rawhide pool and only *then* they can end-up towards EPEL etc. I think this should be spelled out more clearly/often and this is where the confusion came from. (at least in my case, I didn't realize the cycle is kinda obligatory)

Also, good practice here implies to prefer rawhide regardless; this is contrary to: "For packages in EPEL, you have to use the component epel to get the request to the right persons." , found at: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=PackageMaintainers/Join

I guess sending the gentle reminders around would help plenty of fellows!
Comment 30 Christopher Meng 2013-09-06 12:12:25 EDT
What's going on here? Have you found a sponsor?
Comment 31 Fotis Georgatos 2013-09-07 03:49:49 EDT
No sponsor, AFAIK; how do we get there?
Comment 32 Christopher Meng 2013-09-07 03:52:50 EDT
Read: http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Also, you can ask your cern friends who are staying in packager group.
Comment 33 Haïkel Guémar 2013-10-10 08:39:12 EDT
I can sponsor you if you agree, besides i wouldn't mind having a torque co-maintainer ;)
Comment 34 Fotis Georgatos 2013-10-10 09:02:41 EDT
Hi Haikel,

Yes, it would be great if you sponsored ;-)

At present I don't run torque as my main batch scheduler,
yet I have plenty of manhours experience on both it and building rpms!
Comment 35 Haïkel Guémar 2013-10-10 09:15:06 EDT
Great, let's deal first with the sponsoring part:
Please do two informal reviews from this list so i (and other sponsors) could assess your packaging skills.
http://fedoraproject.org/PackageReviewStatus/NEW.html
Please avoid the ones requiring sponsoring, so we can depile few reviews.
I might ask you on our packaging guidelines or few technicalities either by email or irc, though i'm confident that you have plenty RPM packaging experience from OBS or Scientific Linux :)

For your review, please provide latest spec and srpm. I recommend, that you clean the spec from commented instructions and unrequited echoes before.

Since you have some experience with Torque, i would be grateful if you help me maintain this (i'm a software engineer, not a scientist ;) ) at your convenance though.
Comment 36 Miroslav Suchý 2015-07-21 09:05:16 EDT
Closing due long inactivity. Feel free to reopne if you want to continue.
Comment 37 Fotis Georgatos 2015-07-21 11:01:01 EDT
Yes, please, let's keep this open!

F.

Note You need to log in before you can comment on or make changes to this bug.