Not a review, just some comments:

%define	desktop_vendor 	endur
%define name  		streamtuner
%define version 	0.99.99
%define release		2.fc4
%define prefix		/usr

Lose all that.

use the real Name / Version / Release in the appropriate places

Do not define prefix - do not define desktop_vendor

-=-
Source0: %{name}-%{version}.tar.gz

Should be full url to the upstream tarball

Obsoletes: streamtuner-live365, streamtuner-local, streamtuner-xiph
Obsoletes: streamtuner-python

Why the Obsoletes? I'm assuming because there are or were packages with those
names on upstream website? If so that's probably fine - but then you need to add
provides for them.

---
%build
CFLAGS="$RPM_OPT_FLAGS"

%configure --prefix=%{prefix}
 
make %{?_smp_mflags}

should be

%build
%configure
make %{?_smp_mflags}

You don't need to define CFLAGS - rpm will do that.
%configure sets the prefix for you

---
%makeinstall

There's not %install section.
You need

%install
rm -rf %{buildroot}

Then you can use %makeinstall

I prefer make DESTDIR=%{buildroot} install

sometimes %makeinstall results in some wrong path stuff, but it sometimes is OK.

desktop-file-install --vendor %{desktop_vendor} --delete-original \

should be

desktop-file-install --vendor=fedora \

-=-
%find_lang %{name}

That means you need to BuildRequires: gettext

-=-
%files -f %{name}.lang
%defattr(-, root, root)
%{_bindir}/%{name}
%dir %{_libdir}/%{name}
%dir %{_libdir}/%{name}/plugins
%{_libdir}/%{name}/plugins/*.so
%dir %{_datadir}/%{name}
%{_datadir}/%{name}/*
%dir %{_datadir}/omf/%{name}
%{_datadir}/omf/%{name}/*
%{_datadir}/applications/%{desktop_vendor}-%{name}.desktop
%{_datadir}/pixmaps/%{name}.png
%{_datadir}/gtk-doc/html/*
%dir %{_datadir}/help
%{_datadir}/help/*
%doc AUTHORS COPYING INSTALL NEWS README TODO

can be simplified greatly. Usually %doc goes at top - and usually INSTALL insn't
packaged. Make sure the other files aren't empty as well (NEWS often is, though
not always)

%files -f %{name}.lang
%defattr(-, root, root, -)
%doc AUTHORS COPYING NEWS README TODO
%{_bindir}/%{name}
%{_libdir}/%{name}/
%{_datadir}/%{name}/
%{_datadir}/omf/%{name}/
%{_datadir}/applications/*.desktop
%{_datadir}/pixmaps/%{name}.png
%{_datadir}/gtk-doc/html/*
%{_datadir}/help/

-=-
%{_datadir}/help/ though is probably wrong.

-=-

Also - all your summary lines end in a .
They shouldn't.

desktop-file-install --vendor %{desktop_vendor} --delete-original \

should be

desktop-file-install --vendor=fedora \

Note that you don't need to --delete-original because the original isn't
installed in the buildroot (you can, but not need to)

also - noticed

%define release		2.fc4

When you do it - use %{?dist} tag - ie

Release:                2%{?dist}

The build system will have dist defined properly.

Hi, Michael...

...the required things are changed now. You'll find the changed files here...

Spec Name or Url: http://www.bennewitz.com/rpms/streamtuner.spec
SRPM Name or Url: http://www.bennewitz.com/rpms/streamtuner-0.99.99-3.src.rpm

-- 
Regards
       Matthias

Need to add the following BuildRequires:

desktop-file-utils

Otherwise it won't build in mock.


[mpeters@utility result]$ ls *.rpm && rpmlint *.rpm
streamtuner-0.99.99-3.fc4.i386.rpm
streamtuner-0.99.99-3.fc4.src.rpm
streamtuner-debuginfo-0.99.99-3.fc4.i386.rpm
streamtuner-devel-0.99.99-3.fc4.i386.rpm
E: streamtuner obsolete-not-provided streamtuner-live365
E: streamtuner obsolete-not-provided streamtuner-local
E: streamtuner obsolete-not-provided streamtuner-xiph
E: streamtuner obsolete-not-provided streamtuner-python
E: streamtuner useless-explicit-provides live365.so
E: streamtuner useless-explicit-provides local.so
W: streamtuner-devel no-documentation
[mpeters@utility result]$ 

Since you have the obsoletes set on those packages, it needs to Provide them as
well.
The explicit .so provides should probably be removed.
The warning on the devel package - if there isn't any developer doc to package,
it can be ignored.

-=-
From the build.log of the mock build I did:

Installation directories
  --bindir                     /usr/bin
  --datadir                    /usr/share
  --libdir                     /usr/lib
  --includedir                 /usr/include

Features
  --enable-shoutcast           yes 
  --enable-live365             yes 
  --enable-xiph                no (libxml not found)
  --enable-local               yes 
  --enable-local-metadata      yes 
  --enable-python              yes 

I suspect you need to add libxml2-devel to the BuildRequires
-=-
Other than those issues, it looks good.
Oh - one more thing - in the %changelog -

* Sun Dec 18 2005 Matthias Haase <matthias_haase> - 0.99.99-3.fc4

You should drop the .fc4 from the changelog.
That's provided by the %{?dist} tag - so the same spec file could be used to
build for fc3/4/5 (assuming it will build on all of them)


-=-
Please note that I'm happy to informally review this package, it looks likes a
good addition - but I am not able to sponsor people - so a reviewer who is able
to do so still is needed.

I have done some cleanups on the specfile. Again you'll find the changed files
here...

Spec Name or Url: http://www.bennewitz.com/rpms/streamtuner.spec
SRPM Name or Url: http://www.bennewitz.com/rpms/streamtuner-0.99.99-4.fc4.src.rpm

Because I have defined %{dist} in ~/.rpmmacros, the filename of the source rpm
contains 'fc4' now.

And I have considered including streamtuner in extras because some people have
ask about this, already using my Fedora rpm from upstream, in order to make it
easier to install (yum install streamtuner).

Thanks for your review, I'll change some other specfiles later too, following
the provided strict style.

-- 
Regards
       Matthias
 

Excellent - rpmlint is clean -
[mpeters@jerusalem result]$ ls *.rpm && rpmlint *.rpm 
streamtuner-0.99.99-4.fc4.i386.rpm
streamtuner-0.99.99-4.fc4.src.rpm
streamtuner-debuginfo-0.99.99-4.fc4.i386.rpm
streamtuner-devel-0.99.99-4.fc4.i386.rpm
W: streamtuner-devel no-documentation
[mpeters@jerusalem result]$ 

from build.log
-=-
Features
  --enable-shoutcast           yes 
  --enable-live365             yes 
  --enable-xiph                yes 
  --enable-local               yes 
  --enable-local-metadata      yes 
  --enable-python              yes 

-=-
builds everything it is suppose to (in mock)

(In reply to comment #6)
> Excellent - rpmlint is clean -
> builds everything it is suppose to (in mock)

(In reply to comment #4)
> but I am not able to sponsor people - so a reviewer who is able to do so still
is needed.

Have I to do some extra steps at this stage to get sponsored?

--
  Regards
        Matthias

Someone with the power to sponsor has to decide to do so.

One thing to encourage that is to comment on other peoples packages to
demonstrate the understanding of Fedora RPM PackagingGuidelines.

http://fedoraproject.org/wiki/Extras/Contributors

You are at Step 6 - so someone who has authority to sponsor (from the sponsor
list) has to do a formal review of the package.

one thing to mention is that some plugins require you to have a mp3 capable
audio player to listen to the streams.

(In reply to comment #9)
> one thing to mention is that some plugins require you to have a mp3 capable
> audio player to listen to the streams.

Hi, Che...

...streamtuner does not play/decode mp3's _by itself_  and isn't useless without
an mp3 capable audio player. You can use streamtuner, for example, together with
xmms and without an installed mp3 plug-in for xmms, getting from elsewhere.

Same usability as for xmms or beep media player, related to the mp3 decoding stuff.

--- 
Regards
      Matthias

but theres the question if those plugins should maybe be split off... its not
worth it but well i see bogus bug reports coming if this doesent go to the faq e.g.

After a first look...

streamtuner-devel package:

 * Missing "Requires: gtk2-devel" because else it would break the
   pkgconfig dependency chain.

 * The Cflags line in the pkgconfig file is bad. It ought not add
   -I/usr/include because that is a standard search path for header
   files already.

 * The included static libraries and libtool archives are _highly_
   questionable. Unless you can state a very good reason why static
   versions of the plugin (!) DSOs should be shipped, delete them after
   %install or %exclude them in the %files section.

streamtuner binary package:

 * Category X-Red-Hat-Base in .desktop file is wrong. Should be X-Fedora,
   because this is not Fedora Core.

Run-time observation:

 * Clicking "Browse" button or "Help > Homepage" menu gives fatal
   error dialog:

    Unable to browse
    Failed to execute child process
    "epiphany" (no such file or directory)

   Looks like either a missing customisation or a missing
   dependency.

 * "xmms" also seems to be another missing requirement, as an
   error dialog pops up upon trying to tune in to a station.

 * "Record" button fails with 
    
    xterm: Can't execvp streamripper: No such file or directory

   Looks like more missing dependencies: "xterm", and do we have
   "streamripper" anywhere yet?

spec file issues:

 * Licence GPL is wrong. The included file COPYING contains is "BSD"
   licence text. The C source files don't mention GPL either.

 * Requires: gtk2 >= 2.2.2, curl >= 7.10.8, taglib >= 1.3, libidn >= 0.5.6
   All of these must go. There are automatic RPM dependencies on the
   needed SONAMEs already. Check out "rpm -qR streamtuner". Package
   resolvers know which packages to install in order to get the required
   libraries.

 * Requires: pygtk2 >= 2.4.0
   At a first glance it doesn't become clear why this dependency was added.

 * Concerning all those versions in the [build] requirements, consider
   dropping them. Unless, of course, you insist on verifying all of them
   for each and every update. ;)   Basically, upstream's configure script
   ought to check the available build dependencies. That's enough. Trying
   to keep such versions in sync with updates results in a maintenance
   nightmare.

 * -devel package sub-section ought to do
   "Requires: %{name} = %{version}-%{release}
   with full %release to keep -devel package and main package in sync.

 * Missing dependencies on the tools used in the scriptlets:
   Requires(post): desktop-file-utils scrollkeeper
   Requires(postun): desktop-file-utils scrollkeeper

   Although, running update-desktop-database in the scriptlets is not
   necessary for this package [yet].

> pygtk2

Ah, for the included Python scripts.

(In reply to comment #12)
> After a first look...
>
> 
>  * Clicking "Browse" button or "Help > Homepage" menu gives fatal
>    error dialog:
> 
>     Unable to browse
>     Failed to execute child process
>     "epiphany" (no such file or directory)
>    Looks like either a missing customisation or a missing
>    dependency.
One browser is required and should be added to the requirement.
Which browser? Firefox? The default config file should be patched, matching this
browser, I think.

>  * "xmms" also seems to be another missing requirement, as an
>    error dialog pops up upon trying to tune in to a station.
Related to user settings... 
>  * "Record" button fails with 
>     xterm: Can't execvp streamripper: No such file or directory
>    Looks like more missing dependencies: "xterm", and do we have
>    "streamripper" anywhere yet?
Yes, I have created an additional "streamripper" package already... Xterm is
required only for these streamripper feature.

> spec file issues:
I'll change this and I'll cleanup some others tomorrow.

(In reply to comment #14)
> Which browser? Firefox?

htmlview or gnome-open?

(In reply to comment #13)
> > pygtk2
> 
> Ah, for the included Python scripts.
All required cleanups are done now (I think so) ;-)
Beep-media-player, htmlview and xterm are added to the requirement. I had to
patch several files to change the default configuration. You have to delete
~/.streamtuner/ to make these changes visible.

The streamripper rpm is an usefull add-on, but isn't a requirement at this time,
it uses libmad.

The changed files are there already:

Spec Url: http://www.bennewitz.com/rpms/streamtuner.spec
SRPM Url: http://www.bennewitz.com/rpms/streamtuner-0.99.99-5.fc4.src.rpm

-- 
Matthias

Created attachment 122667 [details]
fix .pc file

Created attachment 122668 [details]
spec patch to remove redundant dependencies on hardcoded package names

Dependenies on curl, libidn, taglib and gtk2 are automatic already, see
rpm -qR streamtuner | grep 'curl\|idn\|tag\|gtk'

The new defaults and fixed dependencies work much better here.

Running update-desktop-database is still not necessary since the
.desktop file doesn't activate any MIME-type assignments.

All changes are done and the changed files are there already:

Spec Url: http://www.bennewitz.com/rpms/streamtuner.spec
SRPM Url: http://www.bennewitz.com/rpms/streamtuner-0.99.99-6.fc4.src.rpm

* reported issues fixed
* patches visited
* spec file looks good
* binary package contents look good
* upstream locations verified (home page has moved to
http://www.nongnu.org/streamtuner/ )
* upstream source is signed

APPROVED

The remaining unneeded dependency on desktop-file-utils can be deleted
in CVS.

If you have signed up for an account already, tell me your user name,
and I will sponsor you. Else feel free to sign up.

> If you have signed up for an account already, tell me your user name,
> and I will sponsor you. Else feel free to sign up.
 
My user name is 'endur'.

__
  Regards
         Matthias

> The remaining unneeded dependency on desktop-file-utils can be deleted
> in CVS.
Fixed in CVS. I'm currently at step 18. Branches for FC-3 and FC-4 are missing.
And
$ make plague fails currently with

/usr/bin/plague-client build streamtuner streamtuner-0_99_99-7_fc5 devel
Server returned an error: Insufficient privileges.

Creation of distribution branches in CVS can be requested in the Wiki:

  http://fedoraproject.org/wiki/Extras/FC4Status
  http://fedoraproject.org/wiki/Extras/FC3Status

If you uploaded your correct and undamaged SSH public key in the accounts
system, if you configured plague-client and the needed certificates in your
home directory,

  http://fedoraproject.org/wiki/Extras/BuildSystemClientSetup

and if the problem persists, only the accounts system staff can find out
what the problem with your account is. Help contact address is at the
bottom of https://admin.fedora.redhat.com/accounts/

Verifying the configuration steps and re-uploading your SSH public key
might help meanwhile.

Although, CVS access would fail due to SSH key misconfiguration, so probably
it's plague-client misconfiguration.

(In reply to comment #25)
> Although, CVS access would fail due to SSH key misconfiguration, so probably
> it's plague-client misconfiguration.
 
I can successfully use other plague commands like list and list_builders.
> If you uploaded your correct and undamaged SSH public key in the accounts
> system
This is done already, because there isn't any probblem with the CVS access, as I
said.

This is very cool, thanks everyone!

All builds are done successful.

> it's plague-client misconfiguration.
I couldn't found errors in ~/.plague-client.cfg, but anyway, it is working as
expected today.

--
  Matthias

> Spec Url: http://www.bennewitz.com/rpms/streamtuner.spec
> SRPM Url: http://www.bennewitz.com/rpms/streamtuner-0.99.99-6.fc4.src.rpm
I have removed these files.

> Removing the BR will break the build. Removing the Requires a few lines
> above would be correct, since the scriptlets no longer run
> desktop-update-database.
Thanks, yes, of course... I have noticed this later myself and, at once,
corrected again. Otherwise, the build hangs at the missed desktop-file-install
binary.

This ticket can be closed now.