Red Hat Bugzilla – Bug 245515
Review Request: pv - A tool for monitoring the progress of data through a pipeline
Last modified: 2007-11-30 17:12:08 EST
Spec URL: http://hrozkovi.homelinux.org/pv/pv.spec
SRPM URL: http://hrozkovi.homelinux.org/pv/pv-0.9.9-1.fc8.src.rpm
PV ("Pipe Viewer") is a tool for monitoring the progress of data through a
pipeline. It can be inserted into any normal pipeline between two processes
to give a visual indication of how quickly data is passing through, how long
it has taken, how near to completion it is, and an estimate of how long it
will be until completion.
I'd be happy to review this package.
Look for a full review in a bit here...
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (Artistic)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
See below - BuildRequires correct
OK - Spec handles locales/find_lang
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
OK - No rpmlint output.
OK - final provides and requires are sane.
OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have dist tag
OK - Should package latest version
1. Not a blocker, but I see that the during the build the install attempts
to install a pv.info file, which doesn't exist. Might ask upstream about either
including that or removing the code which refers to it.
/usr/bin/install -c -m 644 doc/pv.info \
/usr/bin/install: cannot stat `doc/pv.info': No such file or directory
make: [install] Error 1 (ignored)
2. Also, it looks like 'man' is called during build:
man doc/quickref.1 | sed 's/.^H//g' | cat -s > doc/quickref.txt || :
/bin/sh: man: command not found
Should there be a BuildRequires: man ?
I suppose only if you wanted to package the quickref.txt file?
That file seems to be a copy of the man page...
Thanks for stepping up for the review and for spotting the issues with man and
I'm on vacation now, so I'll be able to look into the issues you described in
more detail when I'm back (22-Jul).
Excellent. Have a great vacation!
OK, I'm back :)
I queried upstream about the info file, I'll update this bug report when the
Per the quickref issue - I personally don't think that including that file is
necessary since it adds nothing compared to the man page..should I tune the
install phase so it doesn't look for the quickref file? What are your thoughts
Reply from the maintainer:
> I work on packaging pv for Fedora and have one issue that I'd like to
> ask you about - it seems that during install, the install attempts
> to install a pv.info file, which doesn't exist:
Oops. There used to be one, but I stopped maintaining the Texinfo
documentation because I failed to see the point.
Because the install command ignores failures (I think I did that so that
installation would still work on systems without texinfo) I never noticed.
The next release will fix this, but I've no idea when that will be. In the
meantime, something like "sed -i '/\.info/d' autoconf/make/unreal.mk" just
before running "configure" will stop the warning by removing the relevant
bits from the Makefile.
ok, sounds good.
Neither of those issues seems to be blockers, just wanted to make sure they were
addressed. Sounds like the info issue will be fixed upstream. I don't think it
adds much to package quickref.txt, as it's just a duplicate of the man page. You
can package it if you like, or leave it out, it's up to you.
I see no further issues, so this package is APPROVED.
Don't forget to close this once it's been imported and built.
Also, do consider going and reviewing another waiting package to help spread out
the review load.
New Package CVS Request
Package Name: pv
Short Description: A tool for monitoring the progress of data through a
Branches: FC-6 F-7 EL-4 EL-5
Thanks for reviewing my package, Kevin!
The package has been successfully built for the devel branch:
I'm closing this review.