Spec URL: http://www.tepsonic.org/files/fedora/tepsonic.spec SRPM URL: http://www.tepsonic.org/files/fedora/tepsonic-0.98.1-1.fc14.src.rpm Description: a simple, fast and lightweight multiplaform Qt audio player with collections, plugins and all the basic functions that every good player should have.
Some initial comments: The license seems to be GPLv3+: * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License * as published by the Free Software Foundation; either version 3 * of the License, or (at your option) any later version. Please don't put all the Build Requires in one line, since it's not very clear. There is no package called lastfmlib-devel in Fedora. The name should be liblastfm-devel and the package doesn't build for me. The source also contains a copy of qtx. Please make sure it isn't used. Maybe delete it. I noticed, the player also works with MySQL. Does that need any preparation? Please don't mix $RPMBUILDROOT and %{buildroot} -- simply opt for one. I guess, a full stop can replace %{_builddir}/%{name}-%{version} from the cmake call. /usr should be %{_prefix}, but first check, if they are all necessary. Please use %{_datadir}, to substitute for /usr/share -- also in the files section. The category list in the desktop file should terminate in a semicolon (desktop-file-validate). Probably put the %doc first in the files section (after %defattr).
The lastfmlib package (https://bugzilla.redhat.com/show_bug.cgi?id=666633) was just approved and I'm working on it. liblastfm is an another package. The Qxt source is needed to provide cross-platform global shortcuts. There is not whole Qxt source included, only the few files needed to provide the required functionality. Could you please provide an explanation about the replacement of %{_buildir}/%{name}-%{version} in cmake cmd? I'm not sure I understand it. Thank you
Ah, I didn't know about that package. Fedora has it's own libqxt. Please use that instead. cmake \ -DCMAKE_INSTALL_PREFIX=%{_prefix} \ -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_INSTALL_LIBRARY_DIR=%{_libdir}/%{name} \ . Notice the fullstop that stands for the current directory instead of stating its full path. Also notice %{_prefix}.
Thanks for the cmake. About the Qxt - the libqxt (or just the one class to be exact) is statically linked into the executable binary, so no duplicate libqxt.so will be produced. I can fix the CMakeLists.txt to try look after libqxt.so first and then eventually use the included sources, but it will appear first in 0.98.2.
The lastfmlib is now available in the updates-testing repository. I updated the .spec file fixing all the issues you mentioned. For MySQL I added qt-mysql to dependencies. I just left the qxt sources because of reasons mention in the comment above. Spec URL: http://www.tepsonic.org/files/fedora/tepsonic.spec SRPM URL: http://www.tepsonic.org/files/fedora/tepsonic-0.98.1-2.fc14.src.rpm
You forgot to make a changelog entry. The changelog should reflect the changes made to the spec file. You can avoid this and other glitches by running rpmlint on your spec file or src.rpm. That would have also saved you from this one: %{_datadir}/share/tepsonic --> %{_datadir}/%{name} Taking a look at desktop files with desktop-file-validate, the categories list doesn't end in a semicolon. sed -i 's/\(^Categories.*\)/\1;/g' %{name}.desktop %{_prefix}/share/ --> %{_datadir} Please use rm -f or rm -rf instead of the rm macro. Please patch CMakeLists.txt for qxt. If Fedora shipped a new version of qxt, it could be break. Please align "qt-mysql". It now uses a tabulator. The license is still stated as GPLv3, although the files say GPLv3+. Please also take a look at http://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files
I fixed the paths variables, desktop file using your sed, removed the %{rm} macro, fixed tabulator-issue, license and did some patching around Qxt and locale files (I also pushed the changes to upstream, so in next release it will work without the patches). I'm just not sure, if gettext should be a dependency when I'm using Qt's locale format, but I added it. Spec URL: http://www.tepsonic.org/files/fedora/tepsonic.spec SRPM URL: http://www.tepsonic.org/files/fedora/tepsonic-0.98.1-3.fc14.src.rpm
No, Gettext is only necessary if the project uses .po/.mo. The Phonon Requires is also not necessary, as RPM finds that on it's own. I erroneously checked the original desktop file the first time. You can delete the sed call, because --add-category solves the syntax error. But "application" is regarded as invalid, so: desktop-file-install --dir=%{buildroot}/%{_datadir}/applications --remove-category="Application" --add-category="AudioVideo" %{name}.desktop You're using custom permissions for installing the executable. After compilation its 755, after installing 555. I removed the custom settings with this rough sed: sed -i '/PERMISSIONS/d' player/CMakeLists.txt Afterwards, the file section looks like that: %files -f %{name}.lang -f lastfmscrobbler.lang %defattr(-,root,root,-) %doc LICENSE README %{_bindir}/* %{_libdir}/%{name}/*.so.* %{_datadir}/applications/* %{_datadir}/icons/* Some cosmetic improvements: You can also simplify to %cmake -DCMAKE_INSTALL_LIBRARY_DIR=%{_libdir}/%{name} . You can use something like "%patch0 -p1 -b .qxt", so you can have a look at the patched sources, if necessary, and get a kind of comment as well, as a side effect. If you don't need the .so link in general, you may consider the use of NAMELINK_SKIP in CMakeLists.txt. For packaging, deleting it is also fine. If you stay with rm: Your comment there says, it would delete all NON-developer libs, which is wrong, I think. That should be it. I'll do the formal review after you submitted your changes.
I did all proposed changes. Ad removing non-developer libs, the Packaging guide states, that unversioned .so should be in -devel. Since TepSonic does not have any devel package and the libraries are just plugins which nothing will be linked against, the developer libs seem quite useless to me. They are just included, because some distributions require it (at least I was told so by packagers). New sources: Spec URL: http://www.tepsonic.org/files/fedora/tepsonic.spec SRPM URL: http://www.tepsonic.org/files/fedora/tepsonic-0.98.1-4.fc14.src.rpm
You dropped the BuildRequires instead of the Requires for Phonon. Please re-add: BuildRequires: phonon-devel and delete Requires: phonon Please mend that, because otherwise the package won't build. Here is the review. - Package meets naming guidelines - Spec file matches base package name. - Spec has consistent macro usage. - Meets Packaging Guidelines. - License acceptable for Fedora - License field in spec matches - License file included in package - Spec in American English - Spec is legible. - Sources match upstream md5sum: aba7189133005260fcf85046a5f6e80c - BuildRequires correct - Spec handles locales/find_lang - Package has %defattr and permissions on files is good. - Package has a correct %clean section. - Package is code - Packages %doc files don't affect runtime. - Package is a GUI app and has a .desktop file - Package has no duplicate files in %files. - Package doesn't own any directories other packages own. - Package owns all the directories it creates. SHOULD Items: - Functions as described - Has dist tag - Packages latest version The comment should say "Remove all devel libraries ..." not "Remove all non-devel libraries ...". As you don't use a buildroot tag, you can drop the clean section and the rm -rf %{buildroot} in %install. But it doesn't do any harm. Tests MISSING until you revised: - Requires and provides - rpmlint - Package compiles and builds on at least one arch: - Should build on all supported archs -- no buildroot, so it's only Intel.
Fixed Phonon dependency, comment and removed the rm command from install and clean sections. New sources: Spec URL: http://www.tepsonic.org/files/fedora/tepsonic.spec SRPM URL: http://www.tepsonic.org/files/fedora/tepsonic-0.98.1-5.fc14.src.rpm
- rpmlint: tepsonic.x86_64: W: no-manual-page-for-binary tepsonic 3 packages and 0 specfiles checked; 0 errors, 1 warnings. - Should build on all supported archs -- no buildroot, so it's only Intel. http://koji.fedoraproject.org/koji/taskinfo?taskID=2724370 - Package builds in Mock For the future: Please keep the changelog as accurate as you can. - Requires and provides Provides: libtepsonic_lastfmscrobbler.so.1()(64bit) tepsonic = 0.98.1-5.fc13 tepsonic(x86-64) = 0.98.1-5.fc13 tepsonic-debuginfo = 0.98.1-5.fc13 tepsonic-debuginfo(x86-64) = 0.98.1-5.fc13 Requires (without libc and rpmlib): libICE.so.6()(64bit) libQtCore.so.4()(64bit) libQtGui.so.4()(64bit) libQtNetwork.so.4()(64bit) libQtSql.so.4()(64bit) libQxtCore.so.0()(64bit) libQxtGui.so.0()(64bit) libSM.so.6()(64bit) libX11.so.6()(64bit) libXcursor.so.1()(64bit) libXext.so.6()(64bit) libXfixes.so.3()(64bit) libXi.so.6()(64bit) libXinerama.so.1()(64bit) libXrandr.so.2()(64bit) libXrender.so.1()(64bit) libcrypto.so.10()(64bit) libdl.so.2()(64bit) libfontconfig.so.1()(64bit) libfreetype.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libglib-2.0.so.0()(64bit) libgobject-2.0.so.0()(64bit) libgthread-2.0.so.0()(64bit) liblastfmlib.so.1()(64bit) libm.so.6()(64bit) libphonon.so.4()(64bit) libpng12.so.0()(64bit) libpthread.so.0()(64bit) libpthread.so.0(GLIBC_2.2.5)(64bit) librt.so.1()(64bit) libssl.so.10()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(GLIBCXX_3.4)(64bit) libtag.so.1()(64bit) libtepsonic_lastfmscrobbler.so.1()(64bit) libz.so.1()(64bit) qt-mysql qt-sqlite rtld(GNU_HASH) -------- APPROVED -------- You can request SCM.
ew Package SCM Request ======================= Package Name: tepsonic Short Description: A simple, fast and lightweight multiplaform Qt audio player with collections, plugins and all the basic functions that every good player should have. Owners: progdan Branches: f14 InitialCC:
Package SCM Request ======================= Package Name: tepsonic Short Description: A simple, fast and lightweight multiplaform Qt audio player with collections, plugins and all the basic functions that every good player should have. Owners: progdan Branches: f14 InitialCC:
Invalid Request format: "New Package SCM Request" is correct. Also please shorten the description to a few words (probably the package Summary field).
New Package SCM Request ======================= Package Name: tepsonic Short Description: A simple, fast and lightweight Qt audio player. Owners: progdan Branches: f14 InitialCC:
Git done (by process-git-requests).
tepsonic-0.98.1-5.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/tepsonic-0.98.1-5.fc14
tepsonic-0.98.1-5.fc14 has been pushed to the Fedora 14 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 tepsonic'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/tepsonic-0.98.1-5.fc14
tepsonic-0.98.1-5.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.