Bug 459455 - (fmit) Review Request: fmit - Free Music Instrument Tuner
Review Request: fmit - Free Music Instrument Tuner
Status: CLOSED DUPLICATE of bug 665995
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-19 00:57 EDT by Jeff Moe (jebba)
Modified: 2010-12-28 13:19 EST (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-05-19 12:49:12 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jeff Moe (jebba) 2008-08-19 00:57:24 EDT
This is my first Fedora package so I need a sponsor.


Spec URL:
ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora/fmit.spec


SRPM URL:
ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora/fmit-0.97.7-1.fc9.src.rpm


Description: Free Music Instrument Tuner!  :)

Free Music Instrument Tuner features error history, volume history, wave shape
harmonic ratios, microtonal tuning (with Scala file support), and a discrete
Fourier transform view.
Comment 1 Kevin Fenzi 2008-08-24 17:08:45 EDT
Hey Jeff. 

I'll take a look at this package and see about sponsoring you...

Do you have another package or two to submit? 
Or would you be willing to go do some "pre-reviews" of waiting packages in the review queue to show you know the guidelines?

Look for a full review of this package in the next few days...
Comment 2 Jeff Moe (jebba) 2008-08-24 20:13:54 EDT
I'm going to take a look at #454008 (iax), #454010 (iaxclient), #454022 (Coccinella) since I'm interested in them and once took a look at packaging Coccinella and was thwarted. haha. I have packaged synfig (#428568) in the past so I'll look at that and whatever else looks interesting.

I have packaged a number of things which may be of interest to fedora. All of the .specs would need to be cleaned up (some significantly...). Partial list: freej, jamin,  daemontools, ffmpeg2theora, flumotion, freeeee-encyclopedia-en, freeeee-encyclopedia-reader, ihu, indywiki, libclaw, mlt, mlt++, kdenlive, oggfwd, palantir, plee-the-bear, qdvdauthor, ucspi-tcp, gastman, kiax, hasciicam, jabbin, jahshaka, kiax, lives, MuSE, netjack, thoggen, wengophone, xoo, qtnx, pocketphinux, PenguinTV, openlibraries, openmovieeditor, sphinxbase plus a variety multimedia applications which are often found on freeworld servers (mplayer, vlc, etc).

If there's anything in particular there that you'd like to see, I'll prepare it. I'll start the process of cleaning them up...
Comment 3 Rex Dieter 2008-08-25 09:00:36 EDT
fyi, keep in mind most/all of those "freeworld" items are done so for good reason, in that they contain encumbered (ie patented) content, and cannot be in fedora (at least not without surgery to remove the offending bits).
Comment 4 Jeff Moe (jebba) 2008-08-25 10:02:17 EDT
I did a package for mlt "Toolkit for broadcasters, video editors, media players, transcoders" #459979. I believe it meets Fedora's guidelines--I didn't drag in ffmpeg and such, for example. It is needed for mlt++ which is needed for kdenlive which rocks. It doesn't compile on x86_64 (some assembly optimization compile fail), but uploaded it anyway to get input.
Comment 5 Kevin Fenzi 2008-08-31 14:41:53 EDT
Sorry for the delay here... 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
See below - License
See below - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
d8ec5246602b8933839fa8ec0d37df05  fmit-0.97.7.tar.bz2
d8ec5246602b8933839fa8ec0d37df05  fmit-0.97.7.tar.bz2.orig
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
See below - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Package is a GUI app and has a .desktop file

OK - Package compiles and builds on at least one arch.  
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
OK - No rpmlint output.
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should have dist tag
OK - Should package latest version

Issues:

1. The files here are all LGPLv2+ and GPLv2+ (they all have the 'or later' clause).
So, the entire package as a whole would be 'GPLv2+' I think.
Do you agree?

2. You need a
rm -rf $RPM_BUILD_ROOT
at the top of your %install section.

3. Any reason you are shipping your own desktop file instead of using
the one provided by upstream?
Comment 6 Jeff Moe (jebba) 2008-09-01 18:55:06 EDT
1. Changed to GPLv2+

2. Added: rm -rf $RPM_BUILD_ROOT

3. Well, theirs would have to be modified anyway (e.g. their icon doesn't exist, no Qt category, etc.). I could just generate a fresh one from within the spec. Anyway, I didn't change anything here, but I'll do it as you like. :)

Updated packages here:
ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora

Thanks!
Comment 7 Kevin Fenzi 2008-09-06 22:48:23 EDT
Note: it's a good idea to bump the release and add a changelog entry for even changes made during review (this makes sure to track changes and make sure that the reviewer has the latest version). :)

Humm.. this version seems to have dropped out these two lines: 
< make DESTDIR=$RPM_BUILD_ROOT install
< rm $RPM_BUILD_ROOT%{_libdir}/*.a

Can you doublecheck what is going on there? You likely do still need to install and remove static libs... :)
Comment 8 Chris Weyl 2008-09-30 16:41:40 EDT
(In reply to comment #2)

> I have packaged a number of things which may be of interest to fedora. All of
> the .specs would need to be cleaned up (some significantly...). Partial list:
> freej, jamin,  daemontools, ffmpeg2theora, flumotion, freeeee-encyclopedia-en,
> freeeee-encyclopedia-reader, ihu, indywiki, libclaw, mlt, mlt++, kdenlive,
> oggfwd, palantir, plee-the-bear, qdvdauthor, ucspi-tcp, gastman, kiax,
> hasciicam, jabbin, jahshaka, kiax, lives, MuSE, netjack, thoggen, wengophone,
> xoo, qtnx, pocketphinux, PenguinTV, openlibraries, openmovieeditor, sphinxbase
> plus a variety multimedia applications which are often found on freeworld
> servers (mplayer, vlc, etc).
> 
> If there's anything in particular there that you'd like to see, I'll prepare
> it. I'll start the process of cleaning them up...

As a side note, I'd love to see daemontools making it into Fedora :)
Comment 9 Jeff Moe (jebba) 2008-10-02 14:10:35 EDT
/me looks at calendar

Ok, I updated fmit.spec and fmit-0.97.7-2.fc9.src.rpm with updates per Kevin's #7 comments.

ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora
Comment 10 Ralf Corsepius 2008-10-03 02:54:37 EDT
Any particuliar reasons for these Requires:
 
Requires:	alsa-lib >= 0.9
Requires:	qt3 fftw freeglut jack-audio-connection-kit

They seem all redundant and superfluous to me.
Comment 11 Jeff Moe (jebba) 2008-10-03 13:40:44 EDT
Removed Requires: per #10, at version fmit-0.97.7-3.fc9

ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora

Thanks. :)
Comment 12 Kevin Fenzi 2008-10-05 14:37:09 EDT
Thanks for the comment Ralf. 

The package from comment #11 looks good to me, I don't see any further blockers, so this package is APPROVED. 

I'll go ahead and sponsor you jebba. 
Continue the process from: 
http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account
Just let me know what your fedora account is so I can sponsor you. 

Please let me know if you have any questions or issues either here, in email or on irc. 

Thanks!
Comment 13 Jeff Moe (jebba) 2008-10-05 15:34:04 EDT
My fedora account is jebba too (as in here and irc). I already have a fedora account and am cla_done.

Just now I applied here (Fedora Packager CVS Commit Group):
https://admin.fedoraproject.org/accounts/group/list?search=packager*

Thanks kevin!
Comment 14 Mamoru TASAKA 2008-10-05 22:35:58 EDT
(In reply to comment #11)
> Removed Requires: per #10, at version fmit-0.97.7-3.fc9

Well,
* I am not sure why this srpm has BuildRequires "qt-devel" (this is qt4) and 
  "qt3-designer" (this is qt3)
* Fedora specific compilation flags are not honored:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=862771
  https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
* When using "cp" or "install" commands, add "-p" option to keep
  timestamps on installed files:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps
Comment 15 Kevin Fenzi 2008-10-09 18:44:07 EDT
jebba: Can you address the items from comment #14? 
The compile flags may need a Makefile patch. ;( 

Thank you Mamoru for spotting those.
Comment 16 Jeff Moe (jebba) 2008-10-13 16:24:43 EDT
* Mon Oct 13 2008 jeff <moe@blagblagblag.org> - 0.97.7-4
- cp -p fmit.svg
- BuildRequires: portaudio-devel

Re #14:
There is no qt-designer for qt4 and fmit needs it.

I'm not sure why the compile flags aren't honored. I'll take a further look at it, but thought I'd add what I have here now.

Added "cp -p"

===============

I also added a portaudio dep--I can remove this, but I figured I might as well enable the feature as someone may want it.


Thanks again! :)


ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora/fmit.spec
Comment 17 Rex Dieter 2008-10-13 16:38:09 EDT
> There is no qt-designer for qt4 and fmit needs it.

yes there is, it's in the qt-devel (F9+), or qt4-devel (F8+).  That said, designer is a development tool, I've never seen it required to build anything.
Comment 18 Jeff Moe (jebba) 2008-10-13 17:21:27 EDT
* Mon Oct 13 2008 jeff <moe@blagblagblag.org> - 0.97.7-5
- %%configure --enable-debug


Using --enable-debug addresses the FLAGS problem, but I'm not sure if that is the best way to do it.


The other option would be patching out the "-O3 -ffast-math" in configure.ac and exporting $CPPFLAGS (if needed).

configure.ac snippet:
==================================
AC_ARG_ENABLE(debug,            [  --enable-debug       turn on debugging (default=no)])
if test "x$enable_debug" = "xyes"; then
        CXXFLAGS="$CPPFLAGS -g -DDEBUG"
        AC_MSG_RESULT(yes)
else
        CXXFLAGS="$CPPFLAGS -O3 -ffast-math"
        AC_MSG_RESULT(no)
fi
==================================



As for the 'BuildRequires: qt3-designer' question, without it, the build bombs thusly:
==================================
Making all in ui
make[2]: Entering directory `/builddir/build/BUILD/fmit-0.97.7/ui'
ConfigForm.ui > ConfigForm.h
InstrumentTunerForm.ui > InstrumentTunerForm.h
/bin/sh: ConfigForm.ui: command not found
/bin/sh: InstrumentTunerForm.ui: command not found
make[2]: *** [ConfigForm.h] Error 127
==================================

I'm guessing it is something that upstream should be running before they make the tarball, but i'm not certain.

Latest:
ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora/fmit.spec
ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora/fmit-0.97.7-5.fc9.src.rpm
Comment 19 Jeff Moe (jebba) 2008-10-13 20:13:09 EDT
New Package CVS Request
=======================
Package Name: fmit
Short Description: Free Music Instrument Tuner
Owners: jebba
Branches: F-9
InitialCC: jebba kevin
Comment 20 Mamoru TASAKA 2008-10-14 00:55:17 EDT
(In reply to comment #16)
> Re #14:
> There is no qt-designer for qt4 and fmit needs it.

What I am asking is why this package needs both qt4 and qt3.
As far as I checked the build log (for example:
http://koji.fedoraproject.org/koji/getfile?taskID=862773&name=build.log )
this package needs libqt-mt.so.3, which is in qt3 and not in qt(4)
and the rebuilt binary rpm does not seem to have dependencies for qt4 packages.

More explicit question is "Is qt(4)-devel really required for BuildRequires?"
Comment 21 Mamoru TASAKA 2008-10-14 01:00:31 EDT
(In reply to comment #18)
> The other option would be patching out the "-O3 -ffast-math" in configure.ac
- I recommend to patching these out.

> As for the 'BuildRequires: qt3-designer' question, without it, the build bombs
- This is because qt3-designer pulls qt3-devel in, and without qt3-devel
  this package won't build. Also here I guess this package is using qt3, not qt4.
Comment 22 Jeff Moe (jebba) 2008-10-16 22:27:27 EDT
* Thu Oct 16 2008 jeff <moe@blagblagblag.org> - 0.97.7-6
- Add fmit-0.97.7-configure.patch
- Add BuildRequires: qt3-devel libXi-devel
- No BuildRequires: portaudio-devel qt-devel qt-designer
- No %%configure --enable-debug


I see where I earlier went astray. Earlier the QT test done by ./configure was failing. The reason for this was not qt*-devel per se, but a missing BuildRequires: for libXi-devel.

The package is building with correct FLAGS now. The one liner patch simply comments out the line that reset the CXXFLAGS. Thanks for your help! :)

ftp://ftp.blagblagblag.org/pub/BLAG/developers/jebba/jebbadora/
Comment 23 Mamoru TASAKA 2008-10-17 13:16:38 EDT
looks good.
Comment 24 Jeff Moe (jebba) 2008-10-19 14:06:13 EDT
I flagged this to "fedora‑cvs?" a couple days ago or so. It had been flagged "fedora-cvs+", but it may have been me that flagged it that way instead of "?" accidentally. Anyway, I *think* I've done everything I need to do to get this package into Fedora and am waiting on upstream to make the CVS module (when trying to check it out, I get "Error: cvs server: cannot find module `fmit' - ignored").

In short, the ball is in some fedora-admin's court, I believe. But if there is something I missed that I need to do first, let me know.

Mamoru Tasaka thanks again for all your help. :)
Comment 25 Kevin Fenzi 2008-10-19 18:43:16 EDT
yes, if it's tagged + no cvs admin will see it. ;) 

Sorry for the delay... 

cvs done.
Comment 26 Kevin Fenzi 2009-01-14 00:43:03 EST
Hey Jebba. 

Sorry this dropped off my radar, and it sure seems to have dropped off yours as well. ;) 

CVS was done, but I don't see the package imported or built. 
Can you please do so, so we can close this out?
Comment 27 Henrique "LonelySpooky" Junior 2009-05-11 21:32:37 EDT
*** Bug 500277 has been marked as a duplicate of this bug. ***
Comment 28 Kevin Fenzi 2009-05-12 13:22:23 EDT
Well, this package was setup, but never imported or built. ;( 
It was also recently orphaned because of an inactive maintainer. ;( 

So, perhaps we should close this one, and you can re-open your submission?
(You might look at the package here to see if there is any way to improve your submission).
Comment 29 Henrique "LonelySpooky" Junior 2009-05-12 13:39:05 EDT
Yes, thanks!
I'll try to contact jebba first and see if he is around; if no, I'll be happy to take this package and re-open my review request.
Comment 30 Jeff Moe (jebba) 2009-05-16 17:26:11 EDT
Heh. I just got a flood of messages since my email addr just came back online, one of which was about this bug....

I took a "break" in November 2008 and it got turned into a much longer break when my computers were stolen from my house. Gar. I am back online again, but I am not used Fedora or its derivatives (<cough>), so Henrique feel free to take over! 

Oh, and the files referenced above, if needed, can be had (for a limited time only) at:
ftp://216.17.145.32/pub/BLAG/developers/jebba/jebbadora/

Thanks!
Comment 31 Kevin Fenzi 2009-05-18 19:54:22 EDT
Jeff! Glad to hear you are still around. ;) 

Whatever you guys decide to do is fine with me. I would be happy to work with Jeff to get this imported and built, or let Henrique submit a new one.
Comment 32 Henrique "LonelySpooky" Junior 2009-05-18 20:06:14 EDT
Hi jebba, please, feel free to continue your great job with this package. I'm still learning how to package, so,I think you'll be a better maintainner than me.
Glad to hear you are still around. ;) [2]
Comment 33 Jeff Moe (jebba) 2009-05-18 22:36:01 EDT
Heh. Well, i guess i wasnt quite clear enough... I dont have (or really wont have real-soon-now) access to Fedora(-ish) boxes so I wont be able to maintain this package. Sorry about that. ¡Back to you Henrique! You should be able to take that spec and patches and be able to build/import fine. It shouldnt require any further changes at the present. Have fun! :)
Comment 34 Henrique "LonelySpooky" Junior 2009-05-19 06:41:14 EDT
Hmmm... So, I'll take it. If it is ok to you  I'll use your spec since this is already approved.

(In reply to comment #33)
> Heh. Well, i guess i wasnt quite clear enough... I dont have (or really wont
> have real-soon-now) access to Fedora(-ish) boxes so I wont be able to maintain
> this package. Sorry about that. ¡Back to you Henrique! You should be able to
> take that spec and patches and be able to build/import fine. It shouldnt
> require any further changes at the present. Have fun! :)
Comment 35 Kevin Fenzi 2009-05-19 12:49:12 EDT
ok. I think the best way forward is to close this, and re-open the other submission for review. 
:) 

Thanks guys!
Comment 36 Jason Tibbitts 2010-12-28 13:19:27 EST

*** This bug has been marked as a duplicate of bug 665995 ***

Note You need to log in before you can comment on or make changes to this bug.