Bug 492715
Summary: | Review Request: KRadio4 - V4L/V4L2-Radio Application for KDE4 | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Paulo Roma Cavalcanti <promac> | ||||
Component: | Package Review | Assignee: | Zarko (grof) <zarko.pintar> | ||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | low | ||||||
Version: | rawhide | CC: | fedora-package-review, gwync, notting, opensource, zarko.pintar | ||||
Target Milestone: | --- | Flags: | zarko.pintar:
fedora-review+
j: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
URL: | http://kradio.sourceforge.net/ | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2009-06-04 23:12:35 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: | |||||||
Attachments: |
|
Description
Paulo Roma Cavalcanti
2009-03-28 18:32:01 UTC
New link: SRPM URL: http://people.atrpms.net/~pcavalcanti/srpms/kradio4-4.0.0-0.1.r778.20090322.fc10.src.rpm Another update: SRPM URL: http://people.atrpms.net/~pcavalcanti/srpms/kradio4-4.0.0-0.6.r829.20090411.fc10.src.rpm Do you intentionally not use %cmake? Reference: https://fedoraproject.org/wiki/Packaging/cmake You should use an adapted version of this generic, recommended Sourceforge URL: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz Reference: https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net Fixed. Thanks. SRPM URL: http://people.atrpms.net/~pcavalcanti/srpms/kradio4-4.0.0-0.7.r829.20090411.fc10.src.rpm Note about ffmpeg: If KRadio4 can works nice without ffmpeg and lame than you can put it on Fedora, but if this application is useless (or mostly useless) without these dependencies, than put it on RPM Fusion! (In reply to comment #5) > Note about ffmpeg: > > If KRadio4 can works nice without ffmpeg and lame than you can put it on > Fedora, but if this application is useless (or mostly useless) without these > dependencies, than put it on RPM Fusion! It works perfectly without ffmpeg, which is only needed for Internet radio. For using only an FM/TV capture card, there is no need of ffmpeg/libmms, and it will work the same way as gnomeradio. The current version supports pulseaudio: SRPM URL: http://dl.atrpms.net/src/f10-i386/atrpms/stable/kradio4-4.0.0-0.9.r883.20090514.src.rpm Yes, but internet radio is something very useful and I do not know how is smart build this application without internet radio support! Personally, I know more people who use internet radio from those who use FM capture cards... BTW, are you sponsored packager? If you are, than I can do review ;) Yes I am. Please, go ahead .... I also spent days with the developer for removing bugs and adding pulseaudio support. It would be nice if you had a capture card to test all the aspects of the software, which is really nice. (In reply to comment #7) > Yes, but internet radio is something very useful and I do not know how is smart > build this application without internet radio support! > > Personally, I know more people who use internet radio from those who use FM > capture cards... I am one of those people.... I use kradio4 in several computers without a capture card. But the Internet radio is just a plugin, which could go in a separate package kradio4-nonfree something, I guess. (In reply to comment #10) > > But the Internet radio is just a plugin, which could go in > a separate package kradio4-nonfree something, I guess. That is good. Could you make this separate package in the near future? I will today, tomorrow take a look of spec, OK? (In reply to comment #11) > (In reply to comment #10) > > > > But the Internet radio is just a plugin, which could go in > > a separate package kradio4-nonfree something, I guess. > > > That is good. Could you make this separate package in the near future? Sure. > > I will today, tomorrow take a look of spec, OK? Thanks. Take your time. To clarify, the nonfree plugin will be going to rpmfusion, correct? (In reply to comment #13) > To clarify, the nonfree plugin will be going to rpmfusion, correct? Of course. I know it cannot be here. I figured as much, just making sure all the Rs are dotted and the Ms are crossed. ;) Created attachment 345864 [details]
Kdenlive's specfile
OK, I assigned this for review, but we have here some issues in spec file: 1. Please purify spec file. I mean that you drop your macro definitions for snapshot and ffmpeg. ffmpeg must be droped out from spec file! 2. This version is snapshot. Can you expect stable release soon? 3. I think that you do not need these BR-s: -chrpath (RPATHS resolved by %cmake_kde4 macro -gcc-c++ -qt-devel Try to build without them 4. Please, try to use BuildRoot in form of this: BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) 5. Please use kde4 macros for cmake, dirs, (I'll attach kdenlive.spec as example) 6. .desktop file: Why you wrote .desktop file? Isn't a author's job? 7. Why you copy icons from /icons dir to /pixmaps? 8. %defattr must be %defattr(-,root,root,-) 9. {_datadir}/icons/hicolor/*/*/* you can write on this way: {_datadir}/icons/hicolor/* 10.Please, try to build this package with Koji (or local Mock) for F10 and F11 branches. It must be successfully! (In reply to comment #17) > OK, I assigned this for review, but we have here some issues in spec file: > > 1. Please purify spec file. I mean that you drop your macro definitions for > snapshot and ffmpeg. ffmpeg must be droped out from spec file! I will do it. I just kept them for now so I can fully test the resulting rpm. > > 2. This version is snapshot. Can you expect stable release soon? I think so. It was promised sometime ago, but maybe the author is trying to provide a better pulse support (I suggested him to talk to Lennard). > > 3. I think that you do not need these BR-s: > -chrpath (RPATHS resolved by %cmake_kde4 macro > -gcc-c++ > -qt-devel > Try to build without them They are not needed. I just used qt-devel to specify the minimum version. > > 4. Please, try to use BuildRoot in form of this: > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} > -n) > Done. I should have missed the %{version} in a cut-and-paste action. > 5. Please use kde4 macros for cmake, dirs, (I'll attach kdenlive.spec as > example) > Done. > 6. .desktop file: > Why you wrote .desktop file? Isn't a author's job? I did not find any desktop entry, so I wrote one. > > 7. Why you copy icons from /icons dir to /pixmaps? Not really necessary. Removed. > > 8. %defattr must be %defattr(-,root,root,-) Done. > > 9. {_datadir}/icons/hicolor/*/*/* you can write on this way: > {_datadir}/icons/hicolor/* > Would I not have been claiming ownership of the intermediate directories this way? > 10.Please, try to build this package with Koji (or local Mock) for F10 and F11 > branches. It must be successfully! It is building just fine. SPEC: http://orion.lcg.ufrj.br/RPMS/SPECS/kradio4.spec SRPM: http://orion.lcg.ufrj.br/RPMS/src/kradio4-4.0.0-0.10.r883.20090514.fc10.src.rpm Thanks. - I thin that you do not need this: ============================================================= update-mime-database %{_kde4_datadir}/mime &> /dev/null || : ============================================================= This is neded only if application drops an XML file in %{_datadir}/mime/packages. Please, can you explain this: ============================================= unset QTINC QTLIB QTPATH_LRELEASE QMAKESPEC export QT4DIR=%{_libdir}/qt4 export QTDIR=$QT4DIR PATH=$QT4DIR/bin:$PATH ; export PATH ============================================= Do we need this? (In reply to comment #19) > - I thin that you do not need this: > ============================================================= > update-mime-database %{_kde4_datadir}/mime &> /dev/null || : > ============================================================= > > This is neded only if application drops an XML file in > %{_datadir}/mime/packages. I will remove it. > > Please, can you explain this: > ============================================= > unset QTINC QTLIB QTPATH_LRELEASE QMAKESPEC > export QT4DIR=%{_libdir}/qt4 > export QTDIR=$QT4DIR > PATH=$QT4DIR/bin:$PATH ; export PATH > ============================================= > > Do we need this? If we build with mock, the answer is no, we do not need this. However, If a user try to rebuild the package, he could have set in his environment the variables for QT3, and the build will fail. (In reply to comment #20) > If we build with mock, the answer is no, we do not need this. > However, If a user try to rebuild the package, he could have set > in his environment the variables for QT3, and the build will fail. Hmm, I think that is not good idea to change users local environment with src.RPM file. I never seen before that type of case....and nobody want from me to adjust my spec files to users local environment... (In reply to comment #21) > (In reply to comment #20) > > If we build with mock, the answer is no, we do not need this. > > However, If a user try to rebuild the package, he could have set > > in his environment the variables for QT3, and the build will fail. > > > Hmm, I think that is not good idea to change users local environment with > src.RPM file. In fact, I am not changing his environment. The change only occurs in the shell used to build the rpm (which inherits the user variables). > > I never seen before that type of case....and nobody want from me to adjust my > spec files to users local environment... As I said, it can be removed. I am trying to create an internetradio package, with the plugin which requires ffmpeg. Would you mind taking a look at it, also? (In reply to comment #22) > > As I said, it can be removed. > OK, maybe it is a good idea. If someone have complaint about that, we can do an update of package with these lines... > I am trying to create an internetradio package, with the plugin which requires > ffmpeg. Would you mind taking a look at it, also? Sure, why not! :) This is the Fedora version, without ffmpeg: SPEC: http://orion.lcg.ufrj.br/RPMS/SPECS/kradio4.spec SRPMS: http://orion.lcg.ufrj.br/RPMS/src/kradio4-4.0.0-0.12.r883.20090514.fc10.src.rpm This is the full version, which also creates a kradio4-internetradio: SPEC: http://people.atrpms.net/~pcavalcanti/specs/kradio4.spec SRPMS: http://orion.lcg.ufrj.br/RPMS/src/kradio4-4.0.0-0.11.r883.20090514.fc10.src.rpm - We have trouble here: BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u}-n) MUST be: BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) (space between %{__id_u} and -n parameter), otherwise we get rpmlint errors. -rpmlint drops errors: kradio4.x86_64: E: wrong-script-interpreter /usr/share/kde4/apps/kradio4/default-dot-lircrc "lircrcd" kradio4.x86_64: E: non-executable-script /usr/share/kde4/apps/kradio4/default-dot-lircrc 0644 Can you examine this? (In reply to comment #25) > - We have trouble here: > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u}-n) > > MUST be: > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) > > (space between %{__id_u} and -n parameter), otherwise we get rpmlint errors. No problem. Fixed. Same URLS. > > -rpmlint drops errors: > > kradio4.x86_64: E: wrong-script-interpreter > /usr/share/kde4/apps/kradio4/default-dot-lircrc "lircrcd" > kradio4.x86_64: E: non-executable-script > /usr/share/kde4/apps/kradio4/default-dot-lircrc 0644 > > Can you examine this? This is not really a script. This is just a lirc configuration file, for using a remote control. [cascavel:~/specs] more /usr/share/kde4/apps/kradio4/default-dot-lircrc #! lircrcd begin button = * prog = kradio config = eventmap repeat = 1 end (In reply to comment #26) > > This is not really a script. This is just a lirc configuration file, > for using a remote control. OK then. I tested application with my TV/FM card, and works just fine! Please, see with upstream if the next release will be soon. I think that is wise to waiting for release (RC4 is out, so I concluded that the release is close :) ) > OK then. I tested application with my TV/FM card, and works just fine! > Good to know. kradio is really nice. You can import preset stations and add image logos for each one of them. > Please, see with upstream if the next release will be soon. > > I think that is wise to waiting for release (RC4 is out, so I concluded that > the release is close :) ) I talked to Ernst Whitte today, and he told me the final version is almost done. Therefore, it is really better to wait a little bit longer. Thanks. Just a quick update. The desktop entry has been added to the source. I built revision 889, which is very close to the final release. SPEC: http://orion.lcg.ufrj.br/RPMS/SPECS/kradio4.spec SRPMS: http://orion.lcg.ufrj.br/RPMS/src/fedora/kradio4-4.0.0-0.12.r889.20090531.fc10.src.rpm This is the full version, which also creates a kradio4-internetradio: SPEC: http://people.atrpms.net/~pcavalcanti/specs/kradio4.spec SRPMS: http://orion.lcg.ufrj.br/RPMS/src/kradio4-4.0.0-0.12.r889.20090531.fc10.src.rpm Both have the same version, and therefore, any 3th party repo can build the full version and post just kradio4-internetradio rpm. Hi, Zarko kradio4 4.0.0 final is out! SPEC: http://orion.lcg.ufrj.br/RPMS/SPECS/kradio4.spec SRPMS: http://orion.lcg.ufrj.br/RPMS/src/fedora/kradio4-4.0.0-1.fc10.src.rpm This is the full version, which also creates a kradio4-internetradio: SPEC: http://people.atrpms.net/~pcavalcanti/specs/kradio4.spec SRPMS: http://orion.lcg.ufrj.br/RPMS/src/kradio4-4.0.0-1.fc10.src.rpm I am looking forward for your final comments too. Thanks. Let's do some cosmetics: 1) Move out these and correct other spec code to works without these definitions: ------------------------------- #define rel snapshot-2009-05-31-r889 #define rel2 0.12.r889.20090531 %define rel %{version} %define rel2 1 -------------------------------- * 2) Comment included %patch0 and %source1 files! 3) %setup line can be: %setup -q -a 1 4) %cmake can be: %cmake_kde4 5) after removing %define lines you must change: cp -R %{_builddir}/%{name}-%{rel}/html/* \ to cp -R %{_builddir}/%{name}-%{version}/html/* \ 6) .desktop file do not contain mimetype key, so we can now remove these lines from %post and %postun: update-desktop-database &> /dev/null || : (This is my mistake ;) ) 7) Maybe the most important thing! Are you sure that we can make package for internet radio as a nonfree package, and just install them to Kradio works properly, without need to rebuild (compile) Kradio4 with needed BR: -devel packages??? Because, if we must build Kradio4 package with nonfree -devel dependencies, than Kradio4 must fo to RPM Fusion for full functionality. I ask because I saw that builder, at configure time ask for these nonfree devel dependencies... Can you chack this on some way? > > 4) %cmake can be: %cmake_kde4 I was already using %cmake_kde4. Where did you see only %cmake?? > > 5) after removing %define lines you must change: > > cp -R %{_builddir}/%{name}-%{rel}/html/* \ > to > cp -R %{_builddir}/%{name}-%{version}/html/* \ > > 6) .desktop file do not contain mimetype key, so we can now remove these lines > from %post and %postun: > > update-desktop-database &> /dev/null || : > > (This is my mistake ;) ) > > 7) Maybe the most important thing! > > Are you sure that we can make package for internet radio as a nonfree package, > and just install them to Kradio works properly, without need to rebuild > (compile) Kradio4 with needed BR: -devel packages??? > > Because, if we must build Kradio4 package with nonfree -devel dependencies, > than Kradio4 must fo to RPM Fusion for full functionality. > > I ask because I saw that builder, at configure time ask for these nonfree devel > dependencies... It checks for them, but the build goes just fine this way. > > Can you chack this on some way? Well, I am using kradio4 exactly this way. I built and installed the Fedora package, and then installed only the internet radio package. It is working just fine. I made all the modifications, but I kept the release as 1, If you do not mind, because I would like to deploy the Fedora package using 1 for its first release. For now I only update the Fedora package. Same URLS. If you are using F10 x86_64, I have the packages here: http://orion.lcg.ufrj.br/RPMS/myrpms-f10-x86_64/repoview/kradio4.html http://orion.lcg.ufrj.br/RPMS/myrpms-f10-x86_64/repoview/kradio4-internetradio.html The first one is the Fedora version. If you install just it, you will not be able to play internet stations. After installing the second one, you will. The first one was generated with mock, and no internetradio package was created in the process. The second one was created using the full .src.rpm. This is the build log of the Fedora package: http://orion.lcg.ufrj.br/temp/build.log As you can see, no ffmpeg or libmms available, but the combination of the two packages gives us the full functionality. (In reply to comment #32) > > > > 4) %cmake can be: %cmake_kde4 > > I was already using %cmake_kde4. Where did you see only %cmake?? I'm talking about drop parameters right from %cmake_kde4 (path parameter in this case ;) ) > > I ask because I saw that builder, at configure time ask for these nonfree devel > > dependencies... > > It checks for them, but the build goes just fine this way. Yes, but how you know that compiler, if do not find headers from these -devel dependencies, will not compile an essential part of the code that allows the later use of codes from run-time dependencies? > Well, I am using kradio4 exactly this way. I built and installed the Fedora > package, and then installed only the internet radio package. It is working just > fine. OK then, it will be enough :) > > For now I only update the Fedora package. Same URLS. This is OK, because this bug thread isn't for debating about forbidden staff. These we can discuss on RPM Fusion, when you prepare package ;) (In reply to comment #33) > > As you can see, no ffmpeg or libmms available, but the combination of the two > packages gives us the full functionality. OK, sorry on bothering but this is important thing to clean up before build package for Fedora repo... I checked once again your spec and now looks good :) (for nonfree package please send me a link to RPM Fusion.) Now, I APPROVED. (In reply to comment #35) > (In reply to comment #33) > > > > As you can see, no ffmpeg or libmms available, but the combination of the two > > packages gives us the full functionality. > > > OK, sorry on bothering but this is important thing to clean up before build > package for Fedora repo... No. The test was really necessary. That is why I used mock for makeing sure there was no trace of either ffmpeg or libmms during the build. And you are right. The parameter following %cmake_kde4 is not really necessary. Removed (same URLS). The only thing is that the versions and releases must be the same for Fedora and rpmfusion. That is why I want to start in 1, so I can keep track in my mind where we begun, without having to look in the changelog section. (In reply to comment #36) > I checked once again your spec and now looks good :) > (for nonfree package please send me a link to RPM Fusion.) > > Now, I APPROVED. Thanks a lot for all your help! KRadio4 is a great program and it deserve to be in fedora repo ;) BTW, do you maybe want to review my packages of btnx and btnx-conf programs? (daemon for using more than three mouse buttons) https://bugzilla.redhat.com/show_bug.cgi?id=502591 https://bugzilla.redhat.com/show_bug.cgi?id=502592 (In reply to comment #39) > KRadio4 is a great program and it deserve to be in fedora repo ;) > > BTW, do you maybe want to review my packages of btnx and btnx-conf programs? > (daemon for using more than three mouse buttons) > > https://bugzilla.redhat.com/show_bug.cgi?id=502591 > https://bugzilla.redhat.com/show_bug.cgi?id=502592 Of course. I have been using imwheel for a similar purpose, but it is currently unmaintained. Maybe it is time for getting a replacement ... Just give some time to install and learn how to use the package. CVS Request must looks like this: New Package CVS Request ======================= Package Name: kradio4 Short Description: V4L/V4L2-Radio Application for KDE4 Owners: your FAS name Branches: F-9 F-10 F-11 InitialCC: your FAC name and fedora-cvs flag MUST set to: ? (not to plus sign, plus sign will give CVS admin) please, correct this... Look here for details: https://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure New Package CVS Request ======================= Package Name: kradio4 Short Description: V4L/V4L2-Radio Application for KDE4 Owners: roma Branches: F-9 F-10 F-11 InitialCC: roma Thanks Zarko. I must have a full night of sleep ... CVS done. |