Bug 834239

Updated .spec:http://jvlomax.fedorapeople.org/packeging/monobristol.spec
updated SRPM: http://jvlomax.fedorapeople.org/packeging/monobristol-0.60.3-3.fc17.src.rpm

rpmlint .spec:
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint SRPM: 
monobristol.src: W: spelling-error Summary(en_US) bristol -> Bristol, bristle, bristly
monobristol.src: W: spelling-error %description -l en_US synthesisers -> synthesizers, synthesizer's, synthesizes
monobristol.src: W: spelling-error %description -l en_US subtractive -> subtracting, subtracted, subtract
1 packages and 0 specfiles checked; 0 errors, 3 warnings.

I got some notes to get things going.

* Package fails to build [1] with errors:
   RPM build errors:
      Macro %__isa_name has empty body
      Macro %__isa_bits has empty body
      File not found: /builddir/build/BUILDROOT/monobristol-0.60.3-3.fc18.noarch/usr/share/monobristol/monoBristol.exe

* Mono packages are not supposed to be noarch. Please follow the Mono packaging guidelines [2]. Note that you will need to suppress the empty debuginfo failure as explained in the guidelines.

* In the buid log, the .desktop file installation gives the warning. 

   monoBristol.desktop: error: (will be fatal in the future): value "monobristol.png" for key "Icon" in group "Desktop Entry" is an icon name with an extension, but there should be no extension as described in the Icon Theme Specification if the value is not an absolute path

Please fix this. This fix is upstreamable.


[1]   http://koji.fedoraproject.org/koji/taskinfo?taskID=4195234
[2]   http://fedoraproject.org/wiki/Packaging:Mono

Created attachment 594478 [details]
Patch for .desktop file

SPEC:http://jvlomax.fedorapeople.org/packeging/monobristol.spec
SRPM: http://jvlomax.fedorapeople.org/packeging/monobristol-0.60.3-4.fc17.src.rpm

rpmlint gives the following:
> hardcoded-library-path in /usr/lib/monobristol/monoBristol.exe

The reason is obvious, but i can't find a fitting macro. Should i create a macro in the spec file for this(%define/%global)?

Jorn, it is good practice in general 
- to take a look at the build log and the rpmlint output of all resulting packages (RPMs and the SRPM) for warning and error messages, 
- to build the package in mock, at least for rawhide, but better for rawhide and stable release(s) (If you do this you can rpmlint the result/ directory directly to evaluate all the resulting rpms inside. Two birds with one stone)
before you resubmit it during the review. Now,

* The fitting macro is %{_libdir}. You can find the standard macros in file /usr/lib/rpm/macros, and additional macro definition files inside /etc/rpm/

* rpmlint says (duplicates removed):
   monobristol.src: W: spelling-error %description -l en_US subtractive -> subtraction, subtract, attractive
   monobristol.x86_64: E: no-binary
   monobristol.x86_64: W: only-non-binary-in-usr-lib
   monobristol.x86_64: W: no-manual-page-for-binary monobristol

Let us ignore the above

   monobristol.src: W: spelling-error Summary(en_US) bristol -> Bristol
   monobristol.src: W: spelling-error %description -l en_US synthesisers -> synthesizers, synthesizer, synthesis

Please correct the above. Americanos use "synthesizers"

   monobristol.x86_64: E: non-executable-script /usr/share/applications/monoBristol.desktop 0644L /usr/bin/env

This should be fixed. I don't know why they ship a .desktop file with a shebang. Unless this is some very new .desktop file specification, please remove the shebang in your patch. I guess you will need to re-email (sorry).

* You are installing a .desktop file, which has a "Icons" tag. This means we better ship an icon with this package, so we don't end up with a naked menu entry. A good candidate is the monobristol.png file in the top source directory. Since this icon is 48x48 we shall install it at
   %{_datadir}/icons/hicolor/48x48/apps/

! BuildRequires: mono-devel monodevelop
   These don't seem necessary.

! You don't need the line
   mkdir -p %{buildroot}%{_datadir}/applications
it doesn't do any harm though, not a blocker.

? How do I use this application? None of the synths seem to open here.

* Note that since you will install an icon in %{_datadir}/icons/ , the relevant scriptlets need to be added to your specfile:
   http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

I always run rpmlint and mock before submiting an updated package. I'll admit i only use the default mock configuration though. I'll make sure to run it on both rawhide and stable from now on. I looked everywhere for that macro, wiki, google and i grep'ed both /etc/rpm and /usr/lib/rpm/macro in hopes of finding it, no luck.


What i did not know was that i could run rpmlint on .rpm files, i thought i would get the same output as with the srpms, Now,

SPEC: http://jvlomax.fedorapeople.org/packeging/monobristol.spec
SRPMS:  http://jvlomax.fedorapeople.org/packeging/monobristol-0.60.3-5.fc17.src.rpm


rpmlint on the specfile comes out clean

rpmlint on SRPMS:
>[makerpm@Fafnir SPECS]$ rpmlint ../SRPMS/monobristol-0.60.3-5.fc17.src.rpm 
>monobristol.src: W: spelling-error Summary(en_US) bristol -> Bristol, bristle, >bristly
>monobristol.src: W: spelling-error %description -l en_US subtractive -> >subtracting, subtracted, subtract
>1 packages and 0 specfiles checked; 0 errors, 2 warnings.

rpmlint on the built package:
>monobristol.i686: W: spelling-error %description -l en_US subtractive -> >subtracting, subtracted, subtract
>monobristol.i686: E: no-binary
>monobristol.i686: W: only-non-binary-in-usr-lib
>monobristol.i686: W: no-manual-page-for-binary monobristol
>monobristol.i686: W: percent-in-%post

From what i have read on the wikipage, the no-binary error is solved by making the package a "noarch" package? Which  bring up another question i have. When i was searching for macros earlier, i found one titled %mono_arches which is the arches mono builds on. Should i be using this (i was originally, and was told not to anymore)

As for getting it to run? is bristol showing up at all after you have pressed a button, or is the monobristol gui not showing at all? I never got any sound from bristol without running it through JACK instead of alsa

Created attachment 594749 [details]
Updates -desktop.patch

Thanks, I made a full review on this. 

Before I get to that let me explain my experience. The monobristol gui shows up for me with bunch of buttons. When I click on a button a black window pops up, stays for a few seconds, then disappears. There I get error messages in the terminal such as
   "Failed to open audio device default"
When I switch from ALSA to Jack, the Bristol synth GUI shows up, but I don't get a sound. 
   
Here is the review:

! Please replace "bristol" with "Bristol" in Summary too.

* A package must own all directories that it creates or require another package that does [1]. But %{_libdir}/%{name} remains unowned. So you will need to replace
   %{_libdir}/%{name}/monoBristol.exe
by either
   %dir %{_libdir}/%{name}/
   %{_libdir}/%{name}/monoBristol.exe
or by simply
   %{_libdir}/%{name}/

* Similarly %{_datadir}/icons/hicolor/ remains unowned. But this directory already depends on a standard package. So we will need to 
   Requires: hicolor-icon-theme

! Not really necessary in this case, but in my opinion, it would be nice to explain what each patch does with a short comment.

* We got a new rpmlint
   monobristol.x86_64: W: percent-in-%post
Please replace the %>/dev/null with &>/dev/null

* The source code does not specify a license. There is a COPYING file for GPLv3 and the README file says the software is GPL. In particular [2] says
   A GPL or LGPL licensed package that lacks any statement of what version that it's licensed under in the source code/program output/accompanying docs is technically licensed under *any* version of the GPL or LGPL, not just the version in whatever COPYING file they include. 
Therefore we shall set
   License: GPL+
Yet it is best to confirm this with upstream.

[1] http://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership
[2] http://fedoraproject.org/wiki/Licensing

SPEC: http://jvlomax.fedorapeople.org/packeging/monobristol.spec
SRPMS: http://jvlomax.fedorapeople.org/packeging/monobristol-0.60.3-6.fc17.src.rpm

No interesting rpmlint for either the .spec or rpms
rpmlint on built package on the other hand:


monobristol.i686: W: spelling-error %description -l en_US subtractive -> subtracting, subtracted, subtract
monobristol.i686: E: no-binary
monobristol.i686: W: only-non-binary-in-usr-lib
monobristol.i686: W: no-manual-page-for-binary monobristol
monobristol.i686: W: percent-in-%post
1 packages and 0 specfiles checked; 1 errors, 4 warnings.

I think it's "%{_datadir}" that it doesn't like having in post, so i think it can be ignored

I still haven't heard back from upstream, so i'll just label it as GPL+

Bristol does not automatically create jack connections, so you might have to connected it yourself. I also have trouble with the volume on Bristol being low at times too. Not sure if that's a Bristol issue or my setup

Thanks for the quick update.

(In reply to comment #10)
> monobristol.i686: W: percent-in-%post
> I think it's "%{_datadir}" that it doesn't like having in post, so i think
> it can be ignored
> 

I think this is because of "%postrun". It needs to be "%postun".

SPEC:http://jvlomax.fedorapeople.org/packeging/monobristol.spec
SRPMS: http://jvlomax.fedorapeople.org/packeging/monobristol-0.60.3-6.fc17.src.rpm

Same rpmlint output as above, minus "percent-in-%post"

Hi Jorn,
Could you post an SRPM with the new Release tag? By the way, the above links are broken due to typo in "packeging".

SPEC: http://jvlomax.fedorapeople.org/packeging/monobristol.spec
SRPMS: http://jvlomax.fedorapeople.org/packaging/monobristol-0.60.3-7.fc17.src.rpm

I had changed the version in the spec and change log, just seem to have forgotten top build :(

I think this is good to go. One little request: Could you append
   X-Synthesis;X-Jack;
to the Categories entry in the .desktop file? This will allow monobristol to show up in the Multimedia->Creation->DigitalProcessing and Jack submenus inside the Applications menu, once you have the "multimedia-menus" package installed. Thanks!

----------------------------------------------
This package (monobristol) is APPROVED by oget
----------------------------------------------

Created attachment 596192 [details]
patch for monoBRistol.desktop

New Package SCM Request
=======================
Package Name: monobristol
Short Description: gui for bristol synthesizer written in mono 
Owners: jvlomax
Branches:f16 f17
InitialCC:

I wonder how the flag got messed up. I guess it is me whose name is on the fedora-review+. Will try to fix.

Git done (by process-git-requests).