Bug 665853 - Review Request: h5py - A Python interface to the HDF5 library
Review Request: h5py - A Python interface to the HDF5 library
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
: 509658 (view as bug list)
Depends On: 675798
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-27 08:18 EST by Terje Røsten
Modified: 2011-08-31 18:54 EDT (History)
5 users (show)

See Also:
Fixed In Version: h5py-1.3.1-6.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-08-31 18:54:34 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
steve.traylen: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
Patch to h5py to use system provided liblzf. (1.42 KB, patch)
2011-05-22 17:24 EDT, Steve Traylen
no flags Details | Diff
Patch to .spec to use system provided liblzf. (788 bytes, patch)
2011-05-22 17:26 EDT, Steve Traylen
no flags Details | Diff

  None (edit)
Description Terje Røsten 2010-12-27 08:18:07 EST
Spec: http://terjeros.fedorapeople.org/h5py/h5py.spec
SRPM: http://terjeros.fedorapeople.org/h5py/h5py-1.3.1-1.fc14.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2690057
Description:
The h5py package provides both a high- and low-level interface to the
HDF5 library from Python. The low-level interface is intended to be a
complete wrapping of the HDF5 API, while the high-level component
supports access to HDF5 files, datasets and groups using established
Python and NumPy concepts.

A strong emphasis on automatic conversion between Python (Numpy)
datatypes and data structures and their HDF5 equivalents vastly
simplifies the process of reading and writing data from Python.
Comment 1 Jason Tibbitts 2010-12-27 10:54:30 EST
*** Bug 509658 has been marked as a duplicate of this bug. ***
Comment 2 Steve Traylen 2011-01-13 04:17:27 EST
Review of h5py, 12th January:
https://bugzilla.redhat.com/show_bug.cgi?id=665853

- Package meets naming and packaging guidelines
YES: It does.
- Spec file matches base package name.
YES: It does.
- Spec has consistant macro usage.
YES: It does.
- Meets Packaging Guidelines.
YES: It does but see comments about eggs below.
- License
YES: BSD
- License field in spec matches
YES:
- License file included in package
YES: LICENSE.txt and licenses/*.txt
- Spec in American English
YES: It is.
- Spec is legible.
YES: It is.
- Sources match upstream md5sum:
YES: but see rpmlint error below.
$ md5sum h5py-1.3.1.tar.gz ../SOURCES/h5py-1.3.1.tar.gz 
cfef84992d33910a06371dc35becb71b  h5py-1.3.1.tar.gz
cfef84992d33910a06371dc35becb71b  ../SOURCES/h5py-1.3.1.tar.gz
- Package needs ExcludeArch
YES: Builds as is in koji.
- BuildRequires correct
YES: Builds in koji
- Spec handles locales/find_lang
YES: No locale.
- Package is relocatable and has a reason to be.
YES: Is not relocatable.
- Package has %defattr and permissions on files is good.
YES: It does.
- Package has a correct %clean section.
YES:
- Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
NO:
It has %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)

- Package is code or permissible content.
YES:
- Doc subpackage needed/used.
YES: not needed.
- Packages %doc files don't affect runtime.
YES: Theyt don't.
- Headers/static libs in -devel subpackage.
Not relavent.
- Spec has needed ldconfig in post and postun
Not relavent.
- .pc files in -devel subpackage/requires pkgconfig
Not relavent.
- .so files in -devel subpackage.
Not relavent.
- -devel package Requires: %{name} = %{version}-%{release}
Not relavent.
- .la files are removed.
None created.

- Package is a GUI app and has a .desktop file
It's not.
- Package compiles and builds on at least one arch.
Koji.
- Package has no duplicate files in %files.
It does not.
- Package doesn't own any directories other packages own.]
It does not.
- Package owns all the directories it creates.
It does.
- No rpmlint output.
$ rpmlint SPECS/h5py.spec RPMS/x86_64/h5py-* SRPMS/h5py-1.3.1-1.fc14.src.rpm 
SPECS/h5py.spec: W: invalid-url Source0: http://h5py.googlecode.com/files/h5py-1.3.1.tar.gz HTTP Error 404: Not Found
h5py.x86_64: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases
h5py.x86_64: W: spelling-error %description -l en_US datatypes -> data types, data-types, databases
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/utils.so utils.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/_proxy.so _proxy.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5r.so h5r.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/_conv.so _conv.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5o.so h5o.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5.so h5.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5l.so h5l.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5a.so h5a.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5f.so h5f.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5s.so h5s.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5p.so h5p.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5g.so h5g.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5z.so h5z.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5t.so h5t.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5fd.so h5fd.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5e.so h5e.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5i.so h5i.so()(64bit)
h5py.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/h5py/h5d.so h5d.so()(64bit)
h5py.src: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases
h5py.src: W: spelling-error %description -l en_US datatypes -> data types, data-types, databases
h5py.src: W: invalid-url Source0: http://h5py.googlecode.com/files/h5py-1.3.1.tar.gz HTTP Error 404: Not Found
3 packages and 1 specfiles checked; 0 errors, 24 warnings.

You could split datasets and datatypes into two wordss but I believe both to be on common usage.

The "invalid-url" I do not understand, that URL works just fine? Do you see this with rpmlint?


Issues:

1. You have a buildroot of
BuildRoot:      %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
rather than the normal (?)
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

maybe what you have is now permitted? It looks better in someways for sure.
Of course it's not needed at all of new Fedoras.


2. Investigate the "invalid-url"

3. There are a lot of "private-shared-object-provides" , these can be removed with the use of 
%{?filter_provides_in: %filter_provides_in .*/h5py/.*\.so}
%{?filter_setup}

4. Do you need the "Requires: hdf" since this satisfied by the auto requires to "libhdf5.so.6()(64bit)"
   I would presume?

5. What's the reason for 
   rm -rf %{buildroot}/%{python_sitearch}/%{name}-%{version}-py2.*.egg-info/

6. Could you add a comment as to why "-fopenmp" has been added to the CFLAGS.

Comments:
I looked also at adding this to EPEL5. It seems while you can choose the 1.6 api to hdf you can't easily
get around the requirement for python 2.5?

However the package builds fine on EPEL6. Please could it be added to EPEL6 though that is not
a requirement for the review. If my python26-numpy review ever makes it to EPEL5 then I will
submit a patch for h5py to work with it. I have a user request for this on CentOS 5.
Comment 3 Terje Røsten 2011-01-13 14:11:49 EST
Thanks!

Updated package:

- fix buildroot
- add filter
- don't remove egg-info files
- remove explicit hdf5 req
- build and ship docs as html

Unsure about the -fopenp thingie.

There is a bug in rpmlint (or the google server is a bit funny): 
   https://bugzilla.redhat.com/show_bug.cgi?id=669339

python 2.5 seems to be a hard req. 
btw: I don't maintain EPEL packages.

spec: http://terjeros.fedorapeople.org/h5py/h5py.spec
srpm: http://terjeros.fedorapeople.org/h5py/h5py-1.3.1-3.fc14.src.rpm
koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2719548
Comment 4 Steve Traylen 2011-01-23 13:14:28 EST
I was reading the old review in bug #509658  and there are a comment of items left which
I think still need comment.

Is the "open security issue" with lzf addressed?

You say the licensing issues is now resolved I guess because the LZF is an BSD but I still see
GPL stuff in lzf/lzf. 

Why is this only BSD now and not ""BSD and (BSD or GPLv2+)"?

Stve
Comment 5 Terje Røsten 2011-01-25 09:45:43 EST
I don't what the "open security issue" is, might have to ping Jason.

Hm, I must recheck the license issue, I did not see any GPL stuff.
Comment 6 Jason Tibbitts 2011-01-25 09:55:48 EST
Honestly I can't remember what it was.  I think there was an issue with decompression of large files, probably fixed with 1.4.

In any case, bundling libraries is expressly forbidden without an exemption these days, so that point is academic.  Bundling lzf is right out anyway.
Comment 7 Steve Traylen 2011-01-25 10:07:06 EST
 
> In any case, bundling libraries is expressly forbidden without an exemption
> these days, so that point is academic.  Bundling lzf is right out anyway.

http://oldhome.schmorp.de/marc/liblzf.html

looks to be the upstream.

Steve.
Comment 8 Steve Traylen 2011-05-22 17:24:52 EDT
Created attachment 500306 [details]
Patch to h5py to use system provided liblzf.

Hi,
This is patch your last .spec file that allows the system liblzf to be
used rather than the included one.
Comment 9 Steve Traylen 2011-05-22 17:26:56 EDT
Created attachment 500307 [details]
Patch to .spec to use system provided liblzf.

This is the patch to the .spec file. 
The previous patch was the patch the h5py tar ball its self.

Steve.
Comment 11 Steve Traylen 2011-06-17 15:16:31 EDT
Builds in mock f15 x86_64.

rpmlint results:
$ rpmlint ./h5py.spec /var/lib/mock/fedora-15-x86_64/result/*.rpm
./h5py.spec: W: invalid-url Source0: http://h5py.googlecode.com/files/h5py-1.3.1.tar.gz HTTP Error 404: Not Found
h5py.src: W: invalid-url Source0: http://h5py.googlecode.com/files/h5py-1.3.1.tar.gz HTTP Error 404: Not Found

this is clean, known problem with googlecode.


- Package meets naming and packaging guidelines

- Spec file matches base package name.
Yes named after tar ball.
- Spec has consistant macro usage.
They are.
- Meets Packaging Guidelines.
Yes.
- License
BSD
- License field in spec matches
License looks to be consistantly BSD
- License file included in package
Yes license directory included.
- Spec in American English
yes
- Spec is legible.
yes
- Sources match upstream md5sum:
$ md5sum h5py-1.3.1.tar.gz ../SOURCES/h5py-1.3.1.tar.gz 
cfef84992d33910a06371dc35becb71b  h5py-1.3.1.tar.gz
cfef84992d33910a06371dc35becb71b  ../SOURCES/h5py-1.3.1.tar.gz

- Package needs ExcludeArch
It  odes not
- BuildRequires correct
Look good and passes mock
- Spec handles locales/find_lang
Not important.
- Package is relocatable and has a reason to be.
Not relocatable.
- Package has %defattr and permissions on files is good.
Fine
- Package has a correct %clean section.
Fine
- Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
- Package is code or permissible content.
Yes
- Doc subpackage needed/used.
Not needed
- Packages %doc files don't affect runtime.
Thet don't

- Headers/static libs in -devel subpackage.
Not needed.
- Spec has needed ldconfig in post and postun
Not Needed.
- .pc files in -devel subpackage/requires pkgconfig
Not Needed.
- .so files in -devel subpackage.
Not Needed.
- -devel package Requires: %{name} = %{version}-%{release}
Not Needed.
- .la files are removed.
Not Needed.

- Package is a GUI app and has a .desktop file
Not Needed.
- Package compiles and builds on at least one arch.
Mock
- Package has no duplicate files in %files.
No
- Package doesn't own any directories other packages own.
It does not
- Package owns all the directories it creates.
It does.
- No rpmlint output.
See above.

- final provides and requires are sane:
They are indeed.

SHOULD Items:

- Should build in mock.
Yes
- Should build on all supported archs
Not checked but probablyu
- Should function as described.
%checks pass
- Should have sane scriptlets.
None
- Should have subpackages require base package with fully versioned depend.
Not relavent.

- Should package latest version
1.3.1 is  newest except for a beta.


Issues:
None

Package APPROVED.
Comment 12 Terje Røsten 2011-07-10 05:41:07 EDT
Thanks!

New Package SCM Request
=======================
Package Name: h5py
Short Description: A Python interface to the HDF5 library
Owners: terjeros
Branches: f14 f15
InitialCC:
Comment 13 Jon Ciesla 2011-07-10 08:57:54 EDT
Git done (by process-git-requests).

Steve, in the future, please take ownership of review bugs you're working
on.  Thanks!
Comment 14 Steve Traylen 2011-07-10 21:46:22 EDT
>Steve, in the future, please take ownership of review bugs you're working
>on.  Thanks!

Yes of course, just an omission , thanks.

Steve
Comment 15 Steve Traylen 2011-08-14 05:52:02 EDT
(In reply to comment #3)
> btw: I don't maintain EPEL packages.
> 

Package Change Request
======================
Package Name: h5py
New Branches: el6
Owners: stevetraylen
Comment 16 Jon Ciesla 2011-08-14 07:25:44 EDT
Git done (by process-git-requests).
Comment 17 Fedora Update System 2011-08-15 07:47:28 EDT
h5py-1.3.1-6.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/h5py-1.3.1-6.el6
Comment 18 Fedora Update System 2011-08-16 16:54:24 EDT
h5py-1.3.1-6.el6 has been pushed to the Fedora EPEL 6 testing repository.
Comment 19 Fedora Update System 2011-08-31 18:54:25 EDT
h5py-1.3.1-6.el6 has been pushed to the Fedora EPEL 6 stable repository.

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