Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=576349

starting review... (still here ?)

Ok - we need to have festival-devel and festival-speechtools-devel fixed first.
Need to have (STK) packaged also.

So, few others notes.
* Qt i18n 
configure:21663: checking Qt translations
configure:21679: WARNING: *** not found! You may want to install a Qt i18n package
I don't know any package that provides that, but you may want to check
acinclude.m4 where live the gw_CHECK_QT function. and probably remove it to pick
a nicer m4 for qt detection. ( for pathes on lib64 and to not hardcode the
needed library).

* Source0
http://downloads.sourceforge.net/lmms/lmms-%{version}.tar.bz2
instead of 
http://prdownloads.sourceforge.net/lmms/lmms-0.3.2.tar.bz2

* You can use:
%configure --disable-static \
%ifnarch i386
--without-vst \
%endif
<space>

* You need to use make %{?_smp_mflags} (unless it fails and you need to drop a
comment if it does.

* Prevent timestramps changes at install :
 make DESTDIR=%{buildroot} install INSTALL="install -p"

* CFLAGS : -ftree-vectorize -ftree-loop-linear are used and may override some of
our CFLAGS (or not - anyway we may need to see if this is really needed).

* post/postun
update-mime-database %{_datadir}/mime &> /dev/null || :
update-desktop-database &> /dev/null || :
 You need to add full path for them like /usr/bin/update-desktop-database
and remove the &> /dev/null (as if it fails, we will not know)
only the last || : would be needed

* desktop file 
- Usually GNOME HIG compliance requires to use verbs in Comments
- Icon and Exec should mention the path (and .png)
It would be fine to drop a symlink into /usr/share/pixmaps 
- X-Desktop-File-Install-Version=0.3 is written before desktop-file-install is used.
- Categories / "MimeType" is a list and does not have a semicolon as trailing
character, fixing - this could be fixed at source also.

- License is GPLv2+ - OK

ping ?

(In reply to comment #3)
> Need to have (STK) packaged also.

Started, but not finished yet.
 
> So, few others notes.
> * Qt i18n 
> configure:21663: checking Qt translations
> configure:21679: WARNING: *** not found! You may want to install a Qt i18n package

Fixed, and will be fixed in 0.3.3.

> * Source0
> http://downloads.sourceforge.net/lmms/lmms-%{version}.tar.bz2
> instead of 
> http://prdownloads.sourceforge.net/lmms/lmms-0.3.2.tar.bz2

Fixed.

> * You can use:
> %configure --disable-static \
> %ifnarch i386
> --without-vst \
> %endif
> <space>

Done.

> * You need to use make %{?_smp_mflags} (unless it fails and you need to drop a
> comment if it does.

Parallel building indeed fails for 0.3.2, but upstream told me it works for
their current trunk. We'll see. Disabled for now.

> * Prevent timestramps changes at install :
>  make DESTDIR=%{buildroot} install INSTALL="install -p"

Pretty uncommon and not covered by the package guidelines.

> * CFLAGS : -ftree-vectorize -ftree-loop-linear are used and may override some of
> our CFLAGS (or not - anyway we may need to see if this is really needed).

-ftree-vectorize
     Perform loop vectorization on trees. This flag is enabled by
     default at `-O3'.

-ftree-loop-linear
     Perform linear loop transformations on tree.  This flag can
     improve cache performance and allow further loop optimizations to
     take place.

Don't see a reason to not enable this.

> * post/postun
> update-mime-database %{_datadir}/mime &> /dev/null || :
> update-desktop-database &> /dev/null || :
>  You need to add full path for them like /usr/bin/update-desktop-database
> and remove the &> /dev/null (as if it fails, we will not know)
> only the last || : would be needed

Copied that from
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database.

> * desktop file 
> - Usually GNOME HIG compliance requires to use verbs in Comments

Will ask upstream to fix that.

> - Icon and Exec should mention the path (and .png)

Can you explain what you mean?

> It would be fine to drop a symlink into /usr/share/pixmaps 

Done.

> - X-Desktop-File-Install-Version=0.3 is written before desktop-file-install is
used.
> - Categories / "MimeType" is a list and does not have a semicolon as trailing
> character, fixing - this could be fixed at source also.

Again, will ask upstream to fix that. Meanwhile, not a real problem, because
that's fixed by desktop-file-install.

> - License is GPLv2+ - OK

Found one file (drumsynth) that has a MIT license in it.

Spec URL: http://thm.fedorapeople.org/lmms.spec
SRPM URL: http://thm.fedorapeople.org/lmms-0.3.2-2.fc9.src.rpm

%changelog
* Wed Jun 25 2008 Thomas Moschny <...> - 0.3.2-2
- Update license tag.
- Add patch to fix plugin dir on 64bit archs.
- Add ladspa-caps-plugins dependency.
- Suggestions from the review:
  * Simplify configure step.
  * Fix sf.net download link.
  * Add patch to let configure find the qt translations.
  * Link icon to _datadir/pixmaps.

As i told you on irc. Let's see if the internal library can be built shared.
If you didn't managed to get a shared version, just submit a work in progress
version and i will have a look.

@Thomas
Updates here?

(In reply to comment #7)
> @Thomas
> Updates here?

Well, this is currently blocked by the festival problem, bug 242607 (which I have no time to work on, but which on the other hand shouldn't be too hard to solve) and the stk review, bug 454215. I'm working on the latter.

So, currently no updates here, but as soon has there is progress in the other bugs, I'll of course update my lmms package.

Quick test of rebuild lmms:

On initial setup, theses default install directories seems wrong (on lib64 systems)
-VST-plugin directory
/home/kwizart
- FL Studo isntall directory
/home/kwizart
- Ladspa plugin directory
/usr/lib/ladspa/:/usr/local/lib/ladspa/

When running from command line:
which: no x-www-browser in (/usr/lib64/qt-3.3/bin:/usr/kerberos/bin:/usr/local/bin:/usr/bin:/bin:/usr/X11R6/bin:/home/kwizart/bin)
It would be better to switch to "the xdg-open %url" scheme.

I'm currently testing usability...

(In reply to comment #9)
> Quick test of rebuild lmms:
> 
> On initial setup, theses default install directories seems wrong (on lib64
> systems)
> -VST-plugin directory
> /home/kwizart
> - FL Studo isntall directory
> /home/kwizart

As the user is expected to get the corresponding plugins somewhere else (there are no packages yet (ever?) ), these two paths don't seem wrong to me. There simply is no sane system-wide default.

> - Ladspa plugin directory
> /usr/lib/ladspa/:/usr/local/lib/ladspa/

> When running from command line:
> which: no x-www-browser in

Already having patches for these issues, but: as lmms 0.4 is around the corner, and has these issues already fixed, I'll update my package soon.

The second release candidate for LMMS was released recently.

Here's the current state of my package:

Spec URL: http://thm.fedorapeople.org/lmms.spec
SRPM URL: http://thm.fedorapeople.org/lmms-0.4.0-0.1.rc2.fc9.src.rpm

Do you have tested it previously in Rawhide ?
http://koji.fedoraproject.org/koji/taskinfo?taskID=864292
Failed to find wine-devel on x86_64...Might be a temporary problem...

testing F-9: 
http://koji.fedoraproject.org/koji/taskinfo?taskID=864350

Well, I didn't know that the build system is strictly non-multilib.
Everything went fine in my local mock builds.

So, I see two options here:
- disable vst hosting capabilities completely on 64bit.
- make a subpackage for the vst host, and build that 32bit, but make it usable on 64bit also.

The latter is more user-friendly, but needs some patching of the (c)makefiles. If time permits, I'll try to go that way.

Let me known when you have something ready.
(or maybe should we wait for stable lmms as Fedora development Freeze isn't that far).
Actually I've missed why wine-devel would be needed on x86_64!
As a workaround, maybe it would be possible to have the needed headers bundled within the sources?!

We now are at rc3 (no stable yet).

(In reply to comment #16)
> We now are at rc3 (no stable yet).

I know, and I already did some more work on that package, see http://koji.fedoraproject.org/koji/taskinfo?taskID=895060 - however need to find out why it fails on ppc64.

Spec URL: http://thm.fedorapeople.org/lmms.spec
SRPM URL: http://thm.fedorapeople.org/lmms-0.4.0-0.2.rc3.fc9.src.rpm


%changelog
* Fri Oct 24 2008 Thomas Moschny <..> - 0.4.0-0.2.rc3
- Add patch to fix libdir on ppc64.

* Tue Oct 21 2008 Thomas Moschny <..> - 0.4.0-0.1.rc3
- Update to 0.4.0rc3.
- Don't build remote_vst_plugin.exe on x86_64, but do build all other
  parts of the VST host there.


Scratch Builds:
http://koji.fedoraproject.org/koji/taskinfo?taskID=902533 (dist-f10)
http://koji.fedoraproject.org/koji/taskinfo?taskID=902549 (dist-f9)

Spec URL: http://thm.fedorapeople.org/lmms.spec
SRPM URL: http://thm.fedorapeople.org/lmms-0.4.0-1.fc10.src.rpm

%changelog
* Sun Nov  2 2008 Thomas Moschny <..> - 0.4.0-1
- Updated to 0.4.0 final.
- Removed patches already applied upstream, adjusted the remaining.

http://koji.fedoraproject.org/koji/taskinfo?taskID=924578
Package was also tested in F-8 x86_64 with rpmbuild (using a fresh libsamplerate 0.14)

You could just enable shared build for libsamplerate since it was updated in july for rawhide and just today for F-8/F-9. Since ABI has been preserved I will try to push it... in F-8/F-9

The default directories are accurate even for lib64 systems. That was the remaining issue from my previous runtime test.

---------------------

This package is APPROVED by me.

---------------------
Please enable BR: libsamplerate >= 0.1.3 before import

Thanks for this review!


New Package CVS Request
=======================
Package Name: lmms
Short Description: Linux MultiMedia Studio
Owners: thm
Branches: F-9 F-10
InitialCC: none
Cvsextras Commits: yes

cvs done.

Still not imported ! Is there a problem ?

(In reply to comment #23)
> Still not imported ! Is there a problem ?

Yes: See the fixme at the top of the spec file. While the complete package is GPLv2+, some of the included demo and tutorial projects might not. Some of them are taken from the LSP (http://lmms.sourceforge.net/lsp.php), with various licenses, and those that are not, are under a CC license (according to upstream, but upstream couldn't tell me (yet) which one).

I didn't find enough time to sort that out, that's why I delayed the import. If that issue can be postponed, I can do the import real soon.

if some content are unsure, you can also remove them from the source tarball
and repack the whole sources.

(In reply to comment #25)
> if some content are unsure, you can also remove them from the source tarball
> and repack the whole sources.

Yes. I've asked upstream for clarification. If I don't get an answer, I'll repack the tarfile.

Meanwhile, we are at 0.4.2.

(In reply to comment #26)

> I've asked upstream for clarification. If I don't get an answer, I'll
> repack the tarfile.

See http://sourceforge.net/mailarchive/forum.php?thread_name=49790675.5020701%40gmx.de&forum_name=lmms-devel

so, can you please remove out the usure files (if uncertain: every) so the package got imported in fedora.
Please

(In reply to comment #28)
> so, can you please remove out the usure files (if uncertain: every) so the
> package got imported in fedora.

Done. Package imported into rawhide.

It would be awesome if you could build this for F-10 as well, especially since you branched for it.

lmms-0.4.4-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/lmms-0.4.4-1.fc10

lmms-0.4.4-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.