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.
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.
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.
(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
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
koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=3297342
Do you need these? BuildRequires: autoconf BuildRequires: automake BuildRequires: libtool
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
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
I checked the upstream, the license was updated in the tree: $git log COPYING commit 4dcb87905923fd726a33338de8d92af5ac537714 Author: Pablo Neira Ayuso <pablo> 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> Signed-off-by: Pablo Neira Ayuso <pablo> 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?
You can do that but since upstream has already fixed this issue, I wouldn't insist on it. What about the examples?
The examples are not built by the default makefile, I think we cant install the built example to _bindir.
You can keep it as part of %doc
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
to add examples to docs, do you think just add %doc in files section, or install them in %install section? which is better?
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.
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
=== APPROVED ===
thanks Rahul for review!
New Package SCM Request ======================= Package Name: libmnl Short Description: A minimalistic Netlink library Owners: hushan Branches: f16 f15 f14 el6 el5 InitialCC:
Git done (by process-git-requests).
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
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
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
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
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
libmnl-1.0.1-4.fc16 has been pushed to the Fedora 16 testing repository.
libmnl-1.0.1-4.fc14 has been pushed to the Fedora 14 stable repository.
libmnl-1.0.1-4.fc15 has been pushed to the Fedora 15 stable repository.
libmnl-1.0.1-4.fc16 has been pushed to the Fedora 16 stable repository.
libmnl-1.0.1-4.el6 has been pushed to the Fedora EPEL 6 stable repository.
libmnl-1.0.1-5.el5 has been pushed to the Fedora EPEL 5 stable repository.