Bug 848388 - Review Request: liblognorm - Tool to normalize log data
Summary: Review Request: liblognorm - Tool to normalize log data
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tomas Mraz
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 847811 847817
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-08-15 13:17 UTC by mahaveer darade
Modified: 2013-03-01 05:30 UTC (History)
12 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-11-15 02:38:31 UTC
Type: ---
Embargoed:
tmraz: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description mahaveer darade 2012-08-15 13:17:23 UTC
Spec URL: http://theinric.fedorapeople.org/liblognorm/liblognorm.spec
SRPM URL: http://theinric.fedorapeople.org/liblognorm/liblognorm-0.3.4-1.src.rpm
Description: Liblognorm is a tool to normalize log data.
Fedora Account System Username: Mahaveer 

This is my third package and I need a sponsor.


rpmlint logs:
-------------

[mdarade@mdarade SPECS]$ rpmlint -i liblognorm.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
[mdarade@mdarade SPECS]$ rpmlint -i ../SRPMS/liblognorm-0.3.4-1.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[mdarade@mdarade SPECS]$ rpmlint -i ../RPMS/x86_64/liblognorm-
liblognorm-0.3.4-1.x86_64.rpm            liblognorm-devel-0.3.4-1.x86_64.rpm      
liblognorm-debuginfo-0.3.4-1.x86_64.rpm  liblognorm-utils-0.3.4-1.x86_64.rpm      
[mdarade@mdarade SPECS]$ rpmlint -i ../RPMS/x86_64/liblognorm-*
liblognorm-devel.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

liblognorm-utils.x86_64: W: spelling-error Summary(en_US) Normalizer -> Normalize, Normalizes, Normalized
The value of this tag appears to be misspelled. Please double-check.

liblognorm-utils.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

liblognorm-utils.x86_64: W: no-manual-page-for-binary normalizer
Each executable in standard binary directories should have a man page.

4 packages and 0 specfiles checked; 0 errors, 4 warnings.
[mdarade@mdarade SPECS]$

Comment 1 Persona non grata 2012-08-27 08:42:09 UTC
Hi, this is just an informational review.

1. liblognorm requires libestr, it should be in BuildRequires
2. %clean tag is not necessary
3. use INSTALL='install -p'

Regards,
Milan

Comment 2 mahaveer darade 2012-08-28 09:04:38 UTC
Done. All the changes suggested in comment#1 are in-place & below are new rpmlint logs. 



[root@mdarade guest]# rpmlint -i liblognorm.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
[root@mdarade guest]# rpmlint -i liblognorm-0.3.4-2.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[root@mdarade guest]# rpmlint -i liblognorm*x86_64*.rpm
liblognorm-devel.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

liblognorm-utils.x86_64: W: spelling-error Summary(en_US) Normalizer -> Normalize, Normalizes, Normalized
The value of this tag appears to be misspelled. Please double-check.

liblognorm-utils.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

liblognorm-utils.x86_64: W: no-manual-page-for-binary normalizer
Each executable in standard binary directories should have a man page.

4 packages and 0 specfiles checked; 0 errors, 4 warnings.
[root@mdarade guest]#

Comment 3 mahaveer darade 2012-08-28 11:29:23 UTC
Forgot to mention updated link to SRPM so here it is:

http://theinric.fedorapeople.org/liblognorm/liblognorm-0.3.4-2.src.rpm

Comment 4 Michael Schwendt 2012-09-15 13:23:49 UTC
> Release:	2

Not using %dist during review is okay, but be aware of future trouble when building the package for multiple dists.


> Summary:	Tool to normalize log data

The library is not really "a tool". The pkgconfig file contains a good description that could be used for a better %summary:

    Description: fast samples-based log normalization library


* The pkgconfig file as is won't work flawlessly. It contains

    Libs: -L${libdir} -llognorm -lee -lestr

which means a "Requires" dependency is missing in the file, or else one could not link successfully. However, explicitly relinking with shared libee and libestr should not be necessary when linking with liblognorm, so these two linker options should be dropped.

Some of liblognorm's header files access libee/libestr headers, so clearly a "Requires: libee-devel libestr-devel" is needed in the liblognorm-devel package. Note that adding a dependency to the .pc file would result in automatic RPM dependencies.



> %package devel
> Requires:	%{name} = %{version}-%{release}

> %package utils
> Requires:	%{name} = %{version}-%{release}

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package


> %description utils
> The normalizer is the core of liblognorm, it's utility for normalizing
> log files.

s/it's/its/ ?


> %build
> make

"V=1 make" for more verbose output (compiler and linker options) in build log would be nice. Plus (albeit this is just a small source project): 
https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make


> %install
> make install -p DESTDIR=%{buildroot}

Option -p is very likely a mistake here as it is passed to "make" and is short for "--print-data-base". Assumably, what you actually want is:

  make install INSTALL="install -p"  DESTDIR=%{buildroot}


> $ rpmls -p liblognorm-utils-0.3.4-2.t1.x86_64.rpm 
> -rwxr-xr-x  /usr/bin/normalizer

A very generic name, not in a special namespace, and almost caused a conflict already:

  # repoquery --whatprovides /usr/bin/normalize
  normalize-0:0.7.7-6.fc17.x86_64

Upstream is highly encouraged to try to avoid using such generic names.

Comment 5 Persona non grata 2012-09-19 07:44:44 UTC
Normalizer has been renamed in liblognorm 0.3.5 to lognormalizer to be more poignant. (http://www.liblognorm.com/tag/0-3-5/)

Comment 6 mahaveer darade 2012-09-21 02:20:23 UTC
I've worked on all the comments provided by Michael and here is a link to updated package. 

http://siddharths.fedorapeople.org/mahaveer/SPECS/liblognorm.spec
http://siddharths.fedorapeople.org/mahaveer/SRPMS/liblognorm-0.3.4-3.fc15.src.rpm

Comment 7 Tomas Mraz 2012-10-01 14:05:47 UTC
The %description for the main package should be at least slightly more descriptive. And the %description of the utils subpackage contains error 'its' is mistaken for 'it is'.

The same mistake as in libee and libestr is repeated here - the first sentence in COPYING contradicts with the contents of COPYING and the source files.

Comment 8 Michael Schwendt 2012-10-01 17:41:03 UTC
> And the %description of the utils subpackage contains error
> 'its' is mistaken for 'it is'.

See comment 4. "it is" or "it's" would not form a valid sentence, because an article such as "a" or "the" would be missing:

| The lognormalizer is the core of liblognorm, it's utility for
| normalizing log files.

So, either add the missing article or rephrase.

Comment 9 mahaveer darade 2012-10-05 14:34:32 UTC
I've worked on comments provided by Tomas & Michael. Below is a link to updated package.

http://mdarade.fedorapeople.org/SPECS/liblognorm.spec
http://mdarade.fedorapeople.org/SRPMS/liblognorm-0.3.4-4.fc15.src.rpm

Comment 10 Tomas Mraz 2012-10-05 15:26:56 UTC
Please do not change the COPYING file, it needs to be corrected upstream. You can correct this mistake before you import the package into Fedora git.

rpmlint -v liblognorm-0.3.4-4.fc16.src.rpm liblognorm-0.3.4-4.fc16.x86_64.rpm liblognorm-devel-0.3.4-4.fc16.x86_64.rpm liblognorm-utils-0.3.4-4.fc16.x86_64.rpm liblognorm-debuginfo-0.3.4-4.fc16.x86_64.rpm
liblognorm.src: I: checking
liblognorm.src: I: checking-url http://www.liblognorm.com (timeout 10 seconds)
liblognorm.src: I: checking-url http://www.liblognorm.com/files/download/liblognorm-0.3.4.tar.gz (timeout 10 seconds)
liblognorm.x86_64: I: checking
liblognorm.x86_64: I: checking-url http://www.liblognorm.com (timeout 10 seconds)
liblognorm-devel.x86_64: I: checking
liblognorm-devel.x86_64: I: checking-url http://www.liblognorm.com (timeout 10 seconds)
liblognorm-devel.x86_64: W: no-documentation
liblognorm-utils.x86_64: I: checking
liblognorm-utils.x86_64: W: spelling-error Summary(en_US) Lognormalizer -> Normalize
liblognorm-utils.x86_64: I: checking-url http://www.liblognorm.com (timeout 10 seconds)
liblognorm-utils.x86_64: W: no-documentation
liblognorm-utils.x86_64: W: no-manual-page-for-binary lognormalizer
liblognorm-debuginfo.x86_64: I: checking
liblognorm-debuginfo.x86_64: I: checking-url http://www.liblognorm.com (timeout 10 seconds)
5 packages and 0 specfiles checked; 0 errors, 4 warnings.
The spelling error is not a real error.

Missing documentation and manual page is upstream issue.

Tarball matches the upstream sources.

The package complies with Fedora packaging and licensing guidelines.

The package is ACCEPTED.

Comment 11 mahaveer darade 2012-10-06 03:24:23 UTC
Thank you all for the review of all three packages.

Comment 12 Steven Dake 2012-10-06 04:09:29 UTC
reassigning to package review component since this is not pacemaker-cloud component and pacemaker-cloud has been dropped from Fedora.

Comment 13 Tomas Mraz 2012-10-08 06:58:57 UTC
Reverting the accidental changes.

Comment 14 Mahaveer Darade 2012-10-08 13:06:04 UTC
New Package SCM Request
=======================
Package Name: liblognorm
Short Description: Fast samples-based log normalization library
Owners: mdarade
Branches: f17 f18
InitialCC: mbartos

Comment 15 Jason Tibbitts 2012-10-08 16:27:16 UTC
Git done (by process-git-requests).

Comment 16 Fedora Update System 2012-11-07 07:12:33 UTC
liblognorm-0.3.4-4.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/liblognorm-0.3.4-4.fc17

Comment 17 Fedora Update System 2012-11-08 02:01:22 UTC
liblognorm-0.3.4-4.fc17 has been pushed to the Fedora 17 testing repository.

Comment 18 Fedora Update System 2012-11-08 06:36:44 UTC
liblognorm-0.3.4-4.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/liblognorm-0.3.4-4.fc18

Comment 19 Fedora Update System 2012-11-15 02:38:34 UTC
liblognorm-0.3.4-4.fc17 has been pushed to the Fedora 17 stable repository.


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