Bug 619355

This seems to fail to build for me in a Centos5 buildroot in mock.  Maybe I'm doing something wrong, but my understanding is that your pyver macro will break srpm parsing because it is empty before python is installed:

Executing command: ['bash', '--login', '-c', 'rpmbuild -bs --target x86_64 --nodeps builddir/build/SPECS/python26-numpy.spec']
sh: /usr/bin/python2.6: No such file or directory
sh: /usr/bin/python2.6: No such file or directory
error: line 26: Version required: Requires:       python(abi) =

Perhaps it would be better to simply require 2.6 (since it already kind of hardcoded into the name of the package) and dispense with the pyver macro altogether?

A scratch build is at http://koji.fedoraproject.org/koji/taskinfo?taskID=2642342

I've seen the message 
sh: /usr/bin/python2.6: No such file or directory
but in the past it has not been fatal.

I agree given hard coding of the package name hardcoding the python package
name makes sense.

Will post new ones soon.

Steve.

New packages, these build in mock okay for me.

http://cern.ch/straylen/rpms/python26-numpy/python26-numpy-1.5.1-1.el5.src.rpm
http://cern.ch/straylen/rpms/python26-numpy/python26-numpy.spec

rpmlint shows a lot of 

devel-file-in-non-devel-package.

I'm not sure how far it makes sense to deviate from current packing
of numpy as it stands in Fedora. It may be that f2py uses these "dev" files
at runtime as well.

Steve

How does this relate to 601891 ? 

Is it pretty much decided to ship a different package for this?

I've closed #601891 since that one is obsoleted by this one. That
was request to add a python26-numpy subpackage to the existing numpy package
in EPEL5 and that proved to be a bad idea.

Steve.

Hi Kevin,
if you have time to comment that would be great.
Steve.

Actually I wrote up most of a review that I just need to paste in here.  Hopefully I'll find sufficient time today to get that done.

Turns out that somehow I lost that saved review when I was cleaning up my in-progress reviews directory.  Ugh.

Rather than try to retype it all now, I'll go over the basic issues:

All of those devel-file-in-non-devel-package complaints:
These are present in the base numpy package, but this package doesn't get a pass just because the base package may be doing something dumb.  I asked the current maintainer of the numpy package but he did not know what those files were actually used for.  Probably best if you figure that out, given that you're going to be maintaining this.

There are some additional complaints about the f2py subpackage.  My understanding is that one actually does call the compiler, but then there's also the question of why stuff in the tests directory is in the final package.

There are several private-shared-object-provides warnings that I'd suggest filtering if el5 had the filtering infrastructure.

There's one no-soname complaint which I think is inconsequential.

Otherwise I can't really complain about the package.  I'll whip up a full review if we can get those questions answered.

After talking to people and trying a few things out the header files
should be in a -devel. They are used for compiling C extensions to numpy.
New packages shortly.

Steve

New packages , the tests and devel files are both moved out to separate
packages.

http://cern.ch/straylen/rpms/python26-numpy/python26-numpy-1.5.1-1.el5.src.rpm
http://cern.ch/straylen/rpms/python26-numpy/python26-numpy.spec

When 
python2.6 -c "import pkg_resources, numpy ; numpy.test()"

is executed without the -tests package installed then it successfully
runs no tests which is probably the correct thing to do. With -tests
installed they are all executed.

Concerning the f2py package then the work flow for this is roughly

1. Write a fortran.f subroutine.
2. Compile to a .so with 
   f2py -c fortran.f -m fortran
3. In python the .so can be loaded with "import fortran".

I could split python-numpy-f2py into a runtime and devel package
so that only step (3) would work without the devel package, if that 
is done then /usr/bin/f2py ends up in a -devel package which is possibly
odd in itself.

I am happy as it is but there may be comment.

Steve.

Not
> http://cern.ch/straylen/rpms/python26-numpy/python26-numpy-1.5.1-1.el5.src.rpm
> http://cern.ch/straylen/rpms/python26-numpy/python26-numpy.spec

but

http://cern.ch/straylen/rpms/python26-numpy/python26-numpy-1.5.1-2.el5.src.rpm
http://cern.ch/straylen/rpms/python26-numpy/python26-numpy.spec

Hi, any comment?

Steve

I have to apologize; by the beginning of February I completely lost all time to pursue package reviews.  Hopefully that's coming to an end and I'll have a chance to catch back up with everything soon.

Sorry, any comment.

Steve

Looks like you need a review before I'll be able to provide one.  Hopefully you can find someone else to do this.

I'll take this review.

I have a couple of observations before getting to the standard review items.  First, I saw a lot of warnings about code that breaks strict aliasing rules.  I suggest adding -fno-strict-aliasing to CFLAGS, else you may see some subtly weird behavior at runtime.

Second, I don't understand why both lapack-devel and atlas-devel are BRs.  The atlas package provides a lapack replacement (%{_libdir}/atlas/liblapack.so.3).  The only ELF object I can find that is linked to liblapack, linalg/lapack_lite.so, is indeed linked to %{_libdir}/atlas/liblapack.so.3.  Why is lapack-devel a BR?

Third, totally trivial, but 's/in in/be in/' on line 118 of the spec file.

rpmlint output:
python26-numpy.x86_64: W: no-soname /usr/lib64/python2.6/site-packages/numpy/lib/_compiled_base.so
python26-numpy.x86_64: W: unused-direct-shlib-dependency /usr/lib64/python2.6/site-packages/numpy/lib/_compiled_base.so /lib64/libpthread.so.0
python26-numpy-devel.x86_64: W: no-documentation
python26-numpy-f2py.x86_64: W: manpage-not-compressed bz2 /usr/share/man/man1/f2py2.6.1.gz
python26-numpy-f2py.x86_64: E: devel-dependency python26-devel
python26-numpy-f2py.x86_64: W: devel-file-in-non-devel-package /usr/lib64/python2.6/site-packages/numpy/f2py/tests/src/array_from_pyobj/wrapmodule.c
python26-numpy-f2py.x86_64: W: devel-file-in-non-devel-package /usr/lib64/python2.6/site-packages/numpy/f2py/src/fortranobject.h
python26-numpy-f2py.x86_64: W: devel-file-in-non-devel-package /usr/lib64/python2.6/site-packages/numpy/f2py/src/fortranobject.c
python26-numpy-f2py-tests.x86_64: E: devel-dependency python26-devel
python26-numpy-f2py-tests.x86_64: W: no-documentation
python26-numpy-f2py-tests.x86_64: W: devel-file-in-non-devel-package /usr/lib64/python2.6/site-packages/numpy/f2py/tests/src/array_from_pyobj/wrapmodule.c
python26-numpy-tests.x86_64: W: no-documentation
6 packages and 1 specfiles checked; 2 errors, 10 warnings.

#1, #3, #10, #12: appear inconsequential.
#2: it would be good to not link lib/_compiled_base.so against -lpthread if that isn't impossible to arrange.
#4: rpmlint has apparently gone insane, since ALL man pages are gz compressed
#5-#8: these appear to be a consequence of python26-numpy-f2py being a "stealth" devel package
#9, #11: ditto for python26-numpy-f2py-tests

So it looks like only #2 needs attention.  You only get that warning if you run rpmlint on the installed package, by the way; checking the binary rpm won't trigger this warning.

+: OK
-: Must fix
=: Needs attention (but not a review blocker)
N: Not applicable

MUST items:
[=] rpmlint output: shown above
[+] Package name follows the Package Naming Guidelines
[+] Spec file name matches the base package name
[+] Package meets the Packaging Guidelines
[+] Package has a Fedora-approved license
[=] License field matches the actual license
[+] Iff the license appears in a file, that file is in %doc.
[+] Spec file is in American Engligh
[+] Spec file is legible
[+] Sources match the upstream sources: md5sum is 376ef150df41b5353944ab742145352d
[+] Source RPM builds on a least one arch: x86_64
[N] Appropriate use of ExcludeArch
[+] BuildRequires for all build-time dependencies
[N] Proper handling of locales
[N] Proper use of ldconfig in %post and %postun
[+] No bundled copies of system libraries
[N] No relocatable packages
[+] Package owns all directories it creates
[+] No duplicates in %files
[+] Appropriate permissions on files
[+] Consistent use of macros
[+] Package contains code or permissible content
[+] Large documentation goes into a -doc subpackage
[+] No runtime dependencies in %doc
[+] Header files in -devel
[N] Static libraries in -static
[N] If a shared library has a suffix, then a .so symlink is in -devel
[=] -devel has a fully versioned Requires on the main package: this should be %{name}%{?_isa} in Fedora, but I don't know if that is the case in EPEL
[+] No libtool archives in the binary packages
[N] GUI applications install a desktop file
[+] Does not own any files/dirs owned by other packages
[+] All filenames are valid UTF-8

SHOULD items:
[N] Query upstream to include a missing license file
[N] Description and summary should contain translations, if available
[+] Package builds in mock: tried epel-5-x86_64
[=] Package builds on all supported arches: only tried x86_64
[=] Package functions as described: minimal testing only
[N] Sane scriptlets
[+] All subpackages require the main package
[N] Good pkgconfig file placement
[+] Package dependencies instead of file dependencies
[+] Package has man pages for binaries/scripts

Hi Jerry,

Thanks for the comments so far.

New packages:

http://cern.ch/straylen/rpms/python26-numpy/python26-numpy-1.5.1-3.el5.src.rpm
http://cern.ch/straylen/rpms/python26-numpy/python26-numpy.spec


The easy ones:

* The isa tags indeed do nothing on EPEL5
* s/in in/be in/ done.
* Added -fno-strict-aliasing.

The other ones:

Concerning the liblapack in atlas and lapack:
I've been reading around the history of this, good news is we can drop
the requirement on lapack since indeed everything and more is also in atlas.
This also allows the specification of library locations to be dropped in 
the %build and %install sections as well. They were there from a time after
EPEL atlas when lapack in atlas was not including all of lapack, it's just not needed
for EPEL5 versions of these packages.

The current situation with liblapack is definitely ugly and is
detailed in the bug #478856. Hopefully unrelated to this review I added a comment
that there may now be a way out of this situation but the fix will come from 
atlas and lapack and will take time if it comes at all.


I'll look at the 
/usr/lib64/python2.6/site-packages/numpy/lib/_compiled_base.so /lib64/libpthread.so.0
shortly.

I noticed you added a look more for the licensing, was it something in particular, I will
look more anyway.

Argh, I didn't copy in my notes about licenses.  Sorry about that.  I was just poking around in the sources, and found a few things with licenses other than BSD.  Here are some things that need to be looked at:

numpy/core/_mx_datetime_parser.py carries the eGenix public license 1.1.0.
numpy/lib/utils.py contains a SafeEval class which carries the Python license.
numpy/random/mtrand/{distributions.c,distributions.h,mtrand.pyx,randomkit.h} carry MIT licenses.
numpy/random/mtrand/randomkit.c is a mixture of BSD and MIT licenses.

http://cern.ch/straylen/rpms/python26-numpy/python26-numpy-1.5.1-4.el5.src.rpm
http://cern.ch/straylen/rpms/python26-numpy/python26-numpy.spec

These change the license field basically and add your notes plus a bit
more describing the license situation.

Concerning the eGenix license this is a modified python license but
should be approved of course.  I'll do that now.

As for the:
/usr/lib64/python2.6/site-packages/numpy/lib/_compiled_base.so
/lib64/libpthread.so.0

I can't see anything obvious to change here but nor can I find where it picks it up from.

Steve.

Don't worry aobut the _compiled_base.so thing.  It's a minor detail.

I saw your message about the eGenix license, but haven't seen a response.  Poke me if I missed it.  Once that's approved, it looks to me as though this package is ready.

The reply says to call that license "eGenix".  I'll trust you to put that into the license field when you import the package.  Since everything else is ready to go, I'll approve this package.

New Package SCM Request
=======================
Package Name: python26-numpy
Short Description: A fast multi-dimensional array facility for Python
Owners: stevetraylen
Branches: el5
InitialCC:


Thanks a lot for the review and for all the comments from previous folk
as well.

Steve.

Git done (by process-git-requests).

python26-numpy-1.5.1-5.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/python26-numpy-1.5.1-5.el5

python26-numpy-1.5.1-5.el5 has been pushed to the Fedora EPEL 5 testing repository.

python26-numpy-1.5.1-5.el5 has been pushed to the Fedora EPEL 5 stable repository.