https://bugzilla.redhat.com/show_bug.cgi?id=825494
Bug ID: 825494 QA Contact: extras-qa@fedoraproject.org Severity: medium Version: rawhide Priority: medium CC: notting@redhat.com, package-review@lists.fedoraproject.org Assignee: nobody@fedoraproject.org Summary: Review Request: cliquer - Find cliques in arbitrary weighted graphs Regression: --- Story Points: --- Classification: Fedora OS: Linux Reporter: paulo.cesar.pereira.de.andrade@gmail.com Type: --- Documentation: --- Hardware: All Mount Type: --- Status: NEW Component: Package Review Product: Fedora
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
https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #1 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=825494
pcpa paulo.cesar.pereira.de.andrade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |loganjerry@gmail.com
--- Comment #2 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- Adding Jerry to CC, as I am asking a review request of a package he did most of the work.
https://bugzilla.redhat.com/show_bug.cgi?id=825494
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |loganjerry@gmail.com Flags| |fedora-review?
--- Comment #3 from Jerry James loganjerry@gmail.com --- 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_a...
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:
https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #4 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #5 from Jerry James loganjerry@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #6 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=825494
Jerry James loganjerry@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #7 from Jerry James loganjerry@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=825494
pcpa paulo.cesar.pereira.de.andrade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #8 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- New Package SCM Request ======================= Package Name: cliquer Short Description: Find cliques in arbitrary weighted graphs Owners: pcpa Branches: InitialCC: pcpa
https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #9 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=825494
pcpa paulo.cesar.pereira.de.andrade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |RAWHIDE Last Closed| |2012-08-17 21:57:50
--- Comment #10 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- Package has been built for rawhide.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #11 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- Need cliquer built in rawhide to later have a (possible, hopefully) clean sagemath update package.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=825494
pcpa paulo.cesar.pereira.de.andrade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs+ | Flags| |fedora-cvs?
--- Comment #12 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- Package Change Request ====================== Package Name: cliquer New Branches: f18 Owners: pcpa
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #13 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- cliquer-1.21-4.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/cliquer-1.21-4.fc18
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=825494
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|RAWHIDE |CURRENTRELEASE
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=825494
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- cliquer-1.21-4.fc18 has been pushed to the Fedora 18 stable repository.
package-review@lists.fedoraproject.org