Bug 560728 - Review Request: uperf - Network performance tool with modelling and replay support
Summary: Review Request: uperf - Network performance tool with modelling and replay s...
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Kalev Lember
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2010-02-01 17:50 UTC by Terje Røsten
Modified: 2010-03-20 09:27 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2010-03-20 09:27:42 UTC
kalevlember: fedora-review+
kevin: fedora-cvs+

Attachments (Terms of Use)

Description Terje Røsten 2010-02-01 17:50:31 UTC
spec: http://terjeros.fedorapeople.org/uperf/uperf.spec
srpm: http://terjeros.fedorapeople.org/uperf/uperf-1.0.3-0.1.beta.fc12.src.rpm
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1957038
Network performance tool that supports modelling and replay of various
networking patterns.

Comment 1 Susi Lehtola 2010-02-02 10:29:02 UTC
Wouldn't running
 find src -type f -exec chmod 0644 {} \;
be easier?

Comment 2 Terje Røsten 2010-02-02 11:53:47 UTC
Might be, I just don't like the -exec construction and I love to pipe!

Comment 3 Kalev Lember 2010-03-17 22:05:31 UTC
Why are you bundling client.pem and server.pem with other documentation? I don't think it's a good idea to distribute x509 certificates with private keys like that. Everybody should always generate their own certificates and private keys.

Comment 5 Kalev Lember 2010-03-18 09:03:41 UTC
Fedora review uperf-1.0.3-0.1.beta.fc12.src.rpm 2010-03-18

+ OK
! needs attention

rpmlint output:
uperf.src: W: spelling-error Summary(en_US) modelling -> modeling, model ling, model-ling
uperf.src: W: spelling-error %description -l en_US modelling -> modeling, model ling, model-ling
uperf.i686: W: spelling-error Summary(en_US) modelling -> modeling, model ling, model-ling
uperf.i686: W: spelling-error %description -l en_US modelling -> modeling, model ling, model-ling
3 packages and 1 specfiles checked; 0 errors, 4 warnings.

I think rpmlint warning about "modelling" -> "modeling" is wrong. AFAIK both "modelling" and "modeling" are fine for US English. You can change this if you want to get rid of the rpmlint warning, but I believe it to be just noise.

+ The package is named according to the Package Naming Guidelines.
+ Spec file name matches the base package name
+ The package follows the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The license field in the spec file matches the actual license
! The license file (COPYING) only contains the following lines:
> All right reserved
> Sun Microsystems

Please query upstream to include a GPLv3 license file instead, but this isn't a review blocker. All source files have correct GPLv3 headers, leaving no interpretation for actual license.

+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  428035a33729a877f1ecd4ced8911a6f  uperf-1.0.3-beta.tar.gz
  428035a33729a877f1ecd4ced8911a6f  Download/uperf-1.0.3-beta.tar.gz
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a The spec file handles locales properly
+ Package does not bundle copies of system libraries
n/a Does not use Prefix: /usr
+ Package owns all directories it creates
+ No duplicate files in %files
+ Permissions are properly set and %files has %defattr
+ %clean contains rm -rf %{buildroot}
+ Consistent use of macros
+ Package contains code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package (see below)
n/a Header files should be in -devel
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
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
+ Filenames must be valid UTF-8

> # Included in doc/workloads
> %{__rm} -rf %{buildroot}%{_datadir}

I think it would be better to install the .xml workload profile files in %{_datadir}/uperf/. The files aren't just examples; they are pretty useful if you want to quickly try out uperf. Now if some user of uperf sets up a script to use these .xml files from the current location (/usr/share/doc/uperf-1.0.3), he needs to hardcode that directory in the script file. However, putting these profile files in %{_datadir}/uperf/ instead has the benefit that
 a) the directory name remains the same even if you update to uperf 1.0.4,
 b) it's more standard location, as both archlinux pkgbuild and an opensuse rpm I found put the files in /usr/share/uperf/ instead.

Also, /usr/share/doc/uperf-1.0.3/workloads/ currently contains Makefile* files which need to be removed.

Comment 7 Kalev Lember 2010-03-18 09:47:12 UTC
Looks good.


Comment 8 Terje Røsten 2010-03-18 09:54:27 UTC

Comment 9 Terje Røsten 2010-03-18 11:39:12 UTC
New Package CVS Request
Package Name: uperf
Short Description: Network performance tool with modelling and replay support
Owners: terjeros
Branches: F-11 F-12 F-13

Comment 10 Kevin Fenzi 2010-03-19 19:47:05 UTC
CVS done (by process-cvs-requests.py).

Comment 11 Terje Røsten 2010-03-20 09:27:42 UTC
Imported, built and pushed.

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