[Bug 492990] Review Request: zynjacku - LV2 synths and plugins host

Fri Apr 10 18:43:07 UTC 2009

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=492990

--- Comment #2 from Orcan 'oget' Ogetbil <oget.fedora at gmail.com>  2009-04-10 14:43:07 EDT ---
(In reply to comment #1)
> Fedora review zynjacku-4-1.fc10.src.rpm 2009-04-10
> 

Thank you for the review!

> 
> * The stated license (GPLv2 and GPLv2+ and LGPLv2+ and Public Domain)
>   is approved by Fedora
> 
> ! The license tag in the specfile correctly discribes the license of
>   the sources. However, the license tag should describe the license of
>   the content of the binary packages. The content of the binary
>   package in this case is the two python script lv2rack.py and
>   zynjacku.py, and the shared library zynjacku_c.so. None of the
>   header files are packaged as part of the binary package.
> 
>   - The python scripts are licensed under GPLv2.
> 
>   - The shared library is compiled from sources having the 4 different
>     licenses you state in the License tag. A compiled unit compiled
>     from sources with different but compatible licenses falls under
>     the strictest of the licenses, in this case GPLv2. See:
>     https://fedoraproject.org/wiki/Licensing/FAQ#Multiple_licensing_situations
> 
>   So all components of the packaged binary package are GPLv2, so the
>   License tag should simply be GPLv2.
> 

Thanks for the correction. Changed to GPLv2

> 
> ! Package compiles in mock (Fedora 10), however the specfile uses sed
>   to change configure and aclocal.m4 thereby changing the timestamps
>   on these files resulting in compilation warnings like:
> 
> aclocal.m4:14: error: this file was generated for autoconf 2.61.
> You have another version of autoconf.  If you want to use that,
> you should regenerate the build system entirely.
> aclocal.m4:14: the top level
> autom4te: /usr/bin/m4 failed with exit status: 63
> automake-1.10: autoconf failed with exit status: 63
> WARNING: `automake-1.10' is probably too old.  You should only need it if
>          you modified `Makefile.am', `acinclude.m4' or `configure.ac'.
>          You might want to install the `Automake' and `Perl' packages.
>          Grab them from any GNU archive site.
> cd . && /bin/sh ./config.status Makefile depfiles
> config.status: creating Makefile
> config.status: executing depfiles commands
> cd . && /bin/sh /builddir/build/BUILD/zynjacku-4/config/missing --run
> autoheader
> aclocal.m4:14: error: this file was generated for autoconf 2.61.
> You have another version of autoconf.  If you want to use that,
> you should regenerate the build system entirely.
> aclocal.m4:14: the top level
> autom4te: /usr/bin/m4 failed with exit status: 63
> autoheader: '/usr/bin/autom4te' failed with exit status: 63
> WARNING: `autoheader' is probably too old.  You should only need it if
>          you modified `acconfig.h' or `configure.ac'.  You might want
>          to install the `Autoconf' and `GNU m4' packages.  Grab them
>          from any GNU archive site.
> 
>   The timestamps should either be restored after the change:
> 
>   sed s/A/B/ file > file.new ; touch -r file file.new ; mv file.new file
> 
>   or all files that are generated from the changed files should be
>   touched in the appropriate order, so that regeneration is not
>   triggered,
> 
>   or the full autoconf bootstrap chain should be rerun so that all
>   files are generated with the autotools version of the distribution
>   the RPM is compiled for.
> 
>   The first option is the easiest one to implement.
> 
>   At closer inspection, I don't think the modification is needed at
>   all. The hardcoded path that is changed by the sed replacement is
>   only used as a fallback when the call to sysconfig.get_python_lib
>   fails, which should never happen.
> 
>   Rebuilding the package with the sed replacements removed still puts
>   the library in /usr/lib64/python2.5/site-packages/zynjacku_c.so on
>   x86_64 - without the warnings caused by changing the timestamps.
> 

Yes, removing sed's don't break anything. I should've been more careful. I
wiped out those sed's.

> 
>   I assume since JACK_MIDI is found OLD_JACK_MIDI isn't needed, right?
> 

Correct.

> 
> ? The package owns the directories it creates, or depend on packages
>   that do. The dependency path to the owner of the directory
>   /usr/share/icons/hicolor/72x72/apps is quite long though:
> 
>   zynjacku → pygtk2-libglade → pygtk2 → gtk2 → hicolor-icon-theme
> 
>   You are confident that any future updates to any of the packages in
>   this chain will not break it?
> 

All of them are solid dependencies. gtk2 → hicolor-icon-theme dependency is
used in many packages. I don't think that will be broken in their lifetime.

Spec URL: http://oget.fedorapeople.org/review/zynjacku.spec
SRPM URL: http://oget.fedorapeople.org/review/zynjacku-4-2.fc10.src.rpm

Changelog: 4-2
- License is GPLv2
- Clean up unnecessary bits from SPEC file

-- 
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.