I need some help.

Originally I built this package in Fedora 9. I just tried to build the SRPM in fedora 8. It built without any issues. But there is a dependency issue.

In F9, the package requires openjdk. F8 does not have openjdk, it has icedtea instead. icedtea runs the program fine in F8.

How should I set the "Requires" in the SPEC file if I want to cover both F8 and F9?

Nevermind previous message. I realized openjdk obsoletes icedtea. Setting icedtea as Requires covers both F8 and F9. SPEC file updated.

Please consider the updated files:
Spec URL: http://6mata.com:8014/tuxguitar/tuxguitar.spec
SRPM URL: http://6mata.com:8014/tuxguitar/tuxguitar-1.0-2.src.rpm

%changelog
* Thu Sep 25 2008 Orcan Ogetbil <orcanbahri[AT]yahoo[DOT]com> - 1.0-2
- Added desktop-file-utils to BuildRequires.
- Replaced java-1.7.0-icedtea with java-1.6.0-openjdk in BuildRequires.

* Wed Sep 24 2008 Orcan Ogetbil <orcanbahri[AT]yahoo[DOT]com> - 1.0-1
- Initial build.

Ref #1 and #2, the elegant solution is something along the following:

%if "0%{fedora}" > "0.fc8"
Requires: openjdk
%else
Requires: icedtea
%endif

This way you will not be affected if sometime in the future openjdk will no longer provide icedtea. I have assumed you will not try to build on anything earlier than F-8. Otherwise you will need extra test blocks.

Please substitute BuildRequires for Requires in the previous message. But the logic remains the same.

Alright, thanks. I made that change.

I also managed to compile the fluidsynth plugin the upstream offers but does not enable by default.

The thing is, this plugin of tuxguitar requires fluidsynth-libs as requirement. Should I package the plugin separately? 
(The software runs fine without the plugin. But having the plugin is cool, e.g. the software can run in sync with ardour, hydrogen etc. also plays audio through fluidsynth)

Please let me know of the redhat policies in such situations.

If it is a separate source file, you should 
- package it separately (especially if tuxguitar can be built and runs without it),
- make that package Require tuxguitar,
- and probably add a line somewhere in the tuxguitar package recommending to install it. Maybe in the description (something along: "for additional effects, please install blah-blah-blah") or in a Readme.fedora file. Alternatively you could add in tuxguitar a "Requires" for the plugin rpm, but this means that you have to push both packages simultaneously in the repository.

Unfortunately our rpm lacks the ability of soft-requires (aka "suggests") hence you have only the above two options.

The plugin is inside the same source file. But it is not enabled inside the compilation script. I had to patch the compilation script to enable the plugin.

But compiling the plugin build-requires fluidsynth-devel and then running tuxguitar requires fluidsynth-libs . Tuxguitar won't run if it is compiled with the plugin and if fluidsynth-libs is not installed.

So what do you think?

I for one see no reason at all to not enable the plugin, our usual policy is to provide the most enhanced version of whatever we provide, unless there is a very good reason to not do it. As of requiring other packages .. so what ? It is as normal as it could be.

Therefore I suggest to go for building the plugin. Additional to that, if - as a bonus -  you want to let other people build without it, you could use some build conditionals to disable it (i.e. disable BR, disable patch, etc)

Alright, one last question:

Most of the code is java. And AFAIK gcj doesn't take $RPM_OPT_FLAGS. But the code also contains some C files. Should I pass $RPM_OPT_FLAGS to gcc when compiling those files?

I figured that it wouldn't hurt to pass those flags. Here are the updated files:

SRPM: http://6mata.com:8014/tuxguitar/tuxguitar-1.0-3.fc9.src.rpm
SPEC: http://6mata.com:8014/tuxguitar/tuxguitar.spec

%changelog
* Thu Sep 26 2008 Orcan Ogetbil <orcanbahri[AT]yahoo[DOT]com> - 1.0-3
- Fixed java requirement issue by requiring icedtea for F-8 and openjdk for F-9+
- Patched the source to enable the fluidsynth plugin
- Added DistTag
- Patched the source in order to pass RPM_OPT_FLAGS to gcc
- Removed ExclusiveArch

Some notes:
* SourceURL
  - You may want to use %{name} %{version} macros (especially %{version}) tag in
    sourceURL:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D

* License
  - I cannot find out any parts in the source code which specifies the license to
    LGPL "version 2" specific (other than Makefile), so I think the license
    tag must be "LGPLv2+".
    Note that just putting "LGPLv2" copying file into the tarball does not specify
    the LGPL version to 2 only (due to the section 13 of LGPLv2 text).

* Requires
----------------------------------------------------
%if "0%{fedora}" > "0.fc8"
Requires: java-1.7.0-icedtea
%else
Requires: java-1.6.0-openjdk
%endif
----------------------------------------------------
   - Note that on F-9/10 icedtea does not exist (so I guess what you meant here is
     opposite to what is written here). Also "%{fedora}" tag is "10", "9", "8",
     not ".fc10" or so (this is %{?dist} tag). Finally this string comparison won't work
     for F-10: see:
     https://www.redhat.com/archives/fedora-devel-list/2008-May/msg02095.html

     Here if you want to have "java-1.7.0-icedtea" or "java-1.6.0-openjdk" added
     for Requires (and java-1.5.0-gcj is not sufficient), just
----------------------------------------------------
Requires: java >= 1.7
----------------------------------------------------
     is sufficient, because  java-1.7.0-icedtea provides "java = 1.7.0" and
     java-1.6.0-openjdk provides "java = 1:1.6.0" (this has Epoch 1, so is higher
     than 1.7).
     Also "Requies: jpackage-utils" is needed: see:
     https://fedoraproject.org/wiki/Packaging/Java#Java_Packaging

* Macros
  - Use macros for standard directories. For example /usr must be %{_prefix} (e.g.
----------------------------------------------------
        export CLASSPATH=$CLASSPATH:`rpm -ql libswt3-gtk2|grep swt.jar|awk 'NR==1'` \n\
!!!!    cd /usr\n\
        %{_libdir}/tuxguitar/tuxguitar.bin -Dtuxguitar.library.path=%{_libdir}/tuxguitar/plugins/" \$1 \ > tuxguitar
----------------------------------------------------

* Pre-shipped binaries
  - For safety I recommend to remove all .DS_Store files at %prep

* %defattr
  - For %defattr, see my comment on another your review request.

By the scratch build for dist-f10:
http://koji.fedoraproject.org/koji/taskinfo?taskID=847430
(successful)

the license: I saw the copying file and put LGPLv2 in there. I guess I'm wrong, will be more careful next time.

java: Oops, terrible typo! I considered putting java >= 1.7
But wouldn't that conflict with 
"""
For historical reasons, when specifying versions 1.6.0 or greater, an epoch of 1 must be included. Example:

Requires: java >= 1:1.6.0
"""
from https://fedoraproject.org/wiki/Packaging/Java#Java_Packaging ?

notes about macros, cleaning pre-shipped binaries, %defattr: All fixed

Thanks for the very extensive review and also for the sponsorship.

(In reply to comment #14)
> java: Oops, terrible typo! I considered putting java >= 1.7
> But wouldn't that conflict with 
> """
> For historical reasons, when specifying versions 1.6.0 or greater, an epoch of
> 1 must be included. Example:
> 
> Requires: java >= 1:1.6.0
> """
> from https://fedoraproject.org/wiki/Packaging/Java#Java_Packaging ?

This "1.6.0 or greater" refers to java-1.6.0-openjdk (currently, which may change
in the future, for example java-1.0.0-somethingother. In this case perhaps
virtual provides "Provides: java = 2:1.0.0" will be added). 
Note that java-1.7.0-icedtea does not satisfy "Requires: java >= 1:1.6.0".
So here "Requires: java >= 1.7" is okay.

This update covers all the changes mentioned:

SRPM: http://6mata.com:8014/tuxguitar/tuxguitar-1.0-4.fc9.src.rpm
SPEC: http://6mata.com:8014/tuxguitar/tuxguitar.spec

%changelog
* Sun Sep 28 2008 Orcan Ogetbil <orcanbahri[AT]yahoo[DOT]com> - 1.0-4
- Added the comment about %%{?_smp_mflags}
- Used macros more extensively.
- Changed the license to LGPLv2+
- Fixed java requirement issue by requiring java >= 1.7
- Required jpackage-utils
- Removed pre-shipped binaries
- Fixed %%defattr

I will submit it to koji ASAP.

Please do not conduct a review for now, as I am in touch with the upstream about the compilation of java. I will optimize the compilations process and do some more clean-up in the spec file. I will update the spec file ASAP.

Here are the updated files:
SRPM(F-8): http://6mata.com:8014/tuxguitar/tuxguitar-1.0-5.fc8.src.rpm
SRPM(F-9): http://6mata.com:8014/tuxguitar/tuxguitar-1.0-5.fc9.src.rpm
SPEC: http://6mata.com:8014/tuxguitar/tuxguitar.spec

Notes:
-I switched from gcj to openjdk for compiling the code. This enables more features in the software (not all the plugins compiled in gcj).
-I excluded ppc/ppc64 on F-8 since there is no icedtea (java 1.7) for them (see bug #464843). As you can see there are different src.rpms for each build.
-I removed the patches and do the modifications with sed. The link for the discussions of these mods is given in the spec:
http://www.tuxguitar.com.ar/forum/4/817/need-help-with-packaging-for-fedora/

The package can be reviewed now.

Created attachment 319139 [details]
crash log when tuxguitar exits

For 1.0-5:

* Requires
  - I guess at least "Requires: libswt3-gtk2" is missing.

* Using build-classpath
  - For example
--------------------------------------------------------
make SWT_JAR=%{_libdir}/java/swt.jar CFLAGS="${RPM_OPT_FLAGS} -I%{openjdkdir}/include -I%{openjdkdir}/include/linux -fPIC"
--------------------------------------------------------
    I guess using
--------------------------------------------------------
make SWT_JAR=$(build-classpath swt) CFLAGS=.....
--------------------------------------------------------
    is more portable on Fedora (see:
    https://fedoraproject.org/wiki/Packaging/Java#build-classpath )

    ! Note:
      Usually the environment CLASSPATH is not set by default.
      As far as I see misc/tuxguitar.sh when this script is called
      CLASSPATH seems to be set finally like:
--------------------------------------------------------
:/usr/share/tuxguitar/tuxguitar.jar:/usr/lib/java/swt.jar
--------------------------------------------------------
      Usually any environment value with begin with colon is
      unwilling (same for LD_LIBARARY_PATH)

* Some script improvement
  * desktop-file-install
--------------------------------------------------------
install -pm 644 %{SOURCE9} $RPM_BUILD_ROOT/%{_datadir}/applications/
desktop-file-install --dir $RPM_BUILD_ROOT%{_datadir}/applications --delete-original $RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop
--------------------------------------------------------
  - can be
--------------------------------------------------------
desktop-file-install \
	--dir $RPM_BUILD_ROOT%{_datadir}/applications \
	%{SOURCE9}
--------------------------------------------------------

  * installing hicolor icons
--------------------------------------------------------
for dim in 16x16 24x24 32x32 48x48 64x64 96x96; do
 install -dm 755 $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/$dim/apps/%{name}.png
 install -pm 644 TuxGuitar/share/skins/Lavender/icon-$dim.png $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/$dim/apps/%{name}.png
done
--------------------------------------------------------
    "$ rpm -ql tuxguitar" shows unwilling results. This creates
    the directory named "%{_datadir}/icons/hicolor/$dim/apps/%{name}.png"
    and installs icon under the directory.

  * installing mime icons
--------------------------------------------------------
install -pm 644 TuxGuitar/share/skins/Lavender/icon-96x96.png $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/96x96/mimetypes/audio-x-tuxguitar.png
install -pm 644 TuxGuitar/share/skins/Lavender/icon-96x96.png $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/96x96/mimetypes/audio-x-gtp.png
install -pm 644 TuxGuitar/share/skins/Lavender/icon-96x96.png $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/96x96/mimetypes/audio-x-ptb.png
---------------------------------------------------------
  - I guess
---------------------------------------------------------
for type in \
	tuxguitar gtp ptb
	do
	install -pm 644 TuxGuitar/share/skins/Lavender/icon-96x96.png \
		$RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/96x96/mimetypes/audio-x-${type}.png
done
---------------------------------------------------------
    is cleaner

* jni
  - jni files must be installed under %_libdir/%name:
    https://fedoraproject.org/wiki/Packaging/Java#Guideline

* desktop database update
  - When installed desktop file contains MimeType key, desktop database
    must be updated:
    https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database


! By the way it seems I can heard no sound and when I
  close tuxguitar:
---------------------------------------------------------
[tasaka1@localhost tuxguitar]$ tuxguitar 
fluidsynth: error: The "default" audio device is used by another application
#
# An unexpected error has been detected by Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x032afef2, pid=6874, tid=1616784
#
# Java VM: OpenJDK Client VM (1.6.0_0-b12 mixed mode linux-x86)
# Problematic frame:
# C  [libfluidsynth.so.1+0x14ef2]  delete_fluid_audio_driver+0x22
#
# An error report file with more information is saved as:
# /home/tasaka1/rpmbuild/Reviewing/tuxguitar/hs_err_pid6874.log
#
# If you would like to submit a bug report, please visit:
#   http://java.sun.com/webapps/bugreport/crash.jsp
#
^CMultiple segmentation faults occurred; can't display error dialog
-----------------------------------------------------------
  and hs_err_pid6874.log is attached.

(In reply to comment #19)
> Created an attachment (id=319139) [details]
> crash log when tuxguitar exits
> 
> For 1.0-5:
> 
> * Requires
>   - I guess at least "Requires: libswt3-gtk2" is missing.
> 

Yes, in update 1.0-2 I had removed that Requirement because rpmlint complained about explicit library dependency. But as it seems rpmbuild does not pick up this dependency automatically. Hence I re-included it.
FIXED


> * Using build-classpath
>   - For example
> --------------------------------------------------------
> make SWT_JAR=%{_libdir}/java/swt.jar CFLAGS="${RPM_OPT_FLAGS}
> -I%{openjdkdir}/include -I%{openjdkdir}/include/linux -fPIC"
> --------------------------------------------------------
>     I guess using
> --------------------------------------------------------
> make SWT_JAR=$(build-classpath swt) CFLAGS=.....
> --------------------------------------------------------
>     is more portable on Fedora (see:
>     https://fedoraproject.org/wiki/Packaging/Java#build-classpath )
> 

I had tried that. It does not work. Maybe because %{_libdir}/java/swt.jar is a symlink. But the actual jar file is %{_libdir}/eclipse/plugins/org.eclipse.swt.gtk.linux.x86_64_3.3.2.v3349.jar

I don't think SWT_JAR=$(build-classpath org.eclipse.swt.gtk.linux.x86_64_3.3.2.v3349) is a good solution.

NOT FIXED(?)

>     ! Note:
>       Usually the environment CLASSPATH is not set by default.
>       As far as I see misc/tuxguitar.sh when this script is called
>       CLASSPATH seems to be set finally like:
> --------------------------------------------------------
> :/usr/share/tuxguitar/tuxguitar.jar:/usr/lib/java/swt.jar
> --------------------------------------------------------
>       Usually any environment value with begin with colon is
>       unwilling (same for LD_LIBARARY_PATH)
> 
FIXED

> * Some script improvement
>   * desktop-file-install
> --------------------------------------------------------
> install -pm 644 %{SOURCE9} $RPM_BUILD_ROOT/%{_datadir}/applications/
> desktop-file-install --dir $RPM_BUILD_ROOT%{_datadir}/applications
> --delete-original $RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop
> --------------------------------------------------------
>   - can be
> --------------------------------------------------------
> desktop-file-install \
>  --dir $RPM_BUILD_ROOT%{_datadir}/applications \
>  %{SOURCE9}
> --------------------------------------------------------
> 
FIXED

>   * installing hicolor icons
> --------------------------------------------------------
> for dim in 16x16 24x24 32x32 48x48 64x64 96x96; do
>  install -dm 755
> $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/$dim/apps/%{name}.png
>  install -pm 644 TuxGuitar/share/skins/Lavender/icon-$dim.png
> $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/$dim/apps/%{name}.png
> done
> --------------------------------------------------------
>     "$ rpm -ql tuxguitar" shows unwilling results. This creates
>     the directory named "%{_datadir}/icons/hicolor/$dim/apps/%{name}.png"
>     and installs icon under the directory.
> 
FIXED

>   * installing mime icons
> --------------------------------------------------------
> install -pm 644 TuxGuitar/share/skins/Lavender/icon-96x96.png
> $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/96x96/mimetypes/audio-x-tuxguitar.png
> install -pm 644 TuxGuitar/share/skins/Lavender/icon-96x96.png
> $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/96x96/mimetypes/audio-x-gtp.png
> install -pm 644 TuxGuitar/share/skins/Lavender/icon-96x96.png
> $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/96x96/mimetypes/audio-x-ptb.png
> ---------------------------------------------------------
>   - I guess
> ---------------------------------------------------------
> for type in \
>  tuxguitar gtp ptb
>  do
>  install -pm 644 TuxGuitar/share/skins/Lavender/icon-96x96.png \
>   $RPM_BUILD_ROOT/%{_datadir}/icons/hicolor/96x96/mimetypes/audio-x-${type}.png
> done
> ---------------------------------------------------------
>     is cleaner
> 
FIXED

> * jni
>   - jni files must be installed under %_libdir/%name:
>     https://fedoraproject.org/wiki/Packaging/Java#Guideline
> 
FIXED

> * desktop database update
>   - When installed desktop file contains MimeType key, desktop database
>     must be updated:
>     https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#desktop-database
> 
FIXED

> 
> ! By the way it seems I can heard no sound and when I
>   close tuxguitar:
> ---------------------------------------------------------
I usually use gervill or timidity plugins for sound output, for the latter you have to start timidity (timidity -iA -Os & )in the background and this one has the best sound quality. 

The fluidsynth plugin is kinda choppy when it's used directly. Tuxguitar does not pick up correct buffer sizes etc. To use the fluidsynth I would recommend starting a fluidsynth daemon in the background (e.g. one can use qsynth for that purpose).

But I don't think this is a packaging issue. I provided the best I could from the material I have. (but I will report the issue to upstream, for sure)

The updated files are:
SRPM(F-8): http://6mata.com:8014/tuxguitar/tuxguitar-1.0-6.fc8.src.rpm
SRPM(F-9): http://6mata.com:8014/tuxguitar/tuxguitar-1.0-6.fc9.src.rpm
SPEC: http://6mata.com:8014/tuxguitar/tuxguitar.spec

Thanks for the very extensive review!

%changelog
* Wed Oct 01 2008 Orcan Ogetbil <orcanbahri[AT]yahoo[DOT]com> - 1.0-6
- Required libswt3-gtk2 since rpmbuild doesn't pick it up.
- Some more cleanup in the spec file
- Fixed a typo regarding installation of icons
- Called update-desktop-database in %%post and %%postun
- jni files put in %%_libdir_/%%name.

* Mon Sep 29 2008 Orcan Ogetbil <orcanbahri[AT]yahoo[DOT]com> - 1.0-5
- Compiled the package with openjdk instead of gcj.
- ExcludeArch'ed ppc/ppc64 on F-8.
...

Okay, with your explanation and some googling I managed to play sounds by
tuxguitar.

Two things from me:
* One more thing for misc/tuxguitar.sh
  - The line 142 should be:
----------------------------------------------------------------
    exec ${JAVA} ${JAVA_FLAGS} ${PACKAGE_MAIN} $arg "$@"
----------------------------------------------------------------
    i.e. add exec to replace the called shell to java process.

* Freeze at the end
  - Now I can hear sounds played by tuxguitar. However every time
    I quit tuxguitar it freezes and I have to kill the process
    (tuxguitar) by SIGKILL. Would you have any clue?

Wolfy, are you also reviewing this? If you don't think there is
any blocker on this bug I think I can approve this package after I receive
a reply from Orcan about 2 issues above.

> * One more thing for misc/tuxguitar.sh
>  - The line 142 should be:
>----------------------------------------------------------------
>    exec ${JAVA} ${JAVA_FLAGS} ${PACKAGE_MAIN} $arg "$@"
>----------------------------------------------------------------
>    i.e. add exec to replace the called shell to java process.
FIXED

> * Freeze at the end

That happened to me once (when I was running tuxguitar with timidity and openjdk) out of so many and I reported it to the author. See
http://www.tuxguitar.com.ar/forum/4/817/need-help-with-packaging-for-fedora/
Search for "OutOfMemoryError".

But I wasn't able to re-produce this. As I indicated in the above link we can try playing with the stack sizes (the memory allocation for each thread, indicated by "-Xms128m -Xmx128m" in the launching script). The author suggested to increase them while google says decreasing might help.

I suggest switching to sun's java (no need to recompile, just use "alternatives") and try to see if it will freeze again. If it doesn't then there is a problem with openjdk and we need it to report to them.

We might be able to get more feedback once the package hits rawhide.

The updated files:
SRPM(F-8): http://6mata.com:8014/tuxguitar/tuxguitar-1.0-7.fc8.src.rpm
SRPM(F-9): http://6mata.com:8014/tuxguitar/tuxguitar-1.0-7.fc9.src.rpm
SPEC: http://6mata.com:8014/tuxguitar/tuxguitar.spec

%changelog
* Thu Oct 02 2008 Orcan Ogetbil <orcanbahri[AT]yahoo[DOT]com> - 1.0-7
- Added "exec" to replace the called shell to java process in the launching script

Okay. for now I will accept this package (I cannot try Sun's Java
for now...)

--------------------------------------------------------------
    This package (tuxguitar) is APPROVED by mtasaka
--------------------------------------------------------------

Thank you very much !

New Package CVS Request
=======================
Package Name: tuxguitar
Short Description: A multitrack tablature editor and player
Owners: oget
Branches: F-9 F-8
InitialCC: mtasaka

cvs done.

tuxguitar-1.0-7.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/tuxguitar-1.0-7.fc8

tuxguitar-1.0-7.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/tuxguitar-1.0-7.fc9

Thanks. After F-8/9 tuxguitar are pushed to testing, when you think
they can be moved to stable please modify (edit) the submitted
push requests.

Closing.

tuxguitar-1.0-7.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

tuxguitar-1.0-7.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.