Bug 456038 (DarkIce)

While working on this spec file, I was attempting to get it to build with mock.
 Because of this, there is a small patch that was generated as well.  Thanks to
ricky for the patch and ianweller for general help.

Patch0: http://herlo.fedorapeople.org/files/darkice-0.19-configure.patch

I've also updated the src.rpm and the spec file to match.

Hi. This is just an informal review with some comments you might
find helpful.

  * The description line starting "DarkIce is a live ..." exceeds 79
    characters and wraps around on a standard terminal, you should
    break this up.

  * Your %changelog entry version is 0.19.1-1 but your package name
    and version is 0.19-1 - they should be consistent.

  * The "GPL" license is invalid it should be "GPLv2" or "GPLv2+" for
    example, see https://fedoraproject.org/wiki/Licensing.

  * The Group "Applications/Sound and Video" does not exist in the
    official list in /usr/share/doc/rpm-4.4.2.3/GROUPS (or equivalent
    rpm version) whereas it should be one from this list according
    to http://docs.fedoraproject.org/drafts/rpm-guide-en/ch13s02.html
    (section 13.2.2.7), although I don't know how current that is.

  * You should have a %config in front of %{_sysconfdir}/darkice.cfg
    as its a configuration file (then changes to it will be preserved
    across rpm upgrades).

  * The "Requires: libogg" and "Requires: libvorbis" lines are superfluous,
    you should let rpm find these lib dependencies on its own.

  * Consider using %{name}/%{version} in the Source0 line (since you have
    used them in the Patch0 line)?

  * You use $RPM_BUILD_ROOT in the %install block but %{buildroot} in the
    %clean block - choose one or the other and be consistent.

  * Provided configuration file is broken when program run as I got
    the following error:

      DarkIce: ConfigSection.cpp:117: format missing in section icecast-0 [0]

    It seems inconsistent with the man page which says "format" applies
    to [icecast2-x] sections not [icecast-x] sections.

  * Not sure you need an explicit BuildRequires line for "pkgconfig", nor
    libogg and libvorbis (as these are pre-requisites of the equivalent
    -devel packages anyway).

Thank you for the review.

Updated SPEC/SRPM are now available in the same location as above.

I've updated and fixed all of the above issues, with the exception of the following:

 * Provided configuration file is broken when program run as I got
    the following error:

      DarkIce: ConfigSection.cpp:117: format missing in section icecast-0 [0]

    It seems inconsistent with the man page which says "format" applies
    to [icecast2-x] sections not [icecast-x] sections.

The issue with this error is that you are attempting to use darkice to connect
with an icecast 1.x.x server.  In the man page, it specifically states that the
[icecast-x] sections are to be used only with the lame encoder.  

From the man page:

----------------------------------------------------------------------------

[icecast-x]

       This  section describes an output to an IceCast 1.3.x server or Darwin
Streaming Server , while encoding with a lame encoder.

---------------------------------------------------------------------------

Since Fedora doesn't support lame, twolame or faac, this support has been
removed from the package.  My question is, does that mean we should modify the
man page to match?  Or leave it in place?

Cheers,

Clint

Can't the lame related items be run-time detected? Would you ask upstream if they are interested in something like this or gstreamer? If it can't be done, might be worth it to patch the man page or add a README.dist file explaining what has been changed or maybe provide a macro to rebuild it.

Rahul,

What do you mean 'lame related items be run-time detected'?  The application can be built without the lame libraries altogether, no patch needed.  Why would we want to include them?

Cheers,

Clint

I didn't say we would want to include them. I was thinking of a xine-lib like solution to the problem where third repos can add what is being removed here.

Rahul,

Oh, if a thirdparty repo wants to build darkice-nonfree or something, its a simple matter of requiring the right libraries.  The configure script would take care of that already.  I have simply omitted those libraries in my build.

Make more sense?

MD5Sum:
590c152cde2d62fef422f9f773560e95  darkice-0.19.tar.gz

Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Buildroot has all required elements
* All paths begin with macros
* Make succeeds even when %{_smp_mflags} is defined
* Files have appropriate permissions and owners
* Rpmlint produces only the following warning, which can be safely ignored:
darkice.x86_64: W: conffile-without-noreplace-flag /etc/darkice.cfg
* Package installs and uninstalls cleanly

Bad:
* License should be GPLv2+, which you can fix when you import the package into CVS.

+1 APPROVED, and I'll also be you sponsor.

New Package CVS Request
=======================
Package Name: darkice
Short Description:  Live audio streamer
Owners: herlo
Branches: F-9, F-10
InitialCC:

cvs done.

Hey Clint, if this has been built you can close the bug.

Step #13: http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess