[Bug 672395] Review Request: eigen3 - A lightweight C++ template library for vector and matrix math

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=672395

--- Comment #6 from Tim Niemueller <tim at niemueller.de> 2011-04-17 19:41:12 EDT ---
REVIEW:

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

- rpmlint is not silent, some messages can be ignored:
eigen3.src: E: no-description-tag
eigen3.src:49: W: macro-in-comment %{_libdir}
eigen3.src:63: W: macro-in-comment %check
eigen3.src:64: W: macro-in-comment %{_target_platform}
eigen3.src: W: patch-not-applied Patch0: eigen3-fixdso.patch
eigen3-devel.noarch: E: zero-length
/usr/include/eigen3/Eigen/src/Sparse/SparseAssign.h
  - The description tag should be set even though the main package is not
installable. It can just be a copy of the -devel package
  - The macro warnings can be silenced by removing the %
  - The patch should be removed if not used
  - The zero size error should be discussed with upstream. It seems it is
included in the core headers, but it does not appear to be auto-generated.

+ 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
(GPLv2+ or LGPLv3+).
  - The website lists the given licenses, while the LICENSE.* files in the
package mention GPLv3+ and LGPLv3+.
+ The file, containing the text of the license(s) for the package, is included
in %doc.
+ 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.

package# sha256sum ../SOURCES/3.0.0.tar.bz2 
e60efc5b18331b2e6c23ac5a8180a13b987f0aeb6fc6dca316ae338fa0513931 
../SOURCES/3.0.0.tar.bz2
downloaded # sha256sum ~/Downloads/eigen-eigen-65ee2328342f.tar.bz2 
e60efc5b18331b2e6c23ac5a8180a13b987f0aeb6fc6dca316ae338fa0513931 
/home/tim/Downloads/eigen-eigen-65ee2328342f.tar.bz2

I strongly recommend renaming the source file to contain the project name!

+ The package successfully compiles and builds into binary rpms on at least one
primary architecture.
  - No problems on F-14
  - On F-15 I needed to add "#include <cstddef>" to
Eigen/src/StlSupport/details.h to define ptrdiff_t.

+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files.
+ 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.
  - You use both, $RPM_BUILD_ROOT and %{...} style macros. Since rpmdev-newspec
creates spec files this way by default I deem this is acceptable, but you might
consider deciding for one consistent style.

+ 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 -devel subpackage
0 No static libraries.
+ pkg-config files are in -devel subpackage
  - If you intent to build the package for EPEL 5 the -devel package must
depend on pkg-config
0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1).
+ 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.

The package looks good. For this review to be approved please
- consider fixing or give reasons for not fixing the rpmlint warnings
- rename the source file to contain the package base name
- Add the required patch to get it to build on F-15, recheck #691133.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.