Bug 523650

Created attachment 361231 [details]
QMPDClient Spec File

Created attachment 361232 [details]
Patch for Qt4 Project File

Created attachment 361233 [details]
Desktop File

Added FE-NEEDSPONSOR.

Please post hyperlinks to the spec file and the SRPM generated from it instead of posting attachments to the review bug.

Hi

Could you please post the spec file and SRPM to a web or ftp server where they can be downloaded.  I'll have a look at the package once this is done.  

Thanks

CC

http://files.4-web.net/qmpdclient/qmpdclient-1.1.1-1.fc10.src.rpm
http://files.4-web.net/qmpdclient/qmpdclient.spec

Thanks for your support :)

Initial inspection

1) The changelog entry should be %version-%release i.e. 1.1.1-1 (without the %dist tag)

2) The files section should use macros i.e. /usr/share = %_datadir 

3) There is an empty %doc tag

4) The group Application/Music is non-standard

5) Source0 should reference the full downloadable URL for the tarball

6) rpmlint reports mixed use of spaces and tabs on line of the spec file, please try and keep these consistent.  Just use one or the other.

More comments to follow.

CC

This fails building in koji.

+ desktop-file-install --vendor fedora --dir /builddir/build/BUILDROOT/qmpdclient-1.1.1-1.fc11.i386//usr/share/applications --add-category X-Fedora /builddir/build/SOURCES/QMPDClient.desktop
/var/tmp/rpm-tmp.9iHeZb: line 39: desktop-file-install: command not found
error: Bad exit status from /var/tmp/rpm-tmp.9iHeZb (%install)

The file 'desktop-file-install' wasn't found.  Can you please check the filename/path and providing package?

Thanks

CC

for desktop-file-install, just add
BuildRequires: desktop-file-utils

I'm sorry i couldn't respond sooner. Thanks for you hints, Colin.

@Dieter: it's strange, this line is already in the spec file and it did well on OpenSuse Buildservice. Do I have to set a path?

@Colin:
1.) is fixed
2.) Do I have to define these macros?
3.) Should I remove the %doc tag if there isn't any documentation.
4.) Set to Application/Multimedia, is that correct?
5.) There is just this git url as mentioned before, is it better now?
6.) Should be fixed now.

Please advise where updated spec and src.rpm files can be found.  The spec changelog and release should also be updated.

The macros are defined in /usr/lib/rpm/macros file.

Thanks

CC

I updated the "old" files. Is it ok for you if I update release number and changelog next time? I didn't do it because I thought it's not a "release". If it's necessary to do it this time, please let me know.

Yes, please do increment Release and add changelog entries, makes it easier to track changes and progress.

Julian, as an example, http://members.iinet.net.au/~coec/RackTables.spec is an example of a package I recently had reviewed.  Note the changelog entries track the issues raised by the review.

CC

Sorry, busy week. I've updated the files now.

http://files.4-web.net/qmpdclient/qmpdclient-1.1.1-2.fc10.src.rpm
http://files.4-web.net/qmpdclient/qmpdclient.spec

Thanks for your support :)

A few quick comments:

License is GPLv2+
(Look at the source code "either version 2 of the License, or (at your option) any later version.")

There's no need to include BR: gcc-c++
That's part of the minimal build environment:
http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRequires

MUST: Each package must consistently use macros.
Source0: http://files.4-web.net/%{name}/%{name}-%{release}.tar.gz
Source1: %{name}.desktop 
why do you use capital letters there?
Patch: %{name}.pro.patch

What's with the patch anyways? You have it there but dont use it.
You could comment it. But that's up to you.

Be consistent: %{_datadir} %_datadir

%doc is empty, fill it with AUTHOR COPYING INSTALL README

MUST: rpmlint must be run on every package. The output should be posted in the review.
Please post it here.

Please post as well the "Task Info link" of a koji scratch build here:
http://fedoraproject.org/wiki/PackageMaintainers/Join

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Thanks for your reply, Thomas.

The patch gets applied in the %prep section.

Should I replace $RPM_BUILD_ROOT with %{buildroot}?

Are there any not documented options for desktop-file-install?

How do i tell the doc tag to copy the files AUTHOR COPYING INSTALL README Changelog?

I use %{buildroot} over $RPM_BUILD_ROOT, but that's up to you. As long as you dont mix it.

* %doc question
You use:
%doc AUTHOR COPYING README Changelog
Thats it.

* desktop-file-install question
http://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage
Don't apply a vendor tag.

Well, there's a man page for desktop-file-install online. But the host is veeeeeery slow.
http://olympus.het.brown.edu/cgi-bin/man/man2html?desktop-file-install+8


Why you're useing capital letters for your .desktop file?

* patch question
Normal you use: Patch0: foo.patch
and           : %patch0    OR    %patch -P 0

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

I lowered the capital letters already.

http://files.4-web.net/qmpdclient/qmpdclient-1.1.1-3.fc10.src.rpm
http://files.4-web.net/qmpdclient/qmpdclient.spec

rpmlint qmpdclient.spec qmpdclient-1.1.1-3.fc10.src.rpm
1 packages and 1 specfiles checked; 0 errors, 0 warnings.

The spec file linked in Comment #19 is still the old one.
The SRPM is not found on that server.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Sorry, I've uploaded it into the wrong directory. Links are fixed now. I'll double check it in future ;)

[thomas@tusdell ~]$ rpmlint srpm-review-test/qmpdclient.spec
srpm-review-test/qmpdclient.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 48, tab: line 1)
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

[thomas@tusdell ~]$ rpmlint rpmbuild/RPMS/x86_64/qmpdclient-*
qmpdclient-debuginfo.x86_64: E: empty-debuginfo-package
2 packages and 0 specfiles checked; 1 errors, 0 warnings.

[thomas@tusdell ~]$ rpmlint -I empty-debuginfo-package
empty-debuginfo-package:
This debuginfo package contains no files.  This is often a sign of binaries
being unexpectedly stripped too early during the build, rpmbuild not being
able to strip the binaries, the package actually being a noarch one but
erratically packaged as arch dependent, or something else.  Verify what the
case is, and if there's no way to produce useful debuginfo out of it, disable
creation of the debuginfo package.

That's what i get after i recompiled your package and run rpmlint on it.
Installed package is rpmlint clean.
I have to say that my only idea is, to ask upstream if it's known or how to get the debuginfo out..
Maybe one of the other reviewers is able to help out?

http://koji.fedoraproject.org/koji/taskinfo?taskID=1741049

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

http://files.4-web.net/qmpdclient/qmpdclient-1.1.1-4.fc10.src.rpm
http://files.4-web.net/qmpdclient/qmpdclient.spec

rpmlint RPMS/i386/qmpdclient-*4* SRPMS/qmpdclient-1.1.1-4.fc10.src.rpm SPECS/qmpdclient.spec
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient-1.1.1/.res
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient-1.1.1/.res
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient-1.1.1/.moc
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient-1.1.1/.moc
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient-1.1.1/.ui
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient-1.1.1/.ui
3 packages and 1 specfiles checked; 0 errors, 6 warnings.

Any Ideas? Is this debuginfo package usefull or should i disable it?

[thomas@tusdell x86_64]$ rpmlint -I hidden-file-or-dir
hidden-file-or-dir:
The file or directory is hidden. You should see if this is normal, and delete
it from the package if not.

The debuginfo package looks useful to me. You might ask upstream why they use 3 hidden folder (.res, .moc, .ui) and one not hidden (src). That doesn't make sense to me, but who knows.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

I talked to the maintainer of the project. The hidden directories are created by qmake and filled with generated content by make.

I'm build this package on a F11. Nice client. Very much it would be desirable to see in repository. Good job.

Julian, new version is available on project homepage http://bitcheese.net/wiki/QMPDClient ;)
Why not to build it?

http://files.4-web.net/qmpdclient/qmpdclient-1.1.2-1.fc10.src.rpm
http://files.4-web.net/qmpdclient/qmpdclient.spec


rpmlint SPECS/qmpdclient.spec SRPMS/qmpdclient-1.1.2-1.fc10.src.rpm RPMS/i386/qmpdclient-1.1.2-1.fc10.i386.rpm RPMS/i386/qmpdclient-debuginfo-1.1.2-1.fc10.i386.rpm
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient/.ui
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient/.ui
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient/.moc
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient/.moc
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient/.res
qmpdclient-debuginfo.i386: W: hidden-file-or-dir /usr/src/debug/qmpdclient/.res
3 packages and 1 specfiles checked; 0 errors, 6 warnings.

What else is needed to get this into the repos?

Because, that this is your first package, you need a sponsor.

http://files.4-web.net/qmpdclient/qmpdclient-1.1.2-2.fc12.src.rpm
http://files.4-web.net/qmpdclient/qmpdclient.spec

rpmlint SPECS/qmpdclient.spec SRPMS/qmpdclient-1.1.2-2.fc12.src.rpm RPMS/x86_64/qmpdclient*-1.1.2-2.fc12.x86_64.rpm
qmpdclient.x86_64: W: unstripped-binary-or-object /usr/bin/qmpdclient
2 packages and 1 specfiles checked; 0 errors, 1 warnings.

SHOULD:  drop "Qt4 based" from package summary/description.  (hardly) anyone cares what toolkit is used, as long as the application works.

MUST: drop Requires: qt  
not needed

MUST: fix desktop-file-install usage
(see also http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files)
1.  don't use '--add-category X-Fedora'
2.  no need to run both desktop-file-install *and* desktop-file-validate (the former is sufficient).

MUST: update icon-related scriptlets, see,
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

MUST: please install qmpdclient64.png icon too (most DE's don't handle svg icons well or at all)

MUST: please adjust the patch to drop
QMAKE_LFLAGS_RELEASE adjustments completely.  These are not needed or desired either (this part should be upstreamable, they shouldn't be mucking with these flags).

There are lots of MPD Clients (http://mpd.wikia.com/wiki/Clients) so in my opinion it's a useful information if I'm searching for a client which fits into my desktop environment.

Is it possible to use qmpdclient64.png as fallback referenced in the desktop file?

Many thanks for your help :)

http://files.4-web.net/qmpdclient/qmpdclient-1.1.2-3.fc12.src.rpm
http://files.4-web.net/qmpdclient/qmpdclient.spec

rpmlint SPECS/qmpdclient.spec SRPMS/qmpdclient-1.1.2-3.fc12.src.rpm RPMS/x86_64/qmpdclient*-1.1.2-3.fc12.x86_64.rpm
qmpdclient.x86_64: W: unstripped-binary-or-object /usr/bin/qmpdclient
2 packages and 1 specfiles checked; 0 errors, 1 warnings.

Looks good, thanks.

APPROVED.

(what's your FAS username?  given that, I can sponsor you).

Thx :) I've just created an FAS account (jgold).

OK, sponsored.

continue on in the steps outlined in
http://fedoraproject.org/wiki/PackageMaintainers/Join
(ie, submit a cvsadmin request, and start to setup fedora-packager tools).

Feel free to contact me if you ever have any questions or need help.

New Package CVS Request
=======================
Package Name: qmpdclient
Short Description: Qt4 based MPD client
Owners: jgold
Branches: F-12
InitialCC: jgold

CVS done (by process-cvs-requests.py).

qmpdclient-1.1.2-3.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/qmpdclient-1.1.2-3.fc12

Thanks everyone. :)

I think that 'make install' is not working well because not install translations.
All files can be installed manually.

Here spec which not uses 'make install'
http://nucleo.fedorapeople.org/qmpdclient.spec

%find_lang not works, so I have used %lang.
May be somehow %find_lang can work in this case but I don't know how.

The other solution is to use cmake. Then patching is not needed.
http://nucleo.fedorapeople.org/qmpdclient-cmake.spec

desktop file installs but it uses icon that installs in wrong place.

qmpdclient-1.1.2-3.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 qmpdclient'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2010-1246

Thank you nucleo! The cmake way seems more accurate.

http://koji.fedoraproject.org/koji/packageinfo?packageID=9835

qmpdclient-1.1.2-4.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 qmpdclient'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2010-1246

qmpdclient-1.1.2-4.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.