Bug 825494 - Review Request: cliquer - Find cliques in arbitrary weighted graphs
Summary: Review Request: cliquer - Find cliques in arbitrary weighted graphs
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jerry James
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-05-26 21:06 UTC by Paulo Andrade
Modified: 2012-12-08 04:26 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-08-18 01:57:50 UTC
Type: ---
Embargoed:
loganjerry: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Paulo Andrade 2012-05-26 21:06:03 UTC
Spec URL: http://fedorapeople.org/~pcpa/cliquer.spec
SRPM URL: http://fedorapeople.org/~pcpa/cliquer-1.21-2.fc18.src.rpm
Description: Cliquer is a set of C routines for finding cliques in an arbitrary
weighted graph.  It uses an exact branch-and-bound algorithm developed
by Patric Östergård. It is designed with the aim of being efficient
while still being flexible and easy to use.
Fedora Account System Username: pcpa

Comment 1 Paulo Andrade 2012-05-26 21:13:04 UTC
This package was not created by me, I only adapted it
from http://jjames.fedorapeople.org/cliquer/ adding a
few customizations for sagemath build.

I actually also have packaged it in Mandriva some
years ago, and this package is not optional for a
sagemath build.

The package is not very clean, and upstream requests
sending "real" mail for contact, but I am considering
to write a proper configure.ac, Makefile.am, etc if
requested in review.

Package home page http://users.tkk.fi/pat/cliquer.html

Comment 2 Paulo Andrade 2012-05-26 21:15:10 UTC
Adding Jerry to CC, as I am asking a review request of
a package he did most of the work.

Comment 3 Jerry James 2012-08-14 19:59:31 UTC
Sorry to be so slow with this, Paulo.  I'm finally getting a few Fedora tasks done now and then.

This package is in pretty good shape.  There are just a few issues shown below, namely:
- Need a comment about the patch (SHOULD item)
- Spec files mixes use of tabs and spaces (noncritical)
- Nonstandard permissions on the library and binary (have to fix this one)
- Incorrect FSF address in the LICENSE file (don't have to do anything)

Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== C/C++ ====
[x]: MUST Header files in -devel subpackage, if present.
[x]: MUST ldconfig called in %post and %postun if required.
[x]: MUST Package does not contain any libtool archives (.la)
[x]: MUST Package does not contain kernel modules.
[x]: MUST Package contains no static executables.
[x]: MUST Rpath absent or only used for internal libs.
[x]: MUST Development (unversioned) .so files in -devel subpackage, if
     present.


==== Generic ====
[x]: EXTRA Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[x]: 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 Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[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 Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files devel section. This is OK if
     packaging for EPEL5. Otherwise not needed
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[x]: MUST Package contains desktop file if it is a GUI application.
[x]: MUST Development files must be in a -devel package
[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.
[x]: MUST Fully versioned dependency in subpackages, if present.
[x]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: 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 macro is (instead of hard-coded directory
     names).
[x]: MUST Package is named using only allowed ascii characters.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[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 Package is not relocatable.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: MUST Package contains systemd file(s) if in need.
[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.
[-]: 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 Dist tag is present.
[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 Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[x]: SHOULD Scriptlets must be sane, if used.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Source6 (poly.c) Source7 (tetromino.h) Source4 (basic.c) Source5
     (hamming.c) Source9 (cl.h)
[x]: SHOULD SourceX is a working URL.
[x]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[x]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files devel section. This is OK if
     packaging for EPEL5. Otherwise not needed
See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
[!]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
See: http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

Rpmlint
-------
Checking: cliquer-devel-1.21-2.fc17.x86_64.rpm
          cliquer-debuginfo-1.21-2.fc17.x86_64.rpm
          cliquer-1.21-2.fc17.src.rpm
          cliquer-1.21-2.fc17.x86_64.rpm
          cliquer-libs-1.21-2.fc17.x86_64.rpm
cliquer-devel.x86_64: W: no-documentation
cliquer.src:20: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 20)
cliquer.x86_64: E: non-standard-executable-perm /usr/bin/cliquer 0775L
cliquer-libs.x86_64: E: non-standard-executable-perm /usr/lib64/libcliquer.so.1.21 0775L
cliquer-libs.x86_64: E: incorrect-fsf-address /usr/share/doc/cliquer-libs-1.21/LICENSE
5 packages and 0 specfiles checked; 3 errors, 2 warnings.


Rpmlint (installed packages)
----------------------------
# rpmlint cliquer-libs cliquer-debuginfo
cliquer-libs.x86_64: I: enchant-dictionary-not-found en_US
cliquer-libs.x86_64: E: non-standard-executable-perm /usr/lib64/libcliquer.so.1.21 0775L
cliquer-libs.x86_64: E: incorrect-fsf-address /usr/share/doc/cliquer-libs-1.21/LICENSE
2 packages and 0 specfiles checked; 2 errors, 0 warnings.
# echo 'rpmlint-done:'

Requires
--------
cliquer-devel-1.21-2.fc17.x86_64.rpm (rpmlib, GLIBC filtered):

    cliquer-libs(x86-64) = 1.21-2.fc17
    libcliquer.so.1()(64bit)

cliquer-debuginfo-1.21-2.fc17.x86_64.rpm (rpmlib, GLIBC filtered):


cliquer-1.21-2.fc17.x86_64.rpm (rpmlib, GLIBC filtered):

    cliquer-libs(x86-64) = 1.21-2.fc17
    libc.so.6()(64bit)
    libcliquer.so.1()(64bit)
    rtld(GNU_HASH)

cliquer-libs-1.21-2.fc17.x86_64.rpm (rpmlib, GLIBC filtered):

    /sbin/ldconfig
    libc.so.6()(64bit)
    rtld(GNU_HASH)

Provides
--------
cliquer-devel-1.21-2.fc17.x86_64.rpm:

    cliquer-devel = 1.21-2.fc17
    cliquer-devel(x86-64) = 1.21-2.fc17

cliquer-debuginfo-1.21-2.fc17.x86_64.rpm:

    cliquer-debuginfo = 1.21-2.fc17
    cliquer-debuginfo(x86-64) = 1.21-2.fc17

cliquer-1.21-2.fc17.x86_64.rpm:

    cliquer = 1.21-2.fc17
    cliquer(x86-64) = 1.21-2.fc17

cliquer-libs-1.21-2.fc17.x86_64.rpm:

    cliquer-libs = 1.21-2.fc17
    cliquer-libs(x86-64) = 1.21-2.fc17
    libcliquer.so.1()(64bit)

MD5-sum check
-------------
http://users.tkk.fi/~pat/cliquer/cliquer.pdf :
  MD5SUM this package     : 92d68e23b023f5abf16da3e5b0636c1b
  MD5SUM upstream package : 92d68e23b023f5abf16da3e5b0636c1b
http://users.tkk.fi/~pat/cliquer/cliquer_bm.pdf :
  MD5SUM this package     : a8b8aa86460da3f68b9b403bb09bf0e0
  MD5SUM upstream package : a8b8aa86460da3f68b9b403bb09bf0e0
http://users.tkk.fi/~pat/cliquer/cliquer-1.21.tar.gz :
  MD5SUM this package     : ac65de2c89134abe2be36542b9465b16
  MD5SUM upstream package : ac65de2c89134abe2be36542b9465b16
http://users.tkk.fi/~pat/cliquer/cliquer_fm.pdf :
  MD5SUM this package     : 7eb3790632ef86c7363b684e24e8b21c
  MD5SUM upstream package : 7eb3790632ef86c7363b684e24e8b21c
http://users.tkk.fi/~pat/cliquer/poly.c :
  MD5SUM this package     : 288aa9d3baa4c3d4419f5f3c7d0a7206
  MD5SUM upstream package : 288aa9d3baa4c3d4419f5f3c7d0a7206
http://users.tkk.fi/~pat/cliquer/tetromino.h :
  MD5SUM this package     : ff5735fb610f37997e46f62a521198d2
  MD5SUM upstream package : ff5735fb610f37997e46f62a521198d2
http://users.tkk.fi/~pat/cliquer/basic.c :
  MD5SUM this package     : 09e0c1a7c254c4ec5eac91074a3eb3e2
  MD5SUM upstream package : 09e0c1a7c254c4ec5eac91074a3eb3e2
http://users.tkk.fi/~pat/cliquer/hamming.c :
  MD5SUM this package     : 628ba73a87eba1ea50e85f451f7f4d22
  MD5SUM upstream package : 628ba73a87eba1ea50e85f451f7f4d22


Generated by fedora-review 0.2.0 (53cc903) last change: 2012-07-09
Command line :/usr/bin/fedora-review -b 825494
External plugins:

Comment 4 Paulo Andrade 2012-08-15 00:05:11 UTC
Many thanks for the review. I sent an email to upstream
pointing to sagemath specific tracs when it was imported
to sagemath, as well as this review request, and corrected
the other issues.

New package:
Spec URL: http://fedorapeople.org/~pcpa/cliquer.spec
SRPM URL: http://fedorapeople.org/~pcpa/cliquer-1.21-3.fc19.src.rpm

Comment 5 Jerry James 2012-08-15 16:15:16 UTC
Everything is fixed except the nonstandard executable permissions.  Also, there are some explicit uses of the version number.  I suggest fixing both like this:

--- cliquer.spec.orig	2012-08-15 09:12:35.402848523 -0600
+++ cliquer.spec	2012-08-15 10:13:20.123509731 -0600
@@ -72,9 +72,9 @@
 gcc ${RPM_OPT_FLAGS} -fPIC -c cliquer.c
 gcc ${RPM_OPT_FLAGS} -fPIC -c graph.c
 gcc ${RPM_OPT_FLAGS} -fPIC -c reorder.c
-gcc ${RPM_OPT_FLAGS} -fPIC -shared -o libcliquer.so.1.21 \
+gcc ${RPM_OPT_FLAGS} -fPIC -shared -o libcliquer.so.%{version} \
   -Wl,-soname=libcliquer.so.1 cl.o cliquer.o graph.o reorder.o
-ln -s libcliquer.so.1.21 libcliquer.so.1
+ln -s libcliquer.so.%{version} libcliquer.so.1
 ln -s libcliquer.so.1 libcliquer.so
 
 # Now build the binary
@@ -85,10 +85,11 @@
 # Install the library
 mkdir -p $RPM_BUILD_ROOT%{_libdir}
 cp -pP libcliquer.so* $RPM_BUILD_ROOT%{_libdir}
+chmod 0755 $RPM_BUILD_ROOT%{_libdir}/libcliquer.so.%{version}
 
 # Install the binary
 mkdir -p $RPM_BUILD_ROOT%{_bindir}
-cp -p cliquer $RPM_BUILD_ROOT%{_bindir}/cliquer
+install -m 0755 -p cliquer $RPM_BUILD_ROOT%{_bindir}/cliquer
 
 # Install the header file
 mkdir -p $RPM_BUILD_ROOT%{_includedir}/cliquer

Comment 6 Paulo Andrade 2012-08-15 23:49:15 UTC
Thanks again. I added the suggested patch to the new package.
Yesterday I thought it could have been some glitch the issue
with permissions, as rpmlint was not saying anything about it,
but I usually test builds with plain rpmbuild, and when
running fedora-review myself did show the problem.

I think I have a slightly newer fedora-review, that also did
show what I believe can be ignored:

[!]: MUST Header files in -devel subpackage, if present.
     Note: cliquer-libs-1.21-3.fc16.x86_64.rpm : /usr/share/doc/cliquer-
     libs-1.21/example/tetromino.h

it is an example in documentation.


[!]: MUST License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. No licenses
     found. Please check the source files for licenses manually.

not all source files have a license, and none has a full
standard license at start, just something like

 * Licensed under the GNU GPL, read the file LICENSE for details.

but the README file should make it clear the license of the
package.


New package:
Spec URL: http://fedorapeople.org/~pcpa/cliquer.spec
SRPM URL: http://fedorapeople.org/~pcpa/cliquer-1.21-4.fc19.src.rpm

Comment 7 Jerry James 2012-08-16 15:21:30 UTC
Actually, my fedora-review showed the header file issue also, but I removed the warning since it is bogus.  That check should skip documentation files.

Everything looks great.  This package is APPROVED.

Comment 8 Paulo Andrade 2012-08-16 20:39:50 UTC
New Package SCM Request
=======================
Package Name: cliquer
Short Description: Find cliques in arbitrary weighted graphs
Owners: pcpa
Branches: 
InitialCC: pcpa

Comment 9 Gwyn Ciesla 2012-08-16 21:16:35 UTC
Git done (by process-git-requests).

Comment 10 Paulo Andrade 2012-08-18 01:57:50 UTC
Package has been built for rawhide.

Comment 11 Paulo Andrade 2012-11-28 23:32:37 UTC
Need cliquer built in rawhide to later have a (possible, hopefully) clean sagemath update package.

Comment 12 Paulo Andrade 2012-11-28 23:33:08 UTC
Package Change Request
======================
Package Name: cliquer
New Branches: f18
Owners: pcpa

Comment 13 Gwyn Ciesla 2012-11-29 12:01:59 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2012-11-29 15:55:27 UTC
cliquer-1.21-4.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/cliquer-1.21-4.fc18

Comment 15 Fedora Update System 2012-12-08 04:26:11 UTC
cliquer-1.21-4.fc18 has been pushed to the Fedora 18 stable repository.


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