Bug 442871

Summary: Review Request: virt-top - Utility like top(1) for displaying virtualization stats
Product: [Fedora] Fedora Reporter: Richard W.M. Jones <rjones>
Component: Package ReviewAssignee: Tom "spot" Callaway <tcallawa>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting
Target Milestone: ---Flags: tcallawa: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://et.redhat.com/~rjones/virt-top/
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-05-21 09:41:58 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 442867    
Bug Blocks:    

Description Richard W.M. Jones 2008-04-17 10:18:21 UTC
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 18:07:48 UTC
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 19:14:53 UTC
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> - 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:

  http://koji.fedoraproject.org/koji/taskinfo?taskID=591819

Now I'm off to similarly check virt-df & virt-ctrl ...

Comment 3 Tom "spot" Callaway 2008-05-13 17:51:01 UTC
You still need BuildRequires: ocaml-fileutils-devel, ocaml-camlidl-devel ...

Comment 4 Richard W.M. Jones 2008-05-13 18:44:34 UTC
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 19:06:59 UTC
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
virt_top_main.cmx
Cannot find file /usr/lib64/ocaml/fileutils/fileutils.cmxa



Comment 6 Richard W.M. Jones 2008-05-13 20:56:52 UTC
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 17:40:56 UTC
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> - 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
  needed.


Comment 8 Richard W.M. Jones 2008-05-19 17:53:15 UTC
Koji scratch build of this package:

http://koji.fedoraproject.org/koji/taskinfo?taskID=618014

Comment 9 Tom "spot" Callaway 2008-05-19 19:20:19 UTC
Good:

- 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

APPROVED.


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

CVS request coming up ...

Comment 11 Richard W.M. Jones 2008-05-19 20:24:40 UTC
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-20 02:36:28 UTC
cvs done.


Comment 13 Richard W.M. Jones 2008-05-20 08:54:08 UTC
Built in Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=619407

Comment 15 Richard W.M. Jones 2008-05-20 09:14:51 UTC
Waiting for ocaml-libvirt >= 4.1.1 before I can build in F-8.

Comment 16 Richard W.M. Jones 2008-05-21 09:41:58 UTC
Built and pushed for F-8.