Bug 691096 - Review Request: iperf3 - Measurement tool for TCP/UDP bandwidth performance
Summary: Review Request: iperf3 - Measurement tool for TCP/UDP bandwidth performance
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords: Reopened
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-03-26 17:57 UTC by Balaji G
Modified: 2013-05-22 21:40 UTC (History)
7 users (show)

Fixed In Version: iperf3-3.0-0.2.b4.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-05-19 18:51:38 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Balaji G 2011-03-26 17:57:36 UTC
http://balajig8.fedorapeople.org/packages/iperf.spec 
http://balajig8.fedorapeople.org/packages/iperf-3.0b4-1.fc14.src.rpm 

Measurement tool for TCP/UDP bandwidth performance

Comment 1 Kevin Fenzi 2011-03-31 12:34:20 UTC
Hey G. 

Just a few quick things I noticed to be fixed before I do my review checklist. ;) 

1. The spec and Name: should be 'iperf3' here, right? Or are you intending to replace/update the existing iperf package? 

2. The version here is not right. You should not have things like 'b4' in version. ;) 
See: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages
I think you want: 
Version: 3.0
Release: 0.0.b4%{?dist}

Then when the final 3.0 comes out, Release goes to 1 and it updates correctly from this beta version. 
If you make changes to packaging before that you can use: 

Release: 0.1.b4%{?dist}

etc. And the update path works. ;) 

3. You probibly don't want to ship the static library: 
http://fedoraproject.org/wiki/PackagingGuidelines#Packaging_Static_Libraries
I would suggest removing the .a and header files at the end of %install and then commenting out the -devel subpackage entirely. You could add it back in when/if they have a dynamic library or someone really needs the static one. ;) 

Anyhow, if you could look at those, I could start a formal review after that. 

Thanks!

Comment 2 Balaji G 2011-04-06 14:34:24 UTC
Kevin, 

Updated with your comments and uploaded to the following location

http://balajig8.fedorapeople.org/packages/iperf3.spec
http://balajig8.fedorapeople.org/packages/iperf3-3.0-0.0.b4.fc14.src.rpm

Thanks for your time.

Thanks,
Cheers,
  - Balaji

Comment 3 Kevin Fenzi 2011-04-10 21:02:32 UTC
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
See below - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
See below - License
See below - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
fde024a200b064b54accd1959f7e642e  iperf-3.0b4.tar.gz
fde024a200b064b54accd1959f7e642e  iperf-3.0b4.tar.gz.orig

See below - BuildRequires correct
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 - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should function as described. 
OK - Should have sane scriptlets. 
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues: 

1. Can you pick one of "$RPM_BUILD_ROOT" or "%{buildroot}" and use that only
in the spec. Makes things more readable. ;) 

2. Some of the source files contain: 

 * Copyright (c) 2009, The Regents of the University of California, through
 * Lawrence Berkeley National Laboratory (subject to receipt of any required
 * approvals from the U.S. Dept. of Energy).  All rights reserved.

I guess I would suggest a mail to upstream developers. Ask them if these 
files really are supposed to be released under the BSD license. :( 

3. Can you avoid %makeinstall ?
https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used

4. It looks like it's looking for uuid: 
checking for uuid_create... no
You might add uuid-devel to BuildRequires? 

5. rpmlint says: 
iperf3.x86_64: W: incoherent-version-in-changelog 3.0b4-2 ['3.0-0.0.b4.fc15', '3.0-0.0.b4']
the changelog should not have "3.0b4-2" but "3.0-0.0.b4"

Comment 4 Balaji G 2011-04-12 17:48:19 UTC
Kevin,

Thanks for the comments. Have uploaded the updated spec file and the SRPMs in the same path. Have sent an email to the up-stream guys regarding the license issues.

Thanks,
Cheers,
  - Balaji

Comment 5 Kevin Fenzi 2011-04-14 03:14:51 UTC
ok, 1, 3, 4, 5 all look good. 

Just waiting to hear back on the legal item in 2. ;) 

Thanks for the fixes.

Comment 6 Balaji G 2011-04-15 08:51:55 UTC
Kevin,

Thanks a lot.I hope this gets pushed in early :) 

Cheers,
  - Balaji

Comment 7 Kevin Fenzi 2011-04-20 22:51:03 UTC
Upstream replied. They added a note as to what license this is under, but there is still the 'all rights reserved' part, so not sure what to do here. Do they have the right to change the distribution of something thats saying it's all rights reserved and owned by The Regents of the University of California?

http://groups.google.com/group/iperf-dev/t/3ac7f9aafca8419d

Blocking on LEGAL to review.

Comment 8 Tom "spot" Callaway 2011-04-21 16:26:40 UTC
Not a problem. "All Rights Reserved" is a no-op, and doesn't affect the licensing. Lifting FE-Legal.

Comment 9 Kevin Fenzi 2011-04-22 16:44:57 UTC
Everything looks good to me here now, so this package is APPROVED.

Comment 10 Balaji G 2011-04-24 03:21:02 UTC
New Package SCM Request
=======================
Package Name: iperf3
Short Description: Measurement tool for TCP/UDP bandwidth performance
Owners: balajig8 kevin
Branches: f14 f15 e16
InitialCC: balajig8 kevin

Comment 11 Jason Tibbitts 2011-04-24 14:43:55 UTC
You requested an invalid branch "e16".  I'm not sure if this was a typo for
"f16" (in which case it's too early) or for "el6" so I've simply done the f14
and f15 branches.  If you did want an el6 branch, feel free to submit a change
request.

Git done (by process-git-requests).

Comment 12 Fedora Update System 2011-04-26 03:05:27 UTC
iperf3-3.0-0.0.b4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/iperf3-3.0-0.0.b4.fc15

Comment 13 Fedora Update System 2011-04-26 03:09:29 UTC
iperf3-3.0-0.0.b4.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/iperf3-3.0-0.0.b4.fc14

Comment 14 Fedora Update System 2011-04-26 15:35:20 UTC
iperf3-3.0-0.0.b4.fc15 has been pushed to the Fedora 15 testing repository.

Comment 15 Fedora Update System 2011-05-01 03:21:37 UTC
iperf3-3.0-0.0.b4.fc15 has been pushed to the Fedora 15 stable repository.

Comment 16 Fedora Update System 2011-05-05 18:22:59 UTC
iperf3-3.0-0.0.b4.fc14 has been pushed to the Fedora 14 stable repository.

Comment 17 François Cami 2011-12-23 12:10:00 UTC
Could you provide iperf3 for el6, and possibly el5?
I can co-maint for EPEL if needed.
Thank you.

Comment 18 Steven Roberts 2013-02-06 05:44:22 UTC
Package Change Request
======================
Package Name: iperf3
New Branches: el5 el6
Owners: strobert kevin

adding EPEL branches

Comment 19 Gwyn Ciesla 2013-02-06 13:31:49 UTC
Git done (by process-git-requests).

Comment 20 Fedora Update System 2013-02-10 06:36:16 UTC
iperf3-3.0-0.2.b4.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/iperf3-3.0-0.2.b4.el5

Comment 21 Fedora Update System 2013-02-10 06:38:12 UTC
iperf3-3.0-0.2.b4.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/iperf3-3.0-0.2.b4.el6

Comment 22 Fedora Update System 2013-02-10 19:03:46 UTC
iperf3-3.0-0.2.b4.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 23 Kevin Fenzi 2013-05-19 18:51:38 UTC
This is all in and can be closed out.

Comment 24 Fedora Update System 2013-05-22 21:40:09 UTC
iperf3-3.0-0.2.b4.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 25 Fedora Update System 2013-05-22 21:40:59 UTC
iperf3-3.0-0.2.b4.el5 has been pushed to the Fedora EPEL 5 stable repository.


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