Spec URL: http://6mata.com:8014/review/clementine.spec SRPM URL: http://6mata.com:8014/review/clementine-0.2-2.fc12.src.rpm Description: Clementine is a modern music player and library organiser. It is largely a port of Amarok 1.4, with some features rewritten to take advantage of Qt4. --- This is an initial attempt to build clementine on Fedora. It builds and runs fine. However, the way we patch out the bundled libraries and use the system libraries might change, depending on #581220, #582864, #577601.
New vercione! runs and works magnifico. Spec URL: http://6mata.com:8014/review/clementine.spec SRPM URL: http://6mata.com:8014/review/clementine-0.3-1.fc12.src.rpm However, the package is so not ready with linking to qxt etc. I'll have time to sort things out next week. qtsingleapplication is not done with the review, we are not in a hurry.
Alright. I had a discussion with a clementine developer. http://code.google.com/p/clementine-player/issues/detail?id=291 What he says is they made such API breaking modifications in qxt and qtsingleapplication that are not upstreamable. So far I have been using clementine-0.2 built against our own qxt and own-to-be qtsingleapplication and I didn't see any regressions. That is perhaps because I didn't try to use the functionality that is provided by these modified libraries. Or perhaps the modifications are made after 0.2 releasei I didn't have much time to play around with 0.3, I just verified it can do the basic things. So, in particular, quoting the developer: * qxt: changes are to add support for media keys (play, stop, etc.) in the global shortcut library. I've included a private Qt header in there too, so these probably wouldn't get accepted upstream. * qtsingleapplication: changes are twofold: there's an obscure compilation fix for cross-compilation with mingw in release mode, which we could send upstream, but the other one is a compatibility-breaking API change. * universalchardet (new bundled library with 0.3): modified to remove its dependencies on Mozilla. It's not currently provided separately so the only solution is to bundle the source with clementine. qtlsingleapplication and universalchardet are small libraries, so we won't waste too much space by keeping them bundled. qxt is large originally, but clementine only bundles a relatively small portion of it. What do you folks need? Can we make an exception to allow these modified libraries in clementine?
> What do you folks need? s/need/think/
Hi, So what is the next step? Do I need to ask FESCO about the inclusion of these modified libraries in clementine?
After a lot of discussion with upstream, we got the 3rd party software patches backwards compatible except one. We now have command line options for cmake to link to the system libraries. The one exception is qxt. I wrote a patch to use the system qxt. However, this means that the multimedia buttons on the keyboards will not work. Still much better than what we had previously. Note that this package Buldrequires qtsingleapplication and qtiocompressor which are in updates-testing for the time being. It also requires a not-yet-built package: libprojectM. The current libprojectM package has a bug in it that causes clementine to crash. I notified its maintainer, and sent a patch. SPEC: http://oget.fedorapeople.org/review/clementine.spec SRPM: http://oget.fedorapeople.org/review/clementine-0.4.2-1.fc13.src.rpm
SPEC: http://oget.fedorapeople.org/review/clementine.spec SRPM: http://oget.fedorapeople.org/review/clementine-0.4.2-2.fc13.src.rpm This update fixes a segfault because of missing font paths when linked to the system projectM.
SPEC: http://oget.fedorapeople.org/review/clementine.spec SRPM: http://oget.fedorapeople.org/review/clementine-0.4.2-3.fc13.src.rpm Better, more portable patch for splitting qxt. Patch sent upstream. Since all the dependencies are built and are in testing now, this package is ready to review.
Fedora Review clementine 2010-07-20 rpmlint output: $ rpmlint clementine-0.4.2-3.fc14.src.rpm \ clementine-0.4.2-3.fc14.x86_64.rpm \ clementine-debuginfo-0.4.2-3.fc14.x86_64.rpm clementine.src: W: invalid-url Source0: http://clementine-player.googlecode.com/files/clementine-0.4.2.tar.gz HTTP Error 404: Not Found clementine.x86_64: W: no-manual-page-for-binary clementine 3 packages and 0 specfiles checked; 0 errors, 2 warnings. Googlecode is infamous for triggering false 404 warnings in rpmlint. Not quite sure why - wget seems to work without problem every time I try. + Package is named according to guidelines + Specfile is named after the package - Guidelines say cmake projects should use "make VERBOSE=1": https://fedoraproject.org/wiki/Packaging/cmake + Package License tag GPLv3 and GPLv2+ is a Fedora approved license - All the source files say "or (at your option) any later version", so it would make sense to say GPLv3+ and GPLv2+ The universalchardet (bundled code) is MPLv1.1 or GPLv2+ or LGPLv2+. I guess we can choose to use the GPLv2+ in this case, which is alredy covered in the tag. + License file (COPYING) is included as %doc (this is the GPLv3 text) + Specfile is written in legible English + Source matches upstream: $ md5sum srpm/clementine-0.4.2.tar.gz clementine-0.4.2.tar.gz c6819b0d2a8324f1d686fb5a3b1d287b srpm/clementine-0.4.2.tar.gz c6819b0d2a8324f1d686fb5a3b1d287b clementine-0.4.2.tar.gz + Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=2329614 - libprojectM-devel is listed twice as BuildRequires (once with a version and once without) The build log says: -- Building engines: gst -- Skipping engines: vlc xine qt-phonon So you are currently not building the xine engine - but you have xine-lib-devel as a BuildRequires - is it needed in this case? If you try to enable more engines you get a big warning: > The following engines are NOT supported by clementine developers: > xine qt-phonon > > Don't post any bugs if you use them, fix them yourself! So I guess you have a reason for not building them for Fedora. Some other BuildRequires seems redundant too - did you remove those that are only required to build the bundled libraries you do not build anymore? E.g. glew-devel seems to only be used inside the libprojectM code. + No locale files installed. (The translation files are mangled into the main binary by Qt.) + No shared libraries This package must have been a nightmare w.r.t. bundled libraries. You have done an excellent job unbundling the source. Very good job and congratualations. What remains is the universalchardet code. This is tricky. Did you discuss this with some Fedora packaging people? Googling a bit shows that this code is used by quite many projects, and they all bundle it. It would be interesting to know how many packages that exist in Fedora and bundle the code. I don't know any reasonably fast way to figure that out though. By doing a repoquery I found one package that installs the universalchardet headers. In that package (codeblocks-devel) the code is not compiled into a separate library, but bundled with a lot of other stuff into a big library that has very many dependencies - so it is not really a good option to use if you only want to use the universalchardet. Installing the headerfiles for a bundled library the way codeblocks-devel does seems wrong anyway. In a perfect world, I think this would be best compiled as a shared library in a separate package, having a common maintained codebase for all users of the code. But you might have some arguments for treating this case special. ? How do you ensure ownership of /usr/share/icons/hicolor/64x64/apps? + No duplicate files + Permissions are sane and %file has %defattr + Macros are used consistently in the Specfile + %doc is not runtime essential + No headerfiles + No static libraries + No libtool archive + .desktop file verified using desktop-file-validate + Package does not own other's directories + Installed files have valid UTF8 filenames
(In reply to comment #8) > In a perfect world, I think this would be best compiled as a shared > library in a separate package, having a common maintained codebase for > all users of the code. But you might have some arguments for treating > this case special. Rather than creating a new source package, it might be a good idea to talk to the maintainers of the xulrunner package and ask them if that package could provide such a library.
(In reply to comment #8) > Fedora Review clementine 2010-07-20 > Thanks a lot for the review. > - Guidelines say cmake projects should use "make VERBOSE=1": > https://fedoraproject.org/wiki/Packaging/cmake > That makes the build logs, as one can guess, verbose. clementine's logs were verbose by default, but I added it, just in case things change in the future. > - All the source files say "or (at your option) any later version", so it > would make sense to say GPLv3+ and GPLv2+ > I changed it to GPLv3+ and GPLv2+ > - libprojectM-devel is listed twice as BuildRequires (once with a > version and once without) > My sloppiness. Removed. > So you are currently not building the xine engine - but you have > xine-lib-devel as a BuildRequires - is it needed in this case? > Nope, it is a leftover from my experiments. > If you try to enable more engines you get a big warning: > > > The following engines are NOT supported by clementine developers: > > xine qt-phonon > > > > Don't post any bugs if you use them, fix them yourself! > > So I guess you have a reason for not building them for Fedora. > If I remember correctly, they use different engines on different OS'. gst was the default, and it works, so I kept it. :) > Some other BuildRequires seems redundant too - did you remove those > that are only required to build the bundled libraries you do not build > anymore? E.g. glew-devel seems to only be used inside the libprojectM > code. > I think glew-devel is left from the days I was compiling the bundled libprojectM. I think that's the last one though. > This package must have been a nightmare w.r.t. bundled libraries. You > have done an excellent job unbundling the source. Very good job and > congratualations. > Yeah, it was a lot of work, constantly bugging the developers, trying to reason with other upstreams to have the patches accepted, etc. I am sure the review took you some time too. Thanks again for your patience. > > What remains is the universalchardet code. This is tricky. Did you > discuss this with some Fedora packaging people? > > Googling a bit shows that this code is used by quite many projects, > and they all bundle it. It would be interesting to know how many > packages that exist in Fedora and bundle the code. I don't know any > reasonably fast way to figure that out though. > > By doing a repoquery I found one package that installs the > universalchardet headers. In that package (codeblocks-devel) the code > is not compiled into a separate library, but bundled with a lot of > other stuff into a big library that has very many dependencies - so it > is not really a good option to use if you only want to use the > universalchardet. Installing the headerfiles for a bundled library the > way codeblocks-devel does seems wrong anyway. > > In a perfect world, I think this would be best compiled as a shared > library in a separate package, having a common maintained codebase for > all users of the code. But you might have some arguments for treating > this case special. > This library does not have a standalone release by itself. Hence there are no packages for it. People keep copying its code into their projects. I talked to a few people on fedora-devel on IRC. They say xulrunner, gnash etc, everybody is sweeping it under the carpet. We can always switch over if it becomes available as a package. > > ? How do you ensure ownership of /usr/share/icons/hicolor/64x64/apps? > By adding the relevant Requires :) Here is my update: SPEC: http://oget.fedorapeople.org/review/clementine.spec SRPM: http://oget.fedorapeople.org/review/clementine-0.4.2-4.fc13.src.rpm Changelog: 0.4.2-4 - Use: make VERBOSE=1 - License is GPLv3+ and GPLv2+ - Put BRs in alphabetical order - Remove redundant BRs: glew-devel, xine-lib-devel, and the extra libprojectM-devel - Add R: hicolor-icon-theme
(In reply to comment #10) > This library does not have a standalone release by itself. Hence there are no > packages for it. People keep copying its code into their projects. I talked to > a few people on fedora-devel on IRC. They say xulrunner, gnash etc, everybody > is sweeping it under the carpet. We can always switch over if it becomes > available as a package. I understand your point, and I will not block the review waiting for a universalchardet library to appear. As I said in my previous comment, I think the closest thing you could get to an upstream in Fedora is the xulrunner package (since this contains the full code tree from mozilla and not just a copy of this class). So any solution to the problem with bundled universalchardet code should at least involve the xulrunner maintainers. But as I said, I will not block the review over this issue, so ... ... package approved.
Thanks Mattias, I will definitey contact xulrunner maintainers about this. New Package CVS Request ======================= Package Name: clementine Short Description: A music player and library organizer Owners: oget Branches: F-12 F-13 InitialCC:
CVS done (by process-cvs-requests.py).
(In reply to comment #10) > (In reply to comment #8) > > - Guidelines say cmake projects should use "make VERBOSE=1": > > https://fedoraproject.org/wiki/Packaging/cmake > > > > That makes the build logs, as one can guess, verbose. clementine's logs were > verbose by default, but I added it, just in case things change in the future. The reason why clementine's logs were already verbose is that the %cmake macro has -DCMAKE_VERBOSE_MAKEFILE=ON switch. When the guidelines were first approved %cmake macro didn't have this switch; it is something that was added later but it looks like people have forgotten to update the guidelines. The spec file doesn't seem to run icon cache scriplets, but I think it should as it's putting files into %{_datadir}/icons/hicolor/. Also, one of the icons is named application-x-clementine.png which looks like it's an icon for application/x-clementine mime type. I didn't check the .desktop file if it really has mimetype entry, but if it does, then you should also add desktop-database updating scriplets.
ouch. I'll add the scriptlets. Thanks for pointing them out. And yes, the .desktop file has mimetypes
clementine-0.4.2-5.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/clementine-0.4.2-5.fc13
clementine-0.4.2-5.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/clementine-0.4.2-5.fc12
clementine-0.4.2-5.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update clementine'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/clementine-0.4.2-5.fc12
clementine-0.4.2-5.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update clementine'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/clementine-0.4.2-5.fc13
clementine-0.4.2-7.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update clementine'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/clementine-0.4.2-7.fc13
clementine-0.4.2-7.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update clementine'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/clementine-0.4.2-7.fc12
clementine-0.4.2-8.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update clementine'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/clementine-0.4.2-8.fc12
clementine-0.4.2-8.fc13 has been pushed to the Fedora 13 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update clementine'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/clementine-0.4.2-8.fc13
clementine-0.4.2-9.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
clementine-0.4.2-9.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.