Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 224365 - Review Request: cdrkit - cdrtools replacement
Summary: Review Request: cdrkit - cdrtools replacement
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
: 224364 (view as bug list)
Depends On:
Blocks: FC-ACCEPT 214825
TreeView+ depends on / blocked
Reported: 2007-01-25 13:36 UTC by Harald Hoyer
Modified: 2007-11-30 22:11 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2007-03-19 07:59:29 UTC
tibbs: fedora-review+
wtogami: fedora-cvs+

Attachments (Terms of Use)
Patch to fix all specfile issues. (deleted)
2007-02-22 16:27 UTC, Jason Tibbitts
no flags Details | Diff

Description Harald Hoyer 2007-01-25 13:36:06 UTC
Spec URL:
Description: Command line compatible replacement of cdrtools/cdrecord/mkisofs/cdda2wav which obsoletes the latter.

Comment 1 Harald Hoyer 2007-01-25 13:37:42 UTC
*** Bug 224364 has been marked as a duplicate of this bug. ***

Comment 2 Harald Hoyer 2007-01-25 13:40:12 UTC
problem: requires cmake from extras, which itsself requires xmlrpc-c (extras)

Comment 3 Jason Tibbitts 2007-01-25 16:10:52 UTC
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.

Comment 4 Harald Hoyer 2007-01-25 17:57:18 UTC
yes, I want :)

Comment 5 Jason Tibbitts 2007-01-26 17:50:38 UTC
It does seem that moving to cdrkit is in the plan for F7.  I'll review this.

Comment 6 Jason Tibbitts 2007-01-26 18:32:02 UTC
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

[ 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
/builddir/build/BUILD/cdrkit-1.1.2/genisoimage/jte.c:581: error: 'z_stream'
undeclared (first use in this function)

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/ "/usr/local/bin/perl"
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/
W: icedax doc-file-dependency /usr/share/doc/icedax-1.1.2/icedax/
   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.

Comment 7 Harald Hoyer 2007-01-29 09:58:24 UTC
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.

Comment 8 Jason Tibbitts 2007-01-29 15:26:23 UTC
Sorry, where were they uploaded?  All I see is the 1.1.2-1 package and spec.

Comment 9 Harald Hoyer 2007-01-30 08:51:44 UTC
yes, I have overwritten the old version... no need yet to increase the release :)

Comment 10 Thorsten Leemhuis 2007-01-30 09:32:48 UTC
(In reply to comment #9)
> yes, I have overwritten the old version... no need yet to increase the release 


'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.'

Comment 11 Harald Hoyer 2007-01-30 10:44:33 UTC
kk, sry

Comment 12 Jason Tibbitts 2007-02-17 19:24:21 UTC
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

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.

* source files match upstream:
* 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:
   mkisofs = 9:2.01-10.1
   genisoimage = 1.1.2-1
   perl >= 4:5.8.1

   cdda2wav = 9:2.01-10.1
   icedax = 1.1.2-1

   cdrecord = 9:2.01-10.1
   dvdrecord = 0:
   wodim = 1.1.2-1

* %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.

Comment 13 Jason Tibbitts 2007-02-22 16:20:50 UTC
I happened to come across
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.

Comment 14 Jason Tibbitts 2007-02-22 16:27:20 UTC
Created attachment 148588 [details]
Patch to fix all specfile issues.

Comment 16 Jason Tibbitts 2007-02-27 19:27:01 UTC
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.


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.

Comment 17 Harald Hoyer 2007-02-28 12:12:27 UTC
done... sponsored by thl 

Comment 18 Harald Hoyer 2007-02-28 12:17:59 UTC
New Package CVS Request
Package Name: cdrkit
Short Description: A collection of CD/DVD utilities

Comment 19 Jason Tibbitts 2007-03-17 01:19:13 UTC
You should close this bug if the package has been built and pushed.

Comment 20 Michael Schwendt 2007-04-16 14:32:16 UTC
Perhaps the reviewers like to comment on bug 236194, since currently,
cdrkit in Extras (!) conflicts with Core packages.

Comment 21 Jason Tibbitts 2007-04-16 14:51:53 UTC
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?

Comment 22 Michael Schwendt 2007-04-18 14:38:55 UTC
Everything like what has been posted to fab list and the other ticket. ;)
Thank you.

Note You need to log in before you can comment on or make changes to this bug.