Bug 454960

I think I can make some time to review this, but the URL's you've posted are
404, and my psychic powers to guess the correct URL are failing me.

(In reply to comment #1)
> I think I can make some time to review this, but the URL's you've posted are
> 404, and my psychic powers to guess the correct URL are failing me.
> 

Bah!  Try again.  My system last night crashed before the ftp upload was
complete.  (dbus deamon segv)

Thanks!

Initial review done, needs some work:

* License: GPL, GPL is not a valid license tag, it should be "GPLv2+"
* Gives the following errors when doing a local build:
RPM build errors:
    Installed (but unpackaged) file(s) found:
   /usr/lib/python2.5/site-packages/aubio/__init__.py
   /usr/lib/python2.5/site-packages/aubio/__init__.pyc
   /usr/lib/python2.5/site-packages/aubio/__init__.pyo
   /usr/lib/python2.5/site-packages/aubio/_aubiowrapper.so
   /usr/lib/python2.5/site-packages/aubio/aubioclass.py
   /usr/lib/python2.5/site-packages/aubio/aubioclass.pyc
   /usr/lib/python2.5/site-packages/aubio/aubioclass.pyo
   <snip>

  It looks like aubio comes with python bindings, it would probably be best to
  add the necessary BR's and put the python stuff in a sub-package.

* rpmlint says:
  aubio.src: W: mixed-use-of-spaces-and-tabs (spaces: line 31, tab: line 1)
  aubio.x86_64: E: zero-length /usr/share/doc/aubio-0.3.2/NEWS
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libaubioext.so.2.1.1
['/usr/lib64']
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubiotrack ['/usr/lib64']
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubioonset ['/usr/lib64']
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubionotes ['/usr/lib64']
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib/python2.5/site-packages/

  You can fix this by putting the following 3 lines between %configure and make:
# Don't use rpath!
sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool

When I said you can fix this by ..., I meant you can fix the rpath stuff by ...,
the other 2 rpmlint issues need seperate fixing.

New files:

Spec URL: http://spindazzle.org/Fedora/aubio.spec
SRPM URL: http://spindazzle.org/Fedora/aubio-0.3.2-2.fc9.src.rpm

(In reply to comment #3)
> Initial review done, needs some work:
> 
> * License: GPL, GPL is not a valid license tag, it should be "GPLv2+"

Fixed.

> * Gives the following errors when doing a local build:
> RPM build errors:

I think this is because you have swig installed.  I've BuildRequired swig and
created a subpackage for the python bindings.

>     Installed (but unpackaged) file(s) found:
>    /usr/lib/python2.5/site-packages/aubio/__init__.py
>    /usr/lib/python2.5/site-packages/aubio/__init__.pyc
>    /usr/lib/python2.5/site-packages/aubio/__init__.pyo
>    /usr/lib/python2.5/site-packages/aubio/_aubiowrapper.so
>    /usr/lib/python2.5/site-packages/aubio/aubioclass.py
>    /usr/lib/python2.5/site-packages/aubio/aubioclass.pyc
>    /usr/lib/python2.5/site-packages/aubio/aubioclass.pyo
>    <snip>
> 
>   It looks like aubio comes with python bindings, it would probably be best to
>   add the necessary BR's and put the python stuff in a sub-package.
> 
> * rpmlint says:
>   aubio.src: W: mixed-use-of-spaces-and-tabs (spaces: line 31, tab: line 1)
>   aubio.x86_64: E: zero-length /usr/share/doc/aubio-0.3.2/NEWS
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libaubioext.so.2.1.1
> ['/usr/lib64']
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubiotrack
['/usr/lib64']
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubioonset
['/usr/lib64']
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubionotes
['/usr/lib64']
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib/python2.5/site-packages/

All fixed.

Thanks Hans!

AG




>   You can fix this by putting the following 3 lines between %configure and make:
> # Don't use rpath!
> sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
> sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
> 

I can still not approve this I'm afraid, it now fails to build on my system with:
RPM build errors:
    File not found by glob:
/var/tmp/aubio-0.3.2-2.fc10-root-hans/usr/lib64/python2.5/site-packages/aubio/*.so
    Installed (but unpackaged) file(s) found:
   /usr/lib/python2.5/site-packages/aubio/_aubiowrapper.so

Notice the lib64 in the path it is looking for, which is correct, you use 
%{python_sitearch}/%{name}/*.so

and sitearch is in lib64 on x86_64, and looking for the .so in sitearch is
correct, that is where it should be. So appearantly the aubio makefile gets this
wrong.

Note that it is allowed to put the entire python stuff in sitearch if it depends
upon a .so, so you could see if you can override the makefile install location
for the python stuff and just put it all in sitearch.

Also note that currently in your %files section, the %{python_sitearch}/%{name}
directory will be unowned when different from %{python_sitelib}/%{name}

Hmm, thinking some more about this, it would really be the cleanest to put all
of the python stuff in sitearch as it depends on a .so, if you cannot persuade
the makefile todo this you could use something like this after "make install":
if [ %{python_sitearch} != %{python_sitelib} ]; then
  mkdir -p $RPM_BUILD_ROOT%{python_sitearch}
  mv $RPM_BUILD_ROOT%{python_sitelib}/%{name} $RPM_BUILD_ROOT%{python_sitearch}
fi

This will also greatly simplify the %files for the -python package, that can
then become plain and simple:

%files python
%defattr(-,root,root,-)
%{python_sitearch}/%{name}

Thanks for your comments Hans.  Here we go again...

Spec URL: http://spindazzle.org/Fedora/aubio.spec
SRPM URL: http://spindazzle.org/Fedora/aubio-0.3.2-3.fc9.src.rpm

AG

Looks good now, approved!

Thanks!

New Package CVS Request
=======================
Package Name: aubio
Short Description: An audio labelling library
Owners: green
Branches: devel
InitialCC:
Cvsextras Commits: yes

cvs done.

Package Change Request
======================
Package Name: aubio
New Branches: F-9

cvs done.

Anthony,

I would be much oblidged if you could review bug 456772 in return, its a really
easy review and I need to get this into place so that I can continue my work on
the F-10 better webcam support feature:
https://fedoraproject.org/wiki/Features/BetterWebcamSupport

Thanks!

Never mind, already reviewed :)