[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 --- Comment #25 from Fedora Update System upda...@fedoraproject.org 2008-12-21 03:44:37 EDT --- rakarrack-0.2.0-5.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 David Timms [EMAIL PROTECTED] changed: What|Removed |Added Status|NEW |CLOSED Resolution||NEXTRELEASE --- Comment #22 from David Timms [EMAIL PROTECTED] 2008-10-20 08:29:13 EDT --- imported, committed and built on devel, F9, F8, EPEL -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 --- Comment #24 from Fedora Update System [EMAIL PROTECTED] 2008-10-20 17:46:31 EDT --- rakarrack-0.2.0-5.fc8 has been submitted as an update for Fedora 8. http://admin.fedoraproject.org/updates/rakarrack-0.2.0-5.fc8 -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 --- Comment #23 from Fedora Update System [EMAIL PROTECTED] 2008-10-20 17:46:27 EDT --- rakarrack-0.2.0-5.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/rakarrack-0.2.0-5.fc9 -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 Kevin Fenzi [EMAIL PROTECTED] changed: What|Removed |Added Flag|fedora-cvs? |fedora-cvs+ --- Comment #21 from Kevin Fenzi [EMAIL PROTECTED] 2008-10-19 18:22:56 EDT --- cvs done. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 --- Comment #18 from David Timms [EMAIL PROTECTED] 2008-10-16 05:57:03 EDT --- New Package CVS Request === Package Name: rakarrack Short Description: Audio effects processing rack for guitar Owners: dtimms Branches: F-8 F-9 EL-5 InitialCC: ps. oget: I think the review procedures requires you set the assigned to yourself as reviewer. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 Orcan 'oget' Ogetbil [EMAIL PROTECTED] changed: What|Removed |Added AssignedTo|[EMAIL PROTECTED]|[EMAIL PROTECTED] --- Comment #19 from Orcan 'oget' Ogetbil [EMAIL PROTECTED] 2008-10-16 06:14:30 EDT --- sorry, I missed that :) done -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 David Timms [EMAIL PROTECTED] changed: What|Removed |Added Flag||fedora-cvs? --- Comment #20 from David Timms [EMAIL PROTECTED] 2008-10-16 17:15:37 EDT --- And for me to set the cvs flag... doh. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #14 from David Timms [EMAIL PROTECTED] 2008-10-15 09:35:54 EDT
---
(In reply to comment #11)
* rakarrack.desktop - %{name}.desktop
I looked at it and thought, maybe I should assign a variable to store the
complete path to .desktop in. Do people normally do things like that ?
Meanwhile I changed to point the spelt out path using %{name}
? %{name} is a basic rack of effects for guitar... - Rakarrack is a basic
rack of effects for guitar...
Overzealous search and replace. Fixed.
* This line needs to be in the %prep section:
%configure --docdir=%{_docdir}/%{name}-%{version}
--htmldir=%{_docdir}/%{name}-%{version}
I'm learning more about make configure and so on, thanks for these pointers.
* These lines need to be in the %prep section too:
%{__sed} -i 's/Icon=icono_rakarrack_128x128/Icon=rakarrack/'
%{buildroot}%{_datadir}/applications/rakarrack.desktop
%{__sed} -i 's/Guitar Effects Processor/Real-time audio effects processing
rack for guitar/' %{buildroot}%{_datadir}/applications/rakarrack.desktop
echo GenericName=Digital audio effects processor
%{buildroot}%{_datadir}/applications/rakarrack.desktop
echo Version=1.0
%{buildroot}%{_datadir}/applications/rakarrack.desktop
Moved.
You may need to change the %{buildroot}%{_datadir}/applications/ to data/
Correct, and done.
Basically, the %build section is for building/compiling, %install section is
to install the software into %{buildroot}. Everything else that can be done
before coming to these sections must be done in %prep.
First I've heard of it, but makes complete sense, as long as the build doesn't
build the icons / desktop files during make etc on the fly.
* %doc AUTHORS README
%{_datadir}/doc/%{name}/COPYING
%{_datadir}/doc/%{name}/html
This has problems. Now there are two document directories created:
/usr/share/doc/rakarrack /usr/share/doc/rakarrack-0.2.0
You only need one document directory. COPYING and html needs to go into %doc
(which is /usr/share/doc/rakarrack-0.2.0).
What you have to do is to make the program point onto the correct document
directory when you click on the Help-contents button. You may need to hack
Thanks for the sed hack.
In the end I still had to manually move the files to the doc-versioned dir so
that rpm and the app finds these things as expected.
It seems that this is an issue with some part of the source, ie the passed
--docdir and --htmldir are not properly used during configure, make and
install.
I am not sure what causes the html+COPYING files to end up in a non-versioned
doc dir, otherwise I would send a patch upstream.
So you decided to not add a new category? I'd say that would be convenient for
people dealing with audio software. :)
I wanted to, and a response from Matthias on f-packaging suggests it is OK, as
long as they are in addition to the standard categories, and prepended with X-.
Done.
Updated spec: http://members.iinet.net.au/~timmsy/rakarrack/rakarrack.spec
New .src.rpm:
http://members.iinet.net.au/~timmsy/rakarrack/rakarrack-0.2.0-4.fc9.src.rpm
- move non-install commands to setup
- fix configure .ini so that standard help path will be used
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #16 from David Timms [EMAIL PROTECTED] 2008-10-15 17:41:36 EDT
---
(In reply to comment #15)
The variable HELPDIR should normally pick up the --htmldir flag you pass to
configure. But for some reason they put a hardcoded path for HELPDIR which
confused us in the beginning. But make sure you let the upstream know about
these problems.
Will do.
* You won't need this line twice. Once (before %configure) is just fine:
%{__sed} -i
's|HELPDIR=$prefix/share/doc/${PACKAGE}|HELPDIR=$prefix/share/doc/${PACKAGE}-%{version}|'
configure.in
In trying to get html/helpdir to work, I thought the sed changes may have been
failing, or overwritten before the make. Unfortunately I left my debug cruft in
the spec ;-|, now gone.
* When submitting your final version, make sure you remove the commands you
commented out that are not relevant. For instance:
done. it was getting late, and I was keen to have something published so that
my progress could be reviewed.
* Careful! :
#%%{_datadir}/doc/%{name}/COPYING
-- #%%{_datadir}/doc/%%{name}/COPYING
Well you will need to completely remove this line anyways. But keep in mind
that having a macro in a comment with a single % is enough reason to not
approve a package. I know the rules are very strict but it is those rules that
make fedora consistent. :)
Did it again :(. Actually, this check could possibly be performed by rpmlint -
fail bad specs. I'll take that up with it's upstream.
Updated spec: http://members.iinet.net.au/~timmsy/rakarrack/rakarrack.spec
New .src.rpm:
http://members.iinet.net.au/~timmsy/rakarrack/rakarrack-0.2.0-5.fc9.src.rpm
- del debug cruft left in the spec while trying to solve issues
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #15 from Orcan 'oget' Ogetbil [EMAIL PROTECTED] 2008-10-15
15:02:16 EDT ---
(In reply to comment #14)
*
(In reply to comment #11)
* rakarrack.desktop - %{name}.desktop
I looked at it and thought, maybe I should assign a variable to store the
complete path to .desktop in. Do people normally do things like that ?
I don't think you need that. The way it is is fine.
*
In the end I still had to manually move the files to the doc-versioned dir so
that rpm and the app finds these things as expected.
It seems that this is an issue with some part of the source, ie the passed
--docdir and --htmldir are not properly used during configure, make and
install.
I am not sure what causes the html+COPYING files to end up in a non-versioned
doc dir, otherwise I would send a patch upstream.
Yes I noticed that. The problem is in the docdir definitions in doc/Makefile.am
and doc/Makefile.in files. But sed'ding them is not recommended because then
you will need to run autoconf tools which might result in different
configureMakefiles depending on the autoconf version.
I think your workaround (moving the files manually) is just fine until a fix
comes from upstream.
The variable HELPDIR should normally pick up the --htmldir flag you pass to
configure. But for some reason they put a hardcoded path for HELPDIR which
confused us in the beginning. But make sure you let the upstream know about
these problems.
*
Updated spec: http://members.iinet.net.au/~timmsy/rakarrack/rakarrack.spec
New .src.rpm:
http://members.iinet.net.au/~timmsy/rakarrack/rakarrack-0.2.0-4.fc9.src.rpm
- move non-install commands to setup
- fix configure .ini so that standard help path will be used
Great. Last remarks:
* You won't need this line twice. Once (before %configure) is just fine:
%{__sed} -i
's|HELPDIR=$prefix/share/doc/${PACKAGE}|HELPDIR=$prefix/share/doc/${PACKAGE}-%{version}|'
configure.in
* When submitting your final version, make sure you remove the commands you
commented out that are not relevant. For instance:
#echo aftt:==
* Careful! :
#%%{_datadir}/doc/%{name}/COPYING
-- #%%{_datadir}/doc/%%{name}/COPYING
Well you will need to completely remove this line anyways. But keep in mind
that having a macro in a comment with a single % is enough reason to not
approve a package. I know the rules are very strict but it is those rules that
make fedora consistent. :)
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 Orcan 'oget' Ogetbil [EMAIL PROTECTED] changed: What|Removed |Added Alias||rakarrack Flag|fedora-review? |fedora-review+ --- Comment #17 from Orcan 'oget' Ogetbil [EMAIL PROTECTED] 2008-10-15 22:19:08 EDT --- Well done. Now it's time to play some guitar :) - This package (rakarrack) is APPROVED by oget. - I recommend you to do some package reviews. I learned a lot about packaging while doing reviews. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #11 from Orcan 'oget' Ogetbil [EMAIL PROTECTED] 2008-10-12
02:17:21 EDT ---
Thanks. Still there are some issues
*: needs fixed .
?: doesn't need fixed but my preference is different than yours. Take it or
don't.
* rakarrack.desktop - %{name}.desktop
? %{name} is a basic rack of effects for guitar... - Rakarrack is a basic rack
of effects for guitar...
because this is a Description. Unless the application's name is strictly
beginning with a non-capital letter, I would refer to it capital-lettered in
sections like Summary or Description.
* This line needs to be in the %prep section:
%configure --docdir=%{_docdir}/%{name}-%{version}
--htmldir=%{_docdir}/%{name}-%{version}
* These lines need to be in the %prep section too:
%{__sed} -i 's/Icon=icono_rakarrack_128x128/Icon=rakarrack/'
%{buildroot}%{_datadir}/applications/rakarrack.desktop
%{__sed} -i 's/Guitar Effects Processor/Real-time audio effects processing
rack for guitar/' %{buildroot}%{_datadir}/applications/rakarrack.desktop
echo GenericName=Digital audio effects processor
%{buildroot}%{_datadir}/applications/rakarrack.desktop
echo Version=1.0 %{buildroot}%{_datadir}/applications/rakarrack.desktop
You may need to change the %{buildroot}%{_datadir}/applications/ to data/
Basically, the %build section is for building/compiling, %install section is to
install the software into %{buildroot}. Everything else that can be done before
coming to these sections must be done in %prep.
* %doc AUTHORS README
%{_datadir}/doc/%{name}/COPYING
%{_datadir}/doc/%{name}/html
This has problems. Now there are two document directories created:
/usr/share/doc/rakarrack /usr/share/doc/rakarrack-0.2.0
You only need one document directory. COPYING and html needs to go into %doc
(which is /usr/share/doc/rakarrack-0.2.0).
What you have to do is to make the program point onto the correct document
directory when you click on the Help-contents button. You may need to hack the
code (with some patch) to achieve this or most likely (and hopefully) it will
be enough just to pass certain parameters to the configure script. Look at the
output of ./configure --help to see what parameters you may use.
--
So you decided to not add a new category? I'd say that would be convenient for
people dealing with audio software. :)
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #12 from Orcan 'oget' Ogetbil [EMAIL PROTECTED] 2008-10-12
02:23:45 EDT ---
Actually, the last thing in the previous message that I wrote was:
* This line needs to be in the %prep section:
%configure --docdir=%{_docdir}/%{name}-%{version}
--htmldir=%{_docdir}/%{name}-%{version}
These may be the right parameters to pass to the configure script. You may need
to check it out.
Sorry I forgot to edit my last paragraph accordingly.
(Still %configure needs to go into %prep.)
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #13 from Orcan 'oget' Ogetbil [EMAIL PROTECTED] 2008-10-12
02:56:04 EDT ---
OK, this is probably what you need to do (regarding the docs)
...
%prep
...
sed -i
's|HELPDIR=$prefix/share/doc/${PACKAGE}|HELPDIR=$prefix/share/doc/${PACKAGE}-%{version}|'
configure
%configure --docdir=%{_docdir}/%{name}-%{version}
--htmldir=%{_docdir}/%{name}-%{version}
...
%files
%doc AUTHORS README html COPYING
...
It should be something like this.
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #8 from David Timms [EMAIL PROTECTED] 2008-10-11 22:53:49 EDT ---
(In reply to comment #6)
(In reply to comment #4)
(In reply to comment #3)
Thanks! But I need to correct myself here.
%{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/$dim/apps
%{__mv} %{buildroot}%{_datadir}/pixmaps/icono_rakarrack_$dim.png \
%{buildroot}%{_datadir}/icons/hicolor/$dim/apps/rakarrack.png
would be better. None of the existing icons I saw in hicolors/$dim/apps/ are
named as application.$dim.png . They are all named as application.png . Let's
keep the thing as they are.
Yes, reading freedesktop .desktop spec says that preferably the icon should be
just name, and the app can then choose either an svg, or most appropriate size
png. Giving this a go.
You can overwrite the existing .desktop file with %Source1 after make
install
so there won't be an ambiguity. I think the final .desktop file in the RPM
should be named rakarrack.desktop (not rakarrack-fedora.desktop , this would
be
unusual!).
Actually, the typical names from what I can see are fedora-rakarrack.desktop.
This seems to be based on the vendor tag. The vendor item has been removed from
the desktop-file-install command.
In the spec file, avoid using %macros directly inside comments and changelogs.
e.g. this is WRONG:
I'll keep an eye out for my attempted quick comments ;-)
I recommend using the %{name} macro instead of hardcoding the name of the
application into the spec file; i.e. in this case replacing the occurrences of
rakarrack * with %{name} is the preferred way. Same thing applies for the
%{version} macro.
Done.
OK. Finally got the sed and echo mods to the desktop file to work correctly:
Updated spec: http://members.iinet.net.au/~timmsy/rakarrack/rakarrack.spec
New .src.rpm:
http://members.iinet.net.au/~timmsy/rakarrack/rakarrack-0.2.0-3.fc9.src.rpm
* Sun Oct 12 2008 David Timms iinet.net.au @ dtimms - 0.2.0-3
- don't exclude the original .desktop file
* Mon Oct 06 2008 David Timms iinet.net.au @ dtimms - 0.2.0-2
- mod icon names to be appname.png to fit with fallback desktop spec
- replace rakarrack with name macro
- mod .desktop via sed and cat, rather than additional Source file
- add .desktop extra categories using the desktop-file-install utils
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #9 from David Timms [EMAIL PROTECTED] 2008-10-11 23:17:04 EDT ---
(In reply to comment #3)
http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
implies that you can add/remove categories. Did I miss something?
The original PlanetCCRMA spec included:
# desktop file categories
BASE=X-Fedora Application AudioVideo
XTRA=X-Digital_Processing X-Jack
%{__mkdir} -p %{buildroot}%{_datadir}/applications
desktop-file-install --vendor fedora \
--dir %{buildroot}%{_datadir}/applications \
`for c in ${BASE} ${XTRA} ; do echo --add-category $c ; done` \
%{SOURCE1}
see also:
https://www.redhat.com/archives/fedora-packaging/2008-October/msg00096.html
From what you mention, it is OK to add extra categories; I was under the
impression that only the categories mentioned on freedesktop are considered OK
?
--
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.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 --- Comment #10 from David Timms [EMAIL PROTECTED] 2008-10-12 00:11:02 EDT --- License has been confirmed as GPLv2. https://sourceforge.net/forum/forum.php?thread_id=2323002forum_id=778861 -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 --- Comment #7 from Orcan Ogetbil [EMAIL PROTECTED] 2008-10-06 05:50:44 EDT --- Btw, you can also use sed to replace items in the original .desktop file and echo some_new_items file to add items to the .desktop file; so that you won't have to add a new source file. Don't forget to change the icon name in the .desktop file to rakarrack.png -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
--- Comment #4 from David Timms [EMAIL PROTECTED] 2008-10-05 08:05:31 EDT ---
(In reply to comment #3)
The package is pretty good shape. Here are my notes
Thanks for taking the time to review rakarrack !
[?] MUST: The License field in the package spec file must match the actual
license.
COPYING file says GPLv3 , doc/COPYING file says GPLv2 , {.C} files say GPL
(version 2) explicitly. I would contact the author and ask what the actual
license is. If that's not possible I think GPLv2 will be the best option
(which
is what you have already).
As suggested I have posted a forum query on the developers' source forge site:
https://sourceforge.net/forum/forum.php?thread_id=2323002forum_id=778861
In the meantime, I'll assume GPLv2 since that is what is in the source code.
[x] MUST: If a package includes something as %doc, it must not affect the
runtime of the application. To summarize: If it is in %doc, the program must
run properly if it is not present.
If you go to the Help- contents or Help-about-license you will get errors.
Those need fixed.
Confirmed, and fixed.
[?] MUST: Dist tag is present and proper.
Afaik the usual way in Fedora is the usage n%{?dist} where n is the release
number. Why did you use 0.n%{?dist} ?
OK. While initially working on the package on my own system, I used the -0.x
pre-release scheme. I will properly fit the fedora scheme with the next update.
[x] MUST: Compiler flags are appropriate.
Fedora specific compilation flags are not honored correctly. As the result
debuginfo rpm is currently not useful. You can check what optflags are used by
$ rpm --eval %optflags
Please see:
https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
How did you work that out ?
The .debuginfo has a reasonable size, installs OK, contains .c/.h files and
what appears to be the debuginfo.
The opt flags are included, but some are added twice for example from a
rpmbuild:
-
if g++ -DHAVE_CONFIG_H -I. -I. -I.
-O2 -Wall -msse -fno-rtti -pipe -ffunction-sections -fomit-frame-pointer
-Wno-format-y2k -fPIC -fno-exceptions -fno-strict-aliasing
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -MT FilterParams.o -MD -MP -MF
.deps/FilterParams.Tpo -c -o FilterParams.o FilterParams.C; \
-
rpm --eval %optflags
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables
Seems the result would be conflicting options for -f{no}exceptions. I don't
know if the the app will work without that option, and had no success in
avoiding the default CFLAGS. Help wanted !
-
Additional comments:
This code
# move icons to the proper freedesktop location
%{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/32x32/apps
%{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/64x64/apps
%{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/128x128/apps
%{__mv} %{buildroot}%{_datadir}/pixmaps/icono_rakarrack_32x32.png \
%{buildroot}%{_datadir}/icons/hicolor/32x32/apps/rakarrack.32x32.png
%{__mv} %{buildroot}%{_datadir}/pixmaps/icono_rakarrack_64x64.png \
%{buildroot}%{_datadir}/icons/hicolor/64x64/apps/rakarrack.64x64.png
%{__mv} %{buildroot}%{_datadir}/pixmaps/icono_rakarrack_128x128.png \
%{buildroot}%{_datadir}/icons/hicolor/128x128/apps/rakarrack.128x128.png
will be cleaner if its written as
for dim in 32x32 64x64 128x128; do
%{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/$dim/apps
%{__mv} %{buildroot}%{_datadir}/pixmaps/icono_rakarrack_$dim.png \
%{buildroot}%{_datadir}/icons/hicolor/$dim/apps/rakarrack.$dim.png
done
OK, I can see that saves a few lines, and it will limit the amount of rework
that might be required if paths needed to be changed etc. Done.
-
What is the %exclude for?
%exclude %{_datadir}/applications/rakarrack.desktop
%{_datadir}/applications/*desktop
Can't you just use
%{_datadir}/applications/*desktop
or
%{_datadir}/applications/%{name}.desktop
The source includes a .desktop file, that make install puts in the normal
location. The spec generates another with the fedora required bits added, and
prepended with fedora-*. I found that without the %{exclude} I end up with two
Rakarrack items in the menu.
I had tried to remove items from the included .desktop, but didn't seem to be
able to do this with desktop-file-install:
included: rakarrack-0.2.0/data/rakarrack.desktop
-
[Desktop Entry]
Type=Application
Categories=AudioVideo;Audio;
Exec=rakarrack
Name=Rakarrack
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953 --- Comment #5 from David Timms [EMAIL PROTECTED] 2008-10-05 08:20:18 EDT --- ps: Is this a pre-review ? I can't seem to find your user name in FAS. -- 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. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
Orcan Ogetbil [EMAIL PROTECTED] changed:
What|Removed |Added
Flag||fedora-review?
--- Comment #6 from Orcan Ogetbil [EMAIL PROTECTED] 2008-10-05 21:18:39 EDT
---
(In reply to comment #4)
(In reply to comment #3)
The package is pretty good shape. Here are my notes
Thanks for taking the time to review rakarrack !
[?] MUST: The License field in the package spec file must match the actual
license.
COPYING file says GPLv3 , doc/COPYING file says GPLv2 , {.C} files say GPL
(version 2) explicitly. I would contact the author and ask what the actual
license is. If that's not possible I think GPLv2 will be the best option
(which
is what you have already).
As suggested I have posted a forum query on the developers' source forge site:
https://sourceforge.net/forum/forum.php?thread_id=2323002forum_id=778861
In the meantime, I'll assume GPLv2 since that is what is in the source code.
Thanks!
-
[?] MUST: Dist tag is present and proper.
Afaik the usual way in Fedora is the usage n%{?dist} where n is the release
number. Why did you use 0.n%{?dist} ?
OK. While initially working on the package on my own system, I used the -0.x
pre-release scheme. I will properly fit the fedora scheme with the next
update.
That's fine, as long as you turn to the standard at the end of the day.
-
[x] MUST: Compiler flags are appropriate.
Fedora specific compilation flags are not honored correctly. As the result
debuginfo rpm is currently not useful. You can check what optflags are used
by
$ rpm --eval %optflags
Please see:
https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
How did you work that out ?
The .debuginfo has a reasonable size, installs OK, contains .c/.h files and
what appears to be the debuginfo.
The opt flags are included, but some are added twice for example from a
rpmbuild:
-
if g++ -DHAVE_CONFIG_H -I. -I. -I.
-O2 -Wall -msse -fno-rtti -pipe -ffunction-sections -fomit-frame-pointer
-Wno-format-y2k -fPIC -fno-exceptions -fno-strict-aliasing
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -MT FilterParams.o -MD -MP -MF
.deps/FilterParams.Tpo -c -o FilterParams.o FilterParams.C; \
-
rpm --eval %optflags
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables
Seems the result would be conflicting options for -f{no}exceptions. I don't
know if the the app will work without that option, and had no success in
avoiding the default CFLAGS. Help wanted !
Uh oh, sorry my bad. I missed that the %configure macro takes care of the
CFLAGS and CXXFLAGS. About -f{no}exceptions : If two opposite flags are passed,
the latter is picked up. So you are fine in this case. You can keep it the way
you had in the 0.1 spec file.
In the future make sure that nothing overrides the opt flags. (Always the last
ones are picked up!)
-
will be cleaner if its written as
for dim in 32x32 64x64 128x128; do
%{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/$dim/apps
%{__mv} %{buildroot}%{_datadir}/pixmaps/icono_rakarrack_$dim.png \
%{buildroot}%{_datadir}/icons/hicolor/$dim/apps/rakarrack.$dim.png
done
OK, I can see that saves a few lines, and it will limit the amount of rework
that might be required if paths needed to be changed etc. Done.
Thanks! But I need to correct myself here.
%{__mkdir} -p %{buildroot}%{_datadir}/icons/hicolor/$dim/apps
%{__mv} %{buildroot}%{_datadir}/pixmaps/icono_rakarrack_$dim.png \
%{buildroot}%{_datadir}/icons/hicolor/$dim/apps/rakarrack.png
would be better. None of the existing icons I saw in hicolors/$dim/apps/ are
named as application.$dim.png . They are all named as application.png . Let's
keep the thing as they are.
-
What is the %exclude for?
%exclude %{_datadir}/applications/rakarrack.desktop
%{_datadir}/applications/*desktop
Can't you just use
%{_datadir}/applications/*desktop
or
%{_datadir}/applications/%{name}.desktop
The source includes a .desktop file, that make install puts in the normal
location. The spec generates another with the fedora required bits added, and
prepended with fedora-*. I found that without the
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
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=455953
Orcan Ogetbil [EMAIL PROTECTED] changed:
What|Removed |Added
CC||[EMAIL PROTECTED]
--- Comment #3 from Orcan Ogetbil [EMAIL PROTECTED] 2008-10-04 04:32:53 EDT
---
The package is pretty good shape. Here are my notes
-
Please fix or comment about items listed with [?] and [x].
[.]: good/pass
[x]: bad/fail
[o]: ugly... just kidding, this means N/A
[?]: questions/comments
[.] MUST: rpmlint must be run on every package. The output should be posted in
the review.
rpmlint is silent
[.] MUST: The package must be named according to the Package Naming Guidelines
.
[.] MUST: The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption on Package Naming Guidelines
.
[.] MUST: The package must be licensed with a Fedora approved license and meet
the Licensing Guidelines .
[?] MUST: The License field in the package spec file must match the actual
license.
COPYING file says GPLv3 , doc/COPYING file says GPLv2 , {.C} files say GPL
(version 2) explicitly. I would contact the author and ask what the actual
license is. If that's not possible I think GPLv2 will be the best option (which
is what you have already).
[.] MUST: If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of the
license(s) for the package must be included in %doc.
[.] MUST: The spec file must be written in American English.
[.] MUST: The spec file for the package MUST be legible. If the reviewer is
unable to read the spec file, it will be impossible to perform a review. Fedora
is not the place for entries into the Obfuscated Code Contest
(http://www.ioccc.org/).
[.] MUST: The sources used to build the package must match the upstream source,
as provided in the spec URL.
e836896fe76aae6aba447e44cdceba4f1c3d422feaf43857bb8e3e041378d297
rakarrack-0.2.0.tar.gz
[.] MUST: The package must successfully compile and build into binary rpms on
at least one supported architecture.
[o] MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch.
[.] MUST: All build dependencies must be listed in BuildRequires, except for
any that are listed in the exceptions section of the Packaging Guidelines ;
inclusion of those as BuildRequires is optional. Apply common sense.
[o] MUST: The spec file MUST handle locales properly. This is done by using the
%find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
[o] MUST: Every binary RPM package which stores shared library files (not just
symlinks) in any of the dynamic linker's default paths, must call ldconfig in
%post and %postun.
[o] MUST: If the package is designed to be relocatable, the packager must state
this fact in the request for review, along with the rationalization for
relocation of that specific package. Without this, use of Prefix: /usr is
considered a blocker.
[.] MUST: A package must own all directories that it creates. If it does not
create a directory that it uses, then it should require a package which does
create that directory. Refer to the Guidelines for examples.
[.] MUST: A package must not contain any duplicate files in the %files listing.
[.] MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example. Every %files section must include a
%defattr(...) line.
[.] MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot} ( or $RPM_BUILD_ROOT ).
[.] MUST: Each package must consistently use macros, as described in the macros
section of Packaging Guidelines .
[.] MUST: The package must contain code, or permissable content. This is
described in detail in the code vs. content section of Packaging Guidelines .
[o] MUST: Large documentation files should go in a -doc subpackage.
[x] MUST: If a package includes something as %doc, it must not affect the
runtime of the application. To summarize: If it is in %doc, the program must
run properly if it is not present.
If you go to the Help- contents or Help-about-license you will get errors.
Those need fixed.
[.] MUST: ScriptletSnippets are used properly.
[o] MUST: Header files must be in a -devel package.
[o] MUST: Static libraries must be in a -static package.
[o] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
(for directory ownership and usability).
[o] MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1), then library files that end in .so (without suffix) must go in
a -devel package.
[o] MUST: In the
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: Review Request: rakarrack - Audio effects processing rack for guitar
https://bugzilla.redhat.com/show_bug.cgi?id=455953
--- Additional Comments From [EMAIL PROTECTED] 2008-08-01 10:13 EST ---
(In reply to comment #1)
Oops. I noticed that I am ending up placing two desktop items in the Audio
Video
submenu, one without an icon.
New spec: http://members.iinet.net.au/~timmsy/rakarrack/rakarrack.spec
New .src.rpm:
http://members.iinet.net.au/~timmsy/rakarrack/rakarrack-0.2.0-0.2.fc9.src.rpm
- exclude the upstream .desktop file, in favour of fedora one.
2. A request was made from planetccrma users to add two additional categories to
the .desktop file:
# desktop file categories
BASE=X-Fedora Application AudioVideo
XTRA=X-Digital_Processing X-Jack
These put the audio processing tools into a separate single menu. I am happy to
do that if the packaging guidelines don't block me from doing so {which my
reading suggests not to make new categories}. However, there are a lot of
related audio tools that audio experimenters will find useful to be accessible
from the same menu. Any thoughts on whether this is allowed ?
--
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, or are watching someone who is.
___
Fedora-package-review mailing list
Fedora-package-review@redhat.com
http://www.redhat.com/mailman/listinfo/fedora-package-review
[Bug 455953] Review Request: rakarrack - Audio effects processing rack for guitar
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: rakarrack - Audio effects processing rack for guitar https://bugzilla.redhat.com/show_bug.cgi?id=455953 [EMAIL PROTECTED] changed: What|Removed |Added Summary|Review Request: rakarrack - |Review Request: rakarrack - |Audio effects processing|Audio effects processing |rack for guitar |rack for guitar --- Additional Comments From [EMAIL PROTECTED] 2008-07-28 18:25 EST --- Oops. I noticed that I am ending up placing two desktop items in the Audio Video submenu, one without an icon. I think I noticed newer rakarrak includes a .desktop internally, need to check it out. -- 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, or are watching someone who is. ___ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
