Bug 834239
Summary: | Review Request: monobristol - frontend for britsol in mono | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jørn Lomax <northlomax> | ||||||||
Component: | Package Review | Assignee: | Orcan Ogetbil <oget.fedora> | ||||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | medium | ||||||||||
Version: | rawhide | CC: | brendan.jones.it, dwmw2, notting, oget.fedora, package-review | ||||||||
Target Milestone: | --- | Flags: | oget.fedora:
fedora-review+
gwync: fedora-cvs+ |
||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2012-11-26 11:26:02 UTC | Type: | --- | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Bug Depends On: | |||||||||||
Bug Blocks: | 805236 | ||||||||||
Attachments: |
|
Description
Jørn Lomax
2012-06-21 10:00:38 UTC
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). |