Bug 492583

This doesn't seem to me to be a pidgin plugin, more like a purple plugin. Thus the name should be purple-gfire and BuildRequires: libpurple-devel.

The existence of the pixmap does call for the Requires: pidgin.

Fix mixed use of tabs and spaces:
pidgin-gfire.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 15)

Spec URL:
http://fedora.mkdir.name/packages/purple-gfire-0.7.1/purple-gfire.spec

SRPM URL:
http://fedora.mkdir.name/packages/purple-gfire-0.7.1/purple-gfire-0.7.1-2.fc10.src.rpm

* Sat Mar 28 2009 Andreas Osowski <th0br0> - 0.7.1-2
- Renamed to purple-gfire
- Fixed indentation
- Changed description

rpmlint now remains quiet.

Spec URL:
http://fedora.mkdir.name/packages/purple-gfire-0.8.1/purple-gfire.spec

SRPM URL:
http://fedora.mkdir.name/packages/purple-gfire-0.8.1/purple-gfire-0.8.1-1.fc10.src.rpm

* Wed May 20 2009 Andreas Osowski <th0br0> - 0.8.1-1
- New upstream version

(In reply to comment #1)
> This doesn't seem to me to be a pidgin plugin, more like a purple plugin. Thus
> the name should be purple-gfire and BuildRequires: libpurple-devel.

Hmm, I no longer agree with myself :D
This is after all a pidgin plugin, at least the home page clearly says so. So the name should be after all pidgin-xfire. Sorry for the hassle.

Now:

- You should  Requires: libpurple explicitly for dir ownership, even though this should be automatically picked up by rpm as a lib dependency. Also, please make a comment about the dir ownership section of Requires, so that you/other people will not remove it without knowing why it's there.

- Please don't use macros when not necessary (%{__chmod} is just chmod).

- Change
 %{_datadir}/purple/%{srcname}/*
 %{_datadir}/purple/%{srcname}/scripts/server_detection/*
to
 %{_datadir}/purple/%{srcname}/
as this will own the directory (necessary) and everything in it. Now the package doesn't own the directory /usr/share/purple/gfire and you are listing stuff twice:

warning: File listed twice: /usr/share/purple/gfire/scripts/server_detection/4097.py
warning: File listed twice: /usr/share/purple/gfire/scripts/server_detection/4097.pyc
warning: File listed twice: /usr/share/purple/gfire/scripts/server_detection/4097.pyo
warning: File listed twice: /usr/share/purple/gfire/scripts/server_detection/tools

- You can fit the make install -stuff into one line.

- Description uses some odd characters. ’ should be ' and ” should be ".

- License is GPLv3+ not GPLv2+.

- BR: libpurple-devel is redundant, it's already pulled in by pidgin-devel. So is glib2-devel, which is pulled in by pidgin-devel -> gtk2-devel.

- What is the line
 sed -i -e '/#\!/d' %{buildroot}/%{_datadir}/purple/%{srcname}/scripts/server_detection/4097.py
for? I assume it removes a shebang in the python file. Please, add a comment!

***

rpmlint output is clean.


MUST: The spec file for the package is legible and macros are used consistently. NEEDSWORK
- Don't use %{__chmod} for chmod.

MUST: The package must be named according to the Package Naming Guidelines. NEEDSWORK
- Should be named pidgin-xfire (yes, my mistake) :)

MUST: The spec file name must match the base package %{name}. OK
- Remember to rename this back too.

MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK

MUST: The License field in the package spec file must match the actual license. NEEDSFIX
- License is GPLv3+ not GPLv2+.

MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A

MUST: A package must own all directories that it creates or require the package that owns the directory. NEEDSWORK
- Should Requires: libpurple explicitly.

MUST: Files only listed once in %files listings. NEEDSWORK
- Use %{_datadir}/purple/%{srcname}/ to own everything you should.

MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. OK
MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK
MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK

I hope everything is fixed now, I didn't notice upstream's license change.
When adding Requires: libpurple, rpmlint says E: explicit-lib-dependency libpurple
so I dropped that tag again... RPM is recognizing it by itself. Besides, Requires: pidgin automatically pulls in libpurple.

Spec URL:
http://fedora.mkdir.name/packages/pidgin-gfire-0.8.1/pidgin-gfire.spec

SRPM URL:
http://fedora.mkdir.name/packages/pidgin-gfire-0.8.1/pidgin-gfire-0.8.1-2.fc10.src.rpm

(In reply to comment #5)
> I hope everything is fixed now, I didn't notice upstream's license change.
> When adding Requires: libpurple, rpmlint says E: explicit-lib-dependency
> libpurple
> so I dropped that tag again... RPM is recognizing it by itself. Besides,
> Requires: pidgin automatically pulls in libpurple.

Oh, sometimes you don't have to care about rpmlint's warnings since it does produce some false positives; this is one very common type of them. Due to the dir ownership issue it IS wanted that libpurple is explicitly required; the rpmlint warning can be ignored.

I don't have an xfire account, so I can't test this. From a packaging point of view the package looks good, though.

APPROVED


PS. A final nitpick: I'd use
 %{_datadir}/pixmaps/pidgin/protocols/*/%{srcname}.png
instead of
 %{_datadir}/pixmaps/pidgin/protocols/*/%{srcname}*
since thus one sees straight away what kind of stuff in is in those directories.

All right, I fixed that stuff now.

Spec URL:
http://fedora.mkdir.name/packages/pidgin-gfire-0.8.1/pidgin-gfire.spec

SRPM URL:
http://fedora.mkdir.name/packages/pidgin-gfire-0.8.1/pidgin-gfire-0.8.1-3.fc10.src.rpm

* Wed May 21 2009 Andreas Osowski <th0br0> - 0.8.1-3
- Added Requires: libpurple
- Changed the files entry for the pixmaps

New Package CVS Request
=======================
Package Name: pidgin-gfire
Short Description: Xfire plugin for Pidgin
Owners: th0br0
Branches: F-10 F-11

cvs done.

pidgin-gfire-0.8.1-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/pidgin-gfire-0.8.1-3.fc10

pidgin-gfire-0.8.1-3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/pidgin-gfire-0.8.1-3.fc11

pidgin-gfire-0.8.2-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/pidgin-gfire-0.8.2-1.fc10

pidgin-gfire-0.8.2-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/pidgin-gfire-0.8.2-1.fc11