Bug 224365

*** Bug 224364 has been marked as a duplicate of this bug. ***

problem: requires cmake from extras, which itsself requires xmlrpc-c (extras)

Why is the a problem?  Recall that "Core" and "Extras" have no meaning past FC6,
so if you're only targeting F7 then there shouldn't be any trouble.

Even then the issue seems to be one of build-time dependencies, and I wouldn't
expect that these packages have additional runtime dependencies.

So really the fundamental question is whether we actually want to replace
cdrecord and friends in F7 with cdrkit.

yes, I want :)

It does seem that moving to cdrkit is in the plan for F7.  I'll review this.

This fails to build for me:

CMake Error: Error: found a Linux system but no libcap header. Install libcap-dev.

Adding BuildRequires: libcap-devel gets me past that, but then the build fails
later:

[ 70%] Building C object genisoimage/CMakeFiles/genisoimage.dir/jte.o
/builddir/build/BUILD/cdrkit-1.1.2/genisoimage/jte.c:18:18: error: zlib.h: No
such file or directory
/builddir/build/BUILD/cdrkit-1.1.2/genisoimage/jte.c: In function
'flush_gzip_chunk':
/builddir/build/BUILD/cdrkit-1.1.2/genisoimage/jte.c:581: error: 'z_stream'
undeclared (first use in this function)
[etc.]

Adding BR: zlib-devel gets through that and allows the build to complete.

The resulting packages don't install properly:

Error: Missing Dependency: /usr/local/bin/perl is needed by package icedax
Error: Missing Dependency: perl(v5.8.1) is needed by package genisoimage

Some rpmlint complaints:

W: cdrkit summary-ended-with-dot A collection of CD/DVD utilities.
W: genisoimage summary-ended-with-dot Creates an image of an ISO9660 filesystem.
W: wodim summary-ended-with-dot A command line CD/DVD recording program.
W: icedax summary-ended-with-dot A utility for sampling/copying .wav files from
digital audio CDs.
   Best for summaries to not end with dots.

E: icedax invalid-dependency /usr/local/bin/perl
E: icedax wrong-script-interpreter
/usr/share/doc/icedax-1.1.2/icedax/tracknames.pl "/usr/local/bin/perl"
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/tracknames.pl
/usr/local/bin/perl
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/tracknames.pl
perl(strict)
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/tracknames.pl
perl(IO::Handle)
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/tracknames.pl
perl(Env)
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/tracknames.pl
perl(Socket)
   These are all related.  Basically, documentation should not be executable,
and examples in documentation shouldn't pull in dependencies (which they won't
if they're not executable).  If you want to be nice, you can fix up the
/usr/local/bin/perl bits.

W: cdrkit unversioned-explicit-provides dvdrecord
W: cdrkit unversioned-explicit-obsoletes cdrecord
W: cdrkit unversioned-explicit-provides cdrecord
W: cdrkit unversioned-explicit-obsoletes cdrecord-mkisofs
W: cdrkit unversioned-explicit-provides cdrecord-mkisofs
W: cdrkit unversioned-explicit-obsoletes mkisofs
W: cdrkit unversioned-explicit-provides mkisofs
W: cdrkit unversioned-explicit-obsoletes cdrecord-cdda2wav
W: cdrkit unversioned-explicit-provides cdrecord-cdda2wav
W: cdrkit unversioned-explicit-obsoletes cdda2wav
W: cdrkit unversioned-explicit-provides cdda2wav
   These really shouldn't be unversioned; we can't predict what will happen in
the future with these package.  So it's best to obsolete cdrecord <= current
version and provide cdrecord = current version + epsilon.

ok, new spec file and src.rpm uploaded..

rpmlint only complains about:
W: cdrkit unversioned-explicit-obsoletes cdrecord-mkisofs
W: cdrkit unversioned-explicit-provides cdrecord-mkisofs
W: cdrkit unversioned-explicit-obsoletes cdrecord-cdda2wav
W: cdrkit unversioned-explicit-provides cdrecord-cdda2wav

But cdrtools had this for last releases also.

Sorry, where were they uploaded?  All I see is the 1.1.2-1 package and spec.

yes, I have overwritten the old version... no need yet to increase the release :)

(In reply to comment #9)
> yes, I have overwritten the old version... no need yet to increase the release 

Quoting:
http://www.fedoraproject.org/wiki/Packaging/FrequentlyMadeMistakes

'Increase the "Release" tag every time you upload a new package to avoid
confusion. The reviewer and other interested parties probably still have older
versions of your SRPM lying around to check what has changed between the old and
new packages; those get confused when the revision didn't change.'

kk, sry

For some reason this package stopped showing up on my bugzilla front page;
I have no idea why.  Perhaps because the status went to MODIFIED instead
of ASSIGNED?

Anyway, this builds fine now and as you say has only the four rpmlint
unversioned-explicit-provides warnings.  The thing is, these
Obsoletes/Provides pairs have been in cdrtools since FC-1.  The need for
these in order to provide a clean upgrade path has long since passed, and
they should just go away.  (Current policy is to keep such Obsoletes
around for a maximum of three releases.)

So given that, why not just remove them entirely?

Some other issues:
I note you don't use %{dist}.  I generally recommend it because it makes
it easy to maintain one specfile across multiple releases, but ultimately
it's up to you.  (Not a blocker.)

The build root should be
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

The COPYING file gets into the wodim package, but the other packages which
have essentially unrelated names don't get a copy.  This seems bothersome to
me, but I'm not sure if it's really an issue.

I can't tell what cflags are in effect at build time.  I don't see anything
that sets them, and given that the debuginfo package is busted I'm assuming
that something's not right.

Review:
* source files match upstream:
   03a4e80718704e79b50a285b0aac928a3820c5b3c1df028478aa68fe884b7d0d
   cdrkit-1.1.2.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
O dist tag is not present.
X build root is incorrect.
* license field matches the actual license.
* license is open source-compatible.
? License text included in package.
* latest version is being packaged.
* BuildRequires are proper (BR: perl is unnecessary).
? compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
X debuginfo package looks complete.
X rpmlint is silent.
* final provides and requires are sane:
  genisoimage-1.1.2-1.x86_64.rpm
   cdrecord-mkisofs
   mkisofs = 9:2.01-10.1
   genisoimage = 1.1.2-1
  =
   /bin/sh
   /usr/bin/perl
   libz.so.1()(64bit)
   perl >= 4:5.8.1
   perl(Cwd)
   perl(File::Basename)
   perl(File::Path)
   perl(Getopt::Long)
   perl(List::Util)
   perl(strict)

  icedax-1.1.2-1.x86_64.rpm
   cdda2wav = 9:2.01-10.1
   cdrecord-cdda2wav
   icedax = 1.1.2-1
  =
   /bin/sh

  wodim-1.1.2-1.x86_64.rpm
   cdrecord = 9:2.01-10.1
   dvdrecord = 0:0.1.5.1
   wodim = 1.1.2-1
  =
   libcap.so.1()(64bit)

* %check is not present; no test suite upstream.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.

I happened to come across http://fedoraproject.org/wiki/PackagingDrafts/cmake
and applied the information there to this package; it builds fine and solves
issues about compiler flags and the broken debuginfo package.

Since I was in the spec and I know that it is urgent that this package be
approved so that it can make it in for F7, I went ahead and fixed up all of the
other issues as well.  I'll attach a patch to the spec.  With this patch
applied, I would approve this package.

Created attachment 148588 [details]
Patch to fix all specfile issues.

http://people.redhat.com/harald/downloads/cdrkit/cdrkit-1.1.2-3/

OK, everything looks good:

No complaints from rpmlint.
Buildroot is good.
The license is in each package.  (And I'll reiterate: normally this wouldn't be
an issue, but these packages don't seem to be related to each other in name, and
users shouldn't have to go figuring out what other packages are built from the
SRPM just to find the license.)
The compiler flags look good, and the debuginfo package is now complete.

APPROVED

If I'm reading the account system correctly, you require sponsorship.  Go ahead
and apply for cvsextras and fedorabugs and I'll get you set up.

done... sponsored by thl 

New Package CVS Request
=======================
Package Name: cdrkit
Short Description: A collection of CD/DVD utilities
Owners: harald
Branches: 
InitialCC: 

You should close this bug if the package has been built and pushed.

Perhaps the reviewers like to comment on bug 236194, since currently,
cdrkit in Extras (!) conflicts with Core packages.

I discussed this with releng people before approving the review; cdrkit was
supposed by to be removed from core when this package was built.  Of course, I
have no control over what actually happens in core

So, what kind of comment were you fishing for here?

Everything like what has been posted to fab list and the other ticket. ;)
Thank you.