Bug 442871 - Review Request: virt-top - Utility like top(1) for displaying virtualization stats
Review Request: virt-top - Utility like top(1) for displaying virtualization ...
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Extras Quality Assurance
Depends On: 442867
  Show dependency treegraph
Reported: 2008-04-17 06:18 EDT by Richard W.M. Jones
Modified: 2008-05-21 05:41 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-05-21 05:41:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tcallawa: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Richard W.M. Jones 2008-04-17 06:18:21 EDT
Spec URL: http://www.annexia.org/tmp/ocaml/virt-top.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/virt-top-1.0.0-2.fc9.src.rpm
Description: Utility like top(1) for displaying virtualization stats

This program is already in Fedora, but the upstream source package
has now been split up so it now needs to be in a separate package.

rpmlint is clean.
Comment 1 Tom "spot" Callaway 2008-05-01 14:07:48 EDT
A few items:

- You need BuildRequires: ocaml-fileutils-devel, ocaml-camlidl-devel, libvirt-devel
- You do not need to explicitly gzip the man page, this will happen automagically.

Show me a fixed package and I'll finish the review.
Comment 2 Richard W.M. Jones 2008-05-01 15:14:53 EDT
Sorry, that was embarrassing.  The package had suffered some bitrot since
I looked at it last.

This is a fixed package:

Spec URL: http://www.annexia.org/tmp/ocaml/virt-top.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/virt-top-1.0.1-1.fc9.src.rpm

* Thu May  1 2008 Richard W.M. Jones <rjones@redhat.com> - 1.0.1-1
- New upstream release 1.0.1.
- Don't BR ocaml-gettext-devel, it's not used at the moment.
- Don't gzip the manpage, it happens automatically.
- Add BR libvirt-devel.
- Remove spurious executable bit on COPYING.

Here is a Koji scratch build in dist-f10:


Now I'm off to similarly check virt-df & virt-ctrl ...
Comment 3 Tom "spot" Callaway 2008-05-13 13:51:01 EDT
You still need BuildRequires: ocaml-fileutils-devel, ocaml-camlidl-devel ...
Comment 4 Richard W.M. Jones 2008-05-13 14:44:34 EDT
Are you sure that I need these?  I don't see where those dependencies would
come from, and indeed the 1.0.1-1 version does build in Koji.
Comment 5 Tom "spot" Callaway 2008-05-13 15:06:59 EDT
When I do a local build on my x86_64 f9 box, I get:

ocamlfind ocamlopt \
	  -package unix,extlib,curses,str,libvirt -package gettext-stub -package
xml-light -package csv -package calendar -w s -linkpkg \
	  -cclib -lncurses -o virt-top.opt virt_top_gettext.cmx virt_top_utils.cmx
virt_top.cmx virt_top_xml.cmx virt_top_csv.cmx virt_top_calendar2.cmx
Cannot find file /usr/lib64/ocaml/fileutils/fileutils.cmxa

Comment 6 Richard W.M. Jones 2008-05-13 16:56:52 EDT
OK, I understand.  It's an upstream bug in the configure script:

If ocaml-gettext is installed, then configure detects & uses it, but unfortunately it depends on
ocaml-fileutils-devel, etc.

If ocaml-gettext isn't installed, then configure replaces the gettext functions with dummies
which don't need all the other stuff.

I'm setting it back to NEEDINFO for me to fix this ...
Comment 7 Richard W.M. Jones 2008-05-19 13:40:56 EDT
Here's a new version which uses ocaml-gettext.  Thanks for your
patience and persistence.

Spec URL: http://www.annexia.org/tmp/ocaml/virt-top.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/virt-top-1.0.1-2.fc10.src.rpm

* Mon May 19 2008 Richard W.M. Jones <rjones@redhat.com> - 1.0.1-2
- Use RPM percent-configure.
- Add list of BRs for gettext.
- Use find_lang to find PO files.
- Comment out the OCaml dependency generator.  Not a library so not
Comment 8 Richard W.M. Jones 2008-05-19 13:53:15 EDT
Koji scratch build of this package:

Comment 9 Tom "spot" Callaway 2008-05-19 15:20:19 EDT

- rpmlint checks return... nothing! :)
- package meets naming guidelines
- package meets packaging guidelines (general, ocaml)
- license (GPLv2+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (89eeae9afe0c86b262c7adf2d693110050de0f22)
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- locales ok
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

Comment 10 Richard W.M. Jones 2008-05-19 16:23:35 EDT
Thank you for this.  I know it's been somewhat trying.

CVS request coming up ...
Comment 11 Richard W.M. Jones 2008-05-19 16:24:40 EDT
New Package CVS Request
Package Name: virt-top
Short Description: Utility like top(1) for displaying virtualization stats
Owners: rjones
Branches: F-8 F-9 EL-5
InitialCC: rjones
Cvsextras Commits: yes
Comment 12 Kevin Fenzi 2008-05-19 22:36:28 EDT
cvs done.
Comment 13 Richard W.M. Jones 2008-05-20 04:54:08 EDT
Built in Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=619407
Comment 15 Richard W.M. Jones 2008-05-20 05:14:51 EDT
Waiting for ocaml-libvirt >= 4.1.1 before I can build in F-8.
Comment 16 Richard W.M. Jones 2008-05-21 05:41:58 EDT
Built and pushed for F-8.

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