Bug 981707 - Review Request: bmon - bandwidth monitor and rate estimator
Summary: Review Request: bmon - bandwidth monitor and rate estimator
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Meng
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-07-05 14:30 UTC by Thomas Graf
Modified: 2014-09-12 13:44 UTC (History)
8 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2014-01-09 11:00:25 UTC
Type: ---
Embargoed:
i: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Thomas Graf 2013-07-05 14:30:52 UTC
Spec URL: http://www.infradead.org/~tgr/bmon/files/rhel/bmon.spec
SRPM URL: http://www.infradead.org/~tgr/bmon/files/rhel/bmon-3.1-1.fc19.src.rpm
Description: bmon is a monitoring and debugging tool to capture networking related statistics and prepare them visually in a human friendly way.
Fedora Account System Username: tgraf

Comment 1 Christopher Meng 2013-07-05 15:26:46 UTC
I firstly built on my machine and failed, I think something might be wrong within mock, but then I think there are something wrong in your configure script. I've installed libnl1 and 3 with every subpackages on my local machine, and it still failed.

Here is the build task:

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

Please fix by reviewing the build.log.

And please remove "rm -rf %{buildroot}" in %install section.

I'll review when you submit the next one.

Comment 2 Thomas Graf 2013-07-06 08:58:06 UTC
Fixed:
 * BuildRequires typo s/libnl-devel/libnl3-devel/
 * Removed %{buildroot} deletion in install

INFO: Done(bmon-3.1-1.fc19.src.rpm) Config(fedora-19-x86_64) 1 minutes 40 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-19-x86_64/result

Comment 3 Christopher Meng 2013-07-06 09:15:17 UTC
Ah...Where are the new SRPM?

If you update the package, please bump the release from 1 ---> 2 with%{?dist}

Comment 4 Christopher Meng 2013-07-06 09:39:35 UTC
Review from the first URL of SRPM, not sure if this one.

Many issues:

1. 2 files with unknown license

include/bmon/defs.h
include/bmon/list.h

2. In %files section, %{_sysconfdir}/bmon.conf should be:

%config(noreplace) %{_sysconfdir}/bmon.conf

3. rpmlint: 
explicit-lib-dependency libconfuse
explicit-lib-dependency libnl3

4. Manpage has incorrect syntax:
manual-page-warning /usr/share/man/man8/bmon.8.gz 203: warning: macro `NF' not defined
manual-page-warning /usr/share/man/man8/bmon.8.gz 205: warning: macro `FI' not defined

Comment 5 Thomas Graf 2013-07-06 11:49:23 UTC
(In reply to Christopher Meng from comment #4)
> Review from the first URL of SRPM, not sure if this one.

spec and SRPM updated in place.

> Many issues:
> 
> 1. 2 files with unknown license
> 
> include/bmon/defs.h

This is auto generated

> include/bmon/list.h

The header clearly states it comes from the kernel source.

> 2. In %files section, %{_sysconfdir}/bmon.conf should be:
> 
> %config(noreplace) %{_sysconfdir}/bmon.conf

Fixed

> 3. rpmlint: 
> explicit-lib-dependency libconfuse
> explicit-lib-dependency libnl3

Removed Requires

> 4. Manpage has incorrect syntax:
> manual-page-warning /usr/share/man/man8/bmon.8.gz 203: warning: macro `NF'
> not defined
> manual-page-warning /usr/share/man/man8/bmon.8.gz 205: warning: macro `FI'
> not defined

Fixed in git tree, will be included in 3.2

Comment 6 Christopher Meng 2013-07-06 13:15:19 UTC
explicit-lib-dependency libconfuse
explicit-lib-dependency libnl3

is still appeared, however I think they are false positives.


APPROVED.

Comment 7 Thomas Graf 2013-07-07 12:09:58 UTC
New Package SCM Request
=======================
Package Name: bmon
Short Description: bandwidth monitor and rate estimator
Owners: tgraf
Branches: f18 f19 el6
InitialCC:

Comment 8 Gwyn Ciesla 2013-07-07 19:15:17 UTC
Git done (by process-git-requests).

Comment 9 Terje Røsten 2013-07-08 07:20:12 UTC
> 
> > include/bmon/list.h
> 
> The header clearly states it comes from the kernel source.

Then license tag must be modified, kernel is not BSD.

> Source0: http://www.infradead.org/~tgr/bmon/files/bmon-3.1.tar.gz

Use version macro here, it will really help when doing updates:

Source0: http://www.infradead.org/~tgr/bmon/files/bmon-${version}.tar.gz

Comment 10 Christoph Wickert 2013-07-31 22:59:00 UTC
More issues:

As you can see in http://kojipkgs.fedoraproject.org//packages/bmon/3.1/1.fc20/data/logs/i686/build.log, make is not verbose, so compiler flags cannot be verified. Add "V=1" to the make call.

AFAICS bmon.conf is not generated during build but comes directly from the source tarball. You should preserve it's timestamp by adding 'INSTALL=install -p' to the 'make install' call.

Comment 11 Thomas Graf 2013-09-15 12:33:12 UTC
Fixed in bmon-3.1-4

Comment 12 Fedora Update System 2013-09-15 12:42:51 UTC
bmon-3.1-4.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/bmon-3.1-4.fc20

Comment 13 Terje Røsten 2013-11-24 13:35:15 UTC
Thomas,

can you please push bmon to stable?

Or any reason to keep it in testing for many weeks?

Comment 14 Thomas Graf 2013-11-28 08:22:01 UTC
(In reply to Terje Røsten from comment #13)
> Thomas,
> 
> can you please push bmon to stable?
> 
> Or any reason to keep it in testing for many weeks?

Pushed to stable.

Comment 15 Fedora Update System 2013-12-14 03:04:49 UTC
bmon-3.1-4.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Thomas Graf 2014-09-12 13:05:24 UTC
Package Change Request
======================
Package Name: bmon
New Branches: epel7
Owners: tgraf

Comment 17 Gwyn Ciesla 2014-09-12 13:44:52 UTC
Git done (by process-git-requests).


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