Bug 442873 - Review Request: virt-df - Utility like 'df' for virtual guests
Review Request: virt-df - Utility like 'df' for virtual guests
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Chris Lalancette
Fedora Extras Quality Assurance
http://et.redhat.com/~rjones/virt-df/
:
Depends On: 442705 442867
Blocks:
  Show dependency treegraph
 
Reported: 2008-04-17 06:32 EDT by Richard W.M. Jones
Modified: 2008-05-22 07:15 EDT (History)
2 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Richard W.M. Jones 2008-04-17 06:32:42 EDT
Spec URL: http://www.annexia.org/tmp/ocaml/virt-df.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/virt-df-2.0.0-2.fc9.src.rpm
Description: Utility like 'df' for virtual guests

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 Richard W.M. Jones 2008-05-01 15:58:31 EDT
Spec URL: http://www.annexia.org/tmp/ocaml/virt-df.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/virt-df-2.0.2-1.fc9.src.rpm
* Thu May  1 2008 Richard W.M. Jones <rjones@redhat.com> - 2.0.2-1
- New upstream version 2.0.2.
- Don't depend on ocaml-gettext, it's not used at the moment.
- Don't gzip the manpage, it happens automagically.
- This version needs bitmatch >=  0.5.
- +BR ocaml-xml-light-devel
- +BR ocaml-camlp4-devel

Here's a Koji scratch build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=591884
Comment 2 Richard W.M. Jones 2008-05-13 16:50:26 EDT
Spec URL: http://www.annexia.org/tmp/ocaml/virt-df.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/virt-df-2.1.1-1.fc9.src.rpm

* Tue May 13 2008 Richard W.M. Jones <rjones@redhat.com> - 2.1.1-1
- New upstream version 2.1.1.
- Needs bitmatch >= 0.9.

Koji scratch build in dist-f10:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=607745
Comment 3 Richard W.M. Jones 2008-05-13 17:09:02 EDT
Judging by bug 442871 this probably fails to build if ocaml-gettext is installed.
Setting it back to NEEDINFO of me to fix.
Comment 4 Richard W.M. Jones 2008-05-16 12:43:45 EDT
This version works with ocaml-gettext:

Spec URL: http://www.annexia.org/tmp/ocaml/virt-df.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/virt-df-2.1.1-5.fc10.src.rpm
* Fri May 16 2008 Richard W.M. Jones <rjones@redhat.com> - 2.1.1-5
- Reenable ocaml-gettext-devel BR and get it working properly.
- Use find_lang to find PO files.
- +BR ocaml-fileutils-devel (not strictly needed, but a dependency
  of gettext).
- +BR ocaml-camomile-data except on ppc64.

Koji build:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=612794
Comment 5 Chris Lalancette 2008-05-19 10:37:32 EDT
+ rpmlint output

Clean, according to bug submitter.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
? package should satisfy packaging guidelines
? license meets guidelines and is acceptable to Fedora
? license matches the actual package license
? %doc includes license file
? spec file written in American English
? spec file is legible
? upstream sources match sources in the srpm
? package successfully builds on at least one architecture
? ExcludeArch bugs filed
? BuildRequires list all build dependencies
? %find_lang instead of %{_datadir}/locale/*
? binary RPM with shared library files must call ldconfig in %post and %postun
? does not use Prefix: /usr
? package owns all directories it creates
? no duplicate files in %files
? %defattr line
? %clean contains rm -rf $RPM_BUILD_ROOT
? consistent use of macros
? package must contain code or permissible content
? large documentation files should go in -doc subpackage
? files marked %doc should not affect package
? header files should be in -devel
? static libraries should be in -static
? packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
? libfoo.so must go in -devel
? -devel must require the fully versioned base
? packages should not contain libtool .la files
? packages containing GUI apps must include %{name}.desktop file
? packages must not own files or directories owned by other packages
? %install must start with rm -rf %{buildroot} etc.
? filenames must be valid UTF-8

Optional:

? if there is no license file, packager should query upstream
? translations of description and summary for non-English languages, if available
? reviewer should build the package in mock
? the package should build into binary RPMs on all supported architectures
? review should test the package functions as described
? scriptlets should be sane
? pkgconfig files should go in -devel
? shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin
Comment 6 Chris Lalancette 2008-05-19 10:39:12 EDT
Sorry about that last comment....I meant to delete it, but I forgot.  I'll
update with a more complete review.

Chris Lalancette
Comment 7 Chris Lalancette 2008-05-19 10:45:31 EDT
When trying to build this package on F-9 with ocaml packages update to F-10, I
seem to be running into a build failure:

+ make depend
for d in lib virt-df diskzip; do \
          make -C $d depend; \
          if [ $? -ne 0 ]; then exit 1; fi; \
        done
make[1]: Entering directory `/usr/src/redhat/BUILD/virt-df-2.1.1/lib'
rm -f .depend
ocamlfind ocamldep -package unix,extlib -I +bitmatch -pp "camlp4o -I`ocamlc
-where`/bitmatch pa_bitmatch.cmo" diskimage_ext2.mli diskimage_fat.mli
diskimage_impl.mli diskimage_linux_swap.mli diskimage_linux_swsuspend.mli
diskimage_lvm2_metadata.mli diskimage_lvm2.mli diskimage_mbr.mli diskimage.mli
diskimage_ntfs.mli int63_on_32.mli int63_on_64.mli diskimage_ext2.ml
diskimage_fat.ml diskimage_impl.ml diskimage_linux_swap.ml
diskimage_linux_swsuspend.ml diskimage_lvm2_metadata.ml diskimage_lvm2.ml
diskimage_mbr.ml diskimage.ml diskimage_ntfs.ml int63_on_32.ml int63_on_64.ml
test_int63.ml > .depend
Camlp4: Uncaught exception: DynLoader.Error
("/usr/lib/ocaml/bitmatch/pa_bitmatch.cmo", "error while linking
/usr/lib/ocaml/bitmatch/pa_bitmatch.cmo.\nReference to undefined global `Bitmatch'")

(and then there are a bunch more build failures).  Rich points out that this
patch is probably needed:

http://hg.et.redhat.com/virt/applications/virt-df--devel?cs=00a35ad4c881

Chris Lalancette
Comment 8 Richard W.M. Jones 2008-05-19 10:51:45 EDT
Spec URL: http://www.annexia.org/tmp/ocaml/virt-df.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/virt-df-2.1.1-6.fc10.src.rpm
* Sun May 18 2008 Richard W.M. Jones <rjones@redhat.com> - 2.1.1-6
- Include upstream cset 00a35ad4c881 which fixes build with
  latest bitmatch.
Comment 9 Chris Lalancette 2008-05-19 11:38:35 EDT
OK, review with updated package:

+ rpmlint output

Clean, according to bug submitter.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license
+ %doc includes license file
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
+ package successfully builds on at least one architecture
n/a ExcludeArch bugs filed
+ BuildRequires list all build dependencies
+ %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and %postun
+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a -devel must require the fully versioned base
n/a packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.

It actually starts with rm -rf $RPM_BUILD_ROOT, but I believe that is valid as
part of the guidelines, and $RPM_BUILD_ROOT is used consistently throughout the
specfile.

+ filenames must be valid UTF-8

Optional:

+ if there is no license file, packager should query upstream
- translations of description and summary for non-English languages, if available

Translations of these don't seem to be available

- reviewer should build the package in mock
+ the package should build into binary RPMs on all supported architectures
- review should test the package functions as described
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin

This package looks fine.

APPROVED
Comment 10 Richard W.M. Jones 2008-05-19 11:50:31 EDT
Excellent news, thanks for looking at this Chris.

CVS request coming up ...
Comment 11 Richard W.M. Jones 2008-05-19 11:51:32 EDT
New Package CVS Request
=======================
Package Name: virt-df
Short Description: Utility like 'df' for virtual guests
Owners: rjones
Branches: F-8 F-9
InitialCC: rjones
Cvsextras Commits: yes
Comment 12 Kevin Fenzi 2008-05-19 12:07:09 EDT
cvs done.
Comment 13 Richard W.M. Jones 2008-05-19 13:30:21 EDT
Built for Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=617922

F8/F9 have to wait until the latest ocaml-bitmatch is pushed to
stable.
Comment 14 Richard W.M. Jones 2008-05-22 07:15:23 EDT
Now built for F8/F9.

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