Bug 732159 - Review Request: libmnl - A minimalistic Netlink library
Summary: Review Request: libmnl - A minimalistic Netlink library
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rahul Sundaram
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-08-20 04:33 UTC by Hushan Jia
Modified: 2011-09-09 00:04 UTC (History)
5 users (show)

Fixed In Version: libmnl-1.0.1-5.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-09-07 00:13:01 UTC
metherid: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Hushan Jia 2011-08-20 04:33:09 UTC
Spec URL: http://hushan.fedorapeople.org/pkgs/libmnl.spec
SRPM URL: http://hushan.fedorapeople.org/pkgs/libmnl-1.0.1-1.el6.src.rpm
Description: libmnl is a minimalistic user-space library oriented to Netlink developers.
There are a lot of common tasks in parsing, validating, constructing of both
the Netlink header and TLVs that are repetitive and easy to get wrong.
This library aims to provide simple helpers that allows you to re-use code and
to avoid re-inventing the wheel.

Comment 1 Rahul Sundaram 2011-08-20 05:56:37 UTC
Are you just packaging a snapshot or have you talked to upstream and asked whether this is stable and whether they are planning on doing a release anytime soon?

You need to define buildroot or defattr or have a clean stage or clean the buildroot in install stage anymore

Instead of specifying wildcards for everything in %files, you should list the files more explicitly.

Comment 2 Hushan Jia 2011-08-20 07:05:36 UTC
thanks Rahul, fixed, use upstream released tarball instead of source snapshot, can you help review this package?

Spec URL:
http://hushan.fedorapeople.org/pkgs/libmnl.spec
SRPM URL:
http://hushan.fedorapeople.org/pkgs/libmnl-1.0.1-2.el6.src.rpm

the buildroot is cleaned by rm -rf $RPM_BUILD_ROOT in install stage, also about wildcards in files section, is the packaging guide changed? This is OK before, and many packages use this.

Comment 3 Mario Blättermann 2011-08-23 20:24:50 UTC
(In reply to comment #2)
> the buildroot is cleaned by rm -rf $RPM_BUILD_ROOT in install stage, also about
> wildcards in files section, is the packaging guide changed? This is OK before,
> and many packages use this.

It is only needed for older EPEL packages. You might drop the BuildRoot line [1], the %clean section [2] and the %defattr macro [3] in %files, if you don't want to provide your package for EPEL 5 or older.

[1] http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
[2] http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean
[3] http://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

Comment 4 Hushan Jia 2011-08-24 01:49:40 UTC
Fixed buildroot, clean and defattr tags:

SPEC URL:
http://hushan.fedorapeople.org/pkgs/libmnl.spec
SRPM URL:
http://hushan.fedorapeople.org/pkgs/libmnl-1.0.1-3.el6.src.rpm

Comment 5 Hushan Jia 2011-08-24 01:55:28 UTC
koji scratch build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=3297342

Comment 6 Rahul Sundaram 2011-08-24 01:56:21 UTC
Do you need these?

BuildRequires:  autoconf
BuildRequires:  automake
BuildRequires:  libtool

Comment 7 Hushan Jia 2011-08-24 02:06:23 UTC
The buildrequires are not used, fixed spec:
http://hushan.fedorapeople.org/pkgs/libmnl.spec

scratch test build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=3297345

Comment 8 Rahul Sundaram 2011-08-24 04:34:25 UTC
Package Review
==============

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

[x] : MUST - Package successfully compiles and builds into binary rpms on at least one supported architecture.
[x] : MUST - Fully versioned dependency in subpackages, if present.
[x] : MUST - Header files in -devel subpackage, if present.
[x] : MUST - Spec file lacks Packager, Vendor, PreReq tags.
[x] : MUST - ldconfig called in %post and %postun if required.
[x] : MUST - Package does not contain any libtool archives (.la)
[x] : MUST - Package use %makeinstall only when make install DESTDIR=... doesn't work.
[x] : MUST - Package is named according to the Package Naming Guidelines.
[x] : MUST - Development .so files in -devel subpackage, if present.
[x] : MUST - Sources used to build the package matches the upstream source, as provided in the spec URL.
        /home/rahul/tmp/reviewhelper/732159/libmnl-1.0.1.tar.bz2 :
          MD5SUM this package     : e936236bb57a2375afa4e70e75dc3ba9
          MD5SUM upstream package : e936236bb57a2375afa4e70e75dc3ba9
        
[x] : MUST - Spec file name must match the spec package %{name}, in the format %{name}.spec.
[-] : MUST - %config files are marked noreplace or the reason is justified.
[-] : MUST - Package contains a properly installed %{name}.desktop using desktop-file-install file if it is a GUI application.
[-] : MUST - The spec file handles locales properly.
[-] : MUST - No %config files under /usr.
[-] : MUST - Static libraries in -static subpackage, if present.
[!] : MUST - Rpmlint output is silent.
        
        rpmlint libmnl-debuginfo-1.0.1-3.fc17.i686.rpm
        ================================================================================
        1 packages and 0 specfiles checked; 0 errors, 0 warnings.
        ================================================================================
        
        rpmlint libmnl-devel-1.0.1-3.fc17.i686.rpm
        ================================================================================
        libmnl-devel.i686: E: incorrect-fsf-address /usr/share/doc/libmnl-devel-1.0.1/COPYING
        1 packages and 0 specfiles checked; 1 errors, 0 warnings.
        ================================================================================
        
        rpmlint libmnl-1.0.1-3.fc17.src.rpm
        ================================================================================
        libmnl.src: W: spelling-error Summary(en_US) minimalistic -> minimalist, minimalism, animistic
        libmnl.src: W: spelling-error %description -l en_US minimalistic -> minimalist, minimalism, animistic
        1 packages and 0 specfiles checked; 0 errors, 2 warnings.
        ================================================================================
        
        rpmlint libmnl-1.0.1-3.fc17.i686.rpm
        ================================================================================
        libmnl.i686: W: spelling-error Summary(en_US) minimalistic -> minimalist, minimalism, animistic
        libmnl.i686: W: spelling-error %description -l en_US minimalistic -> minimalist, minimalism, animistic
        libmnl.i686: E: incorrect-fsf-address /usr/share/doc/libmnl-1.0.1/COPYING
        1 packages and 0 specfiles checked; 1 errors, 2 warnings.
        ================================================================================
        
[ ] : MUST - Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
[X] : MUST - %build honors applicable compiler flags or justifies otherwise.
[X] : MUST - All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
[X] : MUST - Package contains no bundled libraries.
[X] : MUST - Changelog in prescribed format.
[X] : MUST - Sources contain only permissible code or content.
[X] : MUST - Macros in Summary, %description expandable at SRPM build time.
[X] : MUST - Package requires other packages for directories it uses.
[X] : MUST - Package uses nothing in %doc for runtime.
[X] : MUST - Package is not known to require ExcludeArch.
[X] : MUST - Permissions on files are set properly.
[X ] : MUST - Package does not contain duplicates in %files.
[-] : MUST - Large documentation files are in a -doc subpackage, if required.
[X] : MUST - If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
[X] : MUST - License field in the package spec file matches the actual license.
[X ] : MUST - License file installed when any subpackage combination is installed.
[X] : MUST - Package consistently uses macros. instead of hard-coded directory names.
[X] : MUST - Package meets the Packaging Guidelines.
[X] : MUST - Package does not generates any conflict.
[-] : MUST - Package does not contains kernel modules.
[X] : MUST - Package contains no static executables.
[X] : MUST - Package obeys FHS, except libexecdir and /usr/target.
[X] : MUST - Package must own all directories that it creates.
[X] : MUST - Package does not own files or directories owned by other packages.
[X] : MUST - Package installs properly.
[X] : MUST - Rpath absent or only used for internal libs.
[X] : MUST - Package is not relocatable.
[X] : MUST - Requires correct, justified where necessary.
[X] : MUST - Spec file is legible and written in American English.
[-] : MUST - Package contains a SysV-style init script if in need of one.
[X] : MUST - File names are valid UTF-8.
[X] : MUST - Useful -debuginfo package or justification otherwise.
[x] : SHOULD - Reviewer should test that the package builds in mock.
[x] : SHOULD - Dist tag is present.
[x] : SHOULD - The placement of pkgconfig(.pc) files are correct.
[x] : SHOULD - SourceX / PatchY prefixed with %{name}.
[x] : SHOULD - SourceX is a working URL.
[x] : SHOULD - Spec use %global instead of %define.
[-] : SHOULD - Uses parallel make.
[-] : SHOULD - If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
[X] : SHOULD - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[X] : SHOULD - Final provides and requires are sane (rpm -q --provides and rpm -q --requires).
[X] : SHOULD - Package functions as described.
[X] : SHOULD - Latest version is packaged.
[X] : SHOULD - Package does not include license text files separate from upstream.
[-] : SHOULD - Man pages included for all executables.
[-] : SHOULD - Patches link to upstream bugs/comments/lists or are otherwise justified.
[X] : SHOULD - Scriptlets must be sane, if used.
[-] : SHOULD - Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
[-] : SHOULD - Package should compile and build into binary rpms on all supported architectures.
[X ] : SHOULD - %check is present and all tests pass.
[-] : SHOULD - Packages should try to preserve timestamps of original installed files.

Issues:
[!] : MUST - Rpmlint output is silent.
        
        rpmlint libmnl-debuginfo-1.0.1-3.fc17.i686.rpm
        ================================================================================
        1 packages and 0 specfiles checked; 0 errors, 0 warnings.
        ================================================================================
        
        rpmlint libmnl-devel-1.0.1-3.fc17.i686.rpm
        ================================================================================
        libmnl-devel.i686: E: incorrect-fsf-address /usr/share/doc/libmnl-devel-1.0.1/COPYING
        1 packages and 0 specfiles checked; 1 errors, 0 warnings.
        ================================================================================
        
        rpmlint libmnl-1.0.1-3.fc17.src.rpm
        ================================================================================
        libmnl.src: W: spelling-error Summary(en_US) minimalistic -> minimalist, minimalism, animistic
        libmnl.src: W: spelling-error %description -l en_US minimalistic -> minimalist, minimalism, animistic
        1 packages and 0 specfiles checked; 0 errors, 2 warnings.
        ================================================================================
        
        rpmlint libmnl-1.0.1-3.fc17.i686.rpm
        ================================================================================
        libmnl.i686: W: spelling-error Summary(en_US) minimalistic -> minimalist, minimalism, animistic
        libmnl.i686: W: spelling-error %description -l en_US minimalistic -> minimalist, minimalism, animistic
        libmnl.i686: E: incorrect-fsf-address /usr/share/doc/libmnl-1.0.1/COPYING
        1 packages and 0 specfiles checked; 1 errors, 2 warnings.
        ================================================================================
        

Should you not be compiling the examples that are part of the source?  let upstream know the the FSF address is incorrect

Comment 9 Hushan Jia 2011-08-24 04:59:21 UTC
I checked the upstream, the license was updated in the tree:

$git log COPYING 
commit 4dcb87905923fd726a33338de8d92af5ac537714
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date:   Thu Jun 2 15:19:03 2011 +0200

    COPYING: update file (FSF address was outdated)
    
    Prabin reported that the FSF address was outdated, I downloaded
    the current version of LGPL 2.1 from the website and put it in
    the tree.
    
    Reported-by: Prabin Kumar Datta <prabindatta@fedoraproject.org>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

the 1.0.1 tarball was released before this date, so should I add the right COPYING file to the srpm and install it when installing?

Comment 10 Rahul Sundaram 2011-08-24 05:58:45 UTC
You can do that but since upstream has already fixed this issue, I wouldn't insist on it.   What about the examples?

Comment 11 Hushan Jia 2011-08-24 06:27:43 UTC
The examples are not built by the default makefile, I think we cant install the built example to _bindir.

Comment 12 Rahul Sundaram 2011-08-24 06:37:13 UTC
You can keep it as part of %doc

Comment 13 Martin Gieseking 2011-08-24 06:38:49 UTC
A quick note: According to the guidelines, you should make the Requires of the base package arch specific:
https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

Comment 14 Hushan Jia 2011-08-24 08:04:32 UTC
to add examples to docs, do you think just add %doc in files section, or install them in %install section? which is better?

Comment 15 Rahul Sundaram 2011-08-24 08:14:04 UTC
I prefer just using the %doc macro but either should work fine.  Fix this and the issue pointed out by Martin and I will approve you.

Comment 16 Hushan Jia 2011-08-24 09:39:23 UTC
fixed -devel requires, and add example source files to include to docs:

SPEC URL:
http://hushan.fedorapeople.org/pkgs/libmnl.spec
SRPM URL:
http://hushan.fedorapeople.org/pkgs/libmnl-1.0.1-4.el6.src.rpm

Comment 17 Rahul Sundaram 2011-08-24 09:49:20 UTC
===  APPROVED ===

Comment 18 Hushan Jia 2011-08-24 09:50:41 UTC
thanks Rahul for review!

Comment 19 Hushan Jia 2011-08-24 09:56:40 UTC
New Package SCM Request
=======================
Package Name: libmnl
Short Description: A minimalistic Netlink library
Owners: hushan
Branches: f16 f15 f14 el6 el5
InitialCC:

Comment 20 Gwyn Ciesla 2011-08-24 12:05:13 UTC
Git done (by process-git-requests).

Comment 21 Fedora Update System 2011-08-24 15:22:42 UTC
libmnl-1.0.1-4.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/libmnl-1.0.1-4.fc16

Comment 22 Fedora Update System 2011-08-24 15:34:25 UTC
libmnl-1.0.1-4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/libmnl-1.0.1-4.fc15

Comment 23 Fedora Update System 2011-08-24 15:45:10 UTC
libmnl-1.0.1-4.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/libmnl-1.0.1-4.fc14

Comment 24 Fedora Update System 2011-08-24 16:24:38 UTC
libmnl-1.0.1-4.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/libmnl-1.0.1-4.el6

Comment 25 Fedora Update System 2011-08-24 17:13:16 UTC
libmnl-1.0.1-5.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/libmnl-1.0.1-5.el5

Comment 26 Fedora Update System 2011-08-24 22:47:08 UTC
libmnl-1.0.1-4.fc16 has been pushed to the Fedora 16 testing repository.

Comment 27 Fedora Update System 2011-09-07 00:12:55 UTC
libmnl-1.0.1-4.fc14 has been pushed to the Fedora 14 stable repository.

Comment 28 Fedora Update System 2011-09-07 00:14:31 UTC
libmnl-1.0.1-4.fc15 has been pushed to the Fedora 15 stable repository.

Comment 29 Fedora Update System 2011-09-07 03:25:54 UTC
libmnl-1.0.1-4.fc16 has been pushed to the Fedora 16 stable repository.

Comment 30 Fedora Update System 2011-09-08 23:58:49 UTC
libmnl-1.0.1-4.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 31 Fedora Update System 2011-09-09 00:04:03 UTC
libmnl-1.0.1-5.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.