Bug 672440

Ok, I'm taking this review. Before even looking, can you patch out the call to exit meaningful before upstream reacts?

I emailed the upstream developer about the LICENSE text and the exit(0) problems.  He replied very quickly, and said that he has removed the call to exit(0) in SVN for the next release.  It doesn't look like he's using a public SVN, but I should be able to dig in and fix things later tonight.  He also said he would include a BSD LICENSE text in his next release, in case you stumble upon the fact that it's missing while you're reviewing.

Alright, exit(1) has been fixed.  Flann includes its own implementation of malloc(), and it was calling exit(1) when it couldn't allocate enough memory.  I patched it to return NULL instead.  Once the next version is released, I will be able to drop the patch.

Spec URL: http://rmattes.fedorapeople.org/RPMS/flann/flann.spec
SRPM URL: http://rmattes.fedorapeople.org/RPMS/flann/flann-1.6.7-2.fc14.src.rpm

$ rpmlint flann.spec ../RPMS/x86_64/flann*flann-devel.x86_64: W: no-documentation
flann-python.x86_64: W: no-documentation
flann-static.x86_64: W: no-documentation
5 packages and 1 specfiles checked; 0 errors, 3 warnings.

scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2753897

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

(+) rpmlint is not silent, some messages can be ignored:
  - No documentation for sub-packages ok
+ The package is named according to the Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format
%{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package is licensed with a Fedora approved license and meets the
Licensing Guidelines.
+ The License field in the package spec file matches the actual license (BSD).
(-) The file, containing the text of the license(s) for the package, is included
in %doc.
  - issue is known and packager has contacted upstream about a fix. A license file will be included in the next release, therefore acceptable. Source code files carry the license tag, so license can be verified.

+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided
in the spec URL.

# sha256sum ../SOURCES/flann-1.6.7-src.zip 
c96feb000e7ce430bec4a03fb53ce0fb82c0bda9c475d93691916101a0c6c137  ../SOURCES/flann-1.6.7-src.zip
# sha256sum ~/Downloads/flann-1.6.7-src.zip 
c96feb000e7ce430bec4a03fb53ce0fb82c0bda9c475d93691916101a0c6c137  /home/tim/Downloads/flann-1.6.7-src.zip

+ The package successfully compiles and builds into binary rpms on at least one
primary architecture.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
+ Main package calls ldconfig in %post/%postun
+ The package does NOT bundle copies of system libraries.
+ The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files
listings.
+ Permissions on files are set properly.
- The package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
(-) The package consistently uses macros.
  - The package uses $RPM_BUILD_ROOT as variable, and everything else as macro. Since this is what rpmdev-newspec provides by default, I figure this is acceptable. Consider changing for consistency, but if you don't it is not a blocker (I'm using that format in many packages myself).
+ The package contains code, or permissible content.
+ No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the
application.
+ Header files are in -devel package
+ Static libraries are in -static package
+ pkg-config files are in -devel package
+ .so (no suffix) are in -devel package
+ -devel package requires base package, -static package requires -devel package
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other
packages.
- At the beginning of %install, the package runs rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Package looks good, things that MUST be changed:
- Add %clean section with rm -rf $RPM_BUILD_ROOT
- Add rm -rf $RPM_BUILD_ROOT at beginning of %install section
- Add license file in a future release (current state acceptable transiently and does not stop the package from being accepted)

Alright added the %clean section, and the rm -rf %{buildroot} at the beginning of install.  I also switched all of the $RPM_BUILD_ROOT variables to %{buildroot}; I think it makes the specfile look a lot cleaner.

The licensing guidelines mention that the license text is required if (and only if) the text ships with the source, but as you note, the issue has been resolved with upstream and should be a non-issue for 1.6.8

Spec URL: http://rmattes.fedorapeople.org/RPMS/flann/flann.spec
SRPM URL: http://rmattes.fedorapeople.org/RPMS/flann/flann-1.6.7-3.fc14.src.rpm

$ rpmlint ../RPMS/x86_64/flann* flann.spec
flann-devel.x86_64: W: no-documentation
flann-python.x86_64: W: no-documentation
flann-static.x86_64: W: no-documentation

Looks good now.

APPROVED.

New Package SCM Request
=======================
Package Name: flann
Short Description: Fast Library for Approximate Nearest Neighbors
Owners: rmattes
Branches: f13 f14
InitialCC:

Would you mind maintaining it in el5/el6? Maybe we can have OpenRAVE there, then. Some teams use CentOS or the like as a long-term stable robot platform.

Sure, I can maintain them there too.  I just ran some mockbuilds, hdf5 and gtest aren't available in EPEL.  el6 is ok without them, but I have to track down a build error for el5.

New Package SCM Request
=======================
Package Name: flann
Short Description: Fast Library for Approximate Nearest Neighbors
Owners: rmattes
Branches: f13 f14 el5 el6
InitialCC:

Git done (by process-git-requests).

flann-1.6.7-4.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/flann-1.6.7-4.el5

flann-1.6.7-4.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/flann-1.6.7-4.fc14

flann-1.6.7-4.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/flann-1.6.7-4.el6

flann-1.6.7-4.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/flann-1.6.7-4.fc13

flann-1.6.7-4.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update flann'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/flann-1.6.7-4.el5

flann-1.6.7-4.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

flann-1.6.7-4.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

/usr/include/flann/flann_mpi.hpp both require some mpi implementation and boost.
1) Second could (but not need, see 2) be fixed by adding Requires: boost.
2) There is no mpi support in current fedora flann at all.

May be better to not provide flann_mpi.hpp?

flann-1.6.7-4.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.

flann-1.6.7-4.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.