[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 Mattias Ellert mattias.ell...@fysast.uu.se changed: What|Removed |Added Flag|fedora-review? |fedora-review+ --- Comment #11 from Mattias Ellert mattias.ell...@fysast.uu.se 2010-07-21 02:04:10 EDT --- (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. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 Mattias Ellert mattias.ell...@fysast.uu.se changed: What|Removed |Added Flag||fedora-review? --- Comment #8 from Mattias Ellert mattias.ell...@fysast.uu.se 2010-07-20 10:51:53 EDT --- 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 -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #10 from Orcan 'oget' Ogetbil oget.fed...@gmail.com 2010-07-21 01:12:17 EDT --- (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 -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #9 from Mattias Ellert mattias.ell...@fysast.uu.se 2010-07-21 01:10:22 EDT --- (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. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 Mattias Ellert mattias.ell...@fysast.uu.se changed: What|Removed |Added Status|NEW |ASSIGNED AssignedTo|nob...@fedoraproject.org|mattias.ell...@fysast.uu.se -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #7 from Orcan 'oget' Ogetbil oget.fed...@gmail.com 2010-07-18 15:18:11 EDT --- 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. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #5 from Orcan 'oget' Ogetbil oget.fed...@gmail.com 2010-07-17 13:18:02 EDT --- 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 -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #6 from Orcan 'oget' Ogetbil oget.fed...@gmail.com 2010-07-17 22:44:40 EDT --- 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. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 Orcan 'oget' Ogetbil oget.fed...@gmail.com changed: What|Removed |Added Depends on||614692 -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #4 from Orcan 'oget' Ogetbil oget.fed...@gmail.com 2010-06-02 22:42:45 EDT --- Hi, So what is the next step? Do I need to ask FESCO about the inclusion of these modified libraries in clementine? -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #2 from Orcan 'oget' Ogetbil oget.fed...@gmail.com 2010-05-08 20:23:08 EDT --- 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? -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #3 from Orcan 'oget' Ogetbil oget.fed...@gmail.com 2010-05-08 20:24:48 EDT --- What do you folks need? s/need/think/ -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 --- Comment #1 from Orcan 'oget' Ogetbil oget.fed...@gmail.com 2010-05-07 04:31:53 EDT --- 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. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 Rex Dieter rdie...@math.unl.edu changed: What|Removed |Added CC||rdie...@math.unl.edu Bug 583327 depends on bug 577601, which changed state. Bug 577601 Summary: Review Request: libqxt - Qt extension library https://bugzilla.redhat.com/show_bug.cgi?id=577601 What|Old Value |New Value Status|ASSIGNED|ON_QA Resolution||ERRATA Status|ON_QA |CLOSED -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 583327] Review Request: clementine - A music player and library organiser
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=583327 Orcan 'oget' Ogetbil oget.fed...@gmail.com changed: What|Removed |Added Depends on||577601, 581220 -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
