Bug 203964 - Review Request: libburn - Library for reading, mastering and writing optical discs
Review Request: libburn - Library for reading, mastering and writing optical ...
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jarod Wilson
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-08-24 14:42 EDT by Jesse Keating
Modified: 2015-05-22 10:06 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-27 22:59:24 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Jesse Keating 2006-08-24 14:42:15 EDT
Spec URL: http://people.redhat.com/jkeating/extras/libburn/libburn.spec
SRPM URL: http://people.redhat.com/jkeating/extras/libburn/libburn-0.2-1.20060823svn.src.rpm
Description: 

Libburn is an open-source library for reading, mastering and writing
optical discs. For now this means only CD-R and CD-RW.

The project comprises of several more or less interdependent parts which
together strive to be a usable foundation for application development.
These are libraries, language bindings, and middleware binaries which emulate
classical (and valuable) Linux tools.

rpmlint warns about no docs and mixed use of spaces and tabs.  Those could be ignored IMHO.

rpmlint also errors about shellbang stuff in the debuginfo package and I think thats because the upstream files have execute permissions on them or some such.  I think I can fix that in my svn checkout, and will need to make sure that the upstream released tarballs have correct perms.
Comment 2 Jarod Wilson 2006-08-27 00:10:36 EDT
Okay, first run through the spec...

FIXME's:

1) per the pre-release package naming conventions at http://fedoraproject.org/wiki/Packaging/
NamingGuidelines, I believe the Release: ought to be 0.1.20060823svn%{?dist}

2) The tarball contains %doc-worthy material. These files need to be added to one of the packages: 
AUTHORS COPYING COPYRIGHT README, probably just to the main libburn package.

3) the 'mixed spaces and tabs' error can be remedied by replacing the tabs in cdrskin's Version: and 
Release: lines with spaces.

4) the rpmlint shellbang warnings appear to be due to everything in libisofs/ having execute 
permissions, an extra line in the spec can take care of it. Doesn't matter if its done in the spec or by 
hand in this case, since you created the svn tarball, but I think its ultimately better to do it in the spec, 
just in case the upstream 0.2 release still has execute bits set, so you don't have to modify an upstream 
release tarball in the future. Certainly holler at them to get that fixed though...

5) the Requires: in cdrskin needs to be corrected, it'll currently result in cdrskin requiring libburn 0.1.4.

6) I'd leave off the Release: field in cdrskin, since its redundant.

7) might want to use %{mainver} in more places, to avoid winding up with conflicting version numbers 
here and there (that different version on cdrskin certainly does make life interesting...).

With some minor spec tweaks, I've got the rpmlint spew down to a minimum now, all ignorable 
warnings:
$ rpmlint /build/RPMS/x86_64/*0.1.20060823*.rpm
W: cdrskin no-documentation
W: libburn-devel no-documentation
W: libisofs no-documentation
W: libisofs-devel no-documentation

Spec diff (not directly applicable, since the tabs copied as spaces, just provided "for your viewing 
pleasure ;):

--- libburn.spec.orig   2006-08-24 14:38:14.000000000 -0400
+++ libburn.spec        2006-08-27 00:06:16.000000000 -0400
@@ -2,8 +2,8 @@
 # define this as version gets overridden by the subpackage with its own version
 
 Name:           libburn
-Version:        0.2
-Release:        1.20060823svn%{?dist}
+Version:        %{mainver}
+Release:        0.1.20060823svn%{?dist}
 Summary:        Library for reading, mastering and writing optical discs
 
 Group:          System Environment/Libraries
@@ -60,9 +60,8 @@
 %package -n     cdrskin
 Summary:        Limited cdrecord compatibility wrapper to ease migration to libburn
 Group:          Applications/Multimedia
-Version:       0.1.4
-Release:       1.20060823svn%{?dist}
-Requires:       %{name} = %{version}-%{release}
+Version:        0.1.4
+Requires:       %{name} = %{mainver}-%{release}
 
 %description -n cdrskin
 A limited cdrecord compatibility wrapper which allows to use some libburn 
@@ -71,6 +70,8 @@
 
 %prep
 %setup -n %{name}-%{mainver}svn -q
+# These shouldn't be executable...
+chmod -x libisofs/*
 
 
 %build
@@ -102,7 +103,7 @@
 
 %files
 %defattr(-,root,root,-)
-%doc
+%doc AUTHORS COPYING COPYRIGHT README
 %{_libdir}/%{name}*.so.*
 
 %files devel
@@ -131,5 +132,5 @@
 
 
 %changelog
-* Wed Aug 23 2006 Jesse Keating <jkeating@redhat.com> - 0.2-1.20060823svn
+* Wed Aug 23 2006 Jesse Keating <jkeating@redhat.com> - 0.2-0.1.20060823svn
 - Initial package for review
Comment 3 Jesse Keating 2006-08-27 11:02:09 EDT
So, this isn't actually a pre-release.  These peices of software have been
released in the past, on other websites, which is why this is a svn snapshot on
top of the old release, thus the 1.<snapshot> naming scheme.

Docs added, tabs fixed, version requires fixed, and file perms changed.  I want
to only use mainver where its needed, as it should go away soon when the release
numbers sync up.  Then I only have to change it a few places.

http://people.redhat.com/jkeating/extras/libburn/libburn.spec
http://people.redhat.com/jkeating/extras/libburn/libburn-0.2-2.20060823svn.fc6.src.rpm
Comment 4 Mario Đanić 2006-08-27 14:50:48 EDT
Hello,

Sorry for making your life bitter with different cdrskin version number.
Current status is that while cdrskin may make a major (or minor) step in a
period of time valuable enough to increase version number, such thing doesn't
have to be true in the same period for libburn library (and cdrskin depends on
libburn). The most obvious reason for this is that cdrskin is still not using
all of libburn options, and that libburn development is much more complicated
then the one of cdrskin itself, altought even cdrskin development isn't without
it's problems.

Kind regards,
Mario
Comment 5 Jarod Wilson 2006-08-27 15:15:10 EDT
(In reply to comment #3)
> So, this isn't actually a pre-release.  These peices of software have been
> released in the past, on other websites, which is why this is a svn snapshot on
> top of the old release, thus the 1.<snapshot> naming scheme.

Ah, okay. Now that I think about it, I do recall seeing some mention of a 0_2_1 tag in some merge 
script in the source, so that makes sense. Works for me.

> Docs added, tabs fixed, version requires fixed, and file perms changed.

Both Makefile and Makefile.am still have unnecessary execute permissions, but rpmlint doesn't seem to 
care, so good enough. Ah, and your changelog entry refers to cdrtools instead of cdrskin.

> I want
> to only use mainver where its needed, as it should go away soon when the release
> numbers sync up.  Then I only have to change it a few places.

Works for me, was only an idea, shouldn't have been under the "FIXME's". Really only needed it in the 
Requires: for cdrskin, looks good to me.

On with the formal review...

* package meets naming and packaging guidelines
* specfile is properly named, is cleanly written and uses macros consistently
* dist tag is present
* build root is correct
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
* license field matches the actual license
* license is open source-compatible (GPL), license text included in package
* source files match upstream:
      n/a, svn snapshot
* latest version is being packaged
* BuildRequires are proper
* package builds in mock (rawhide x86_64)
* rpmlint only spits acceptable warnings (W: about lack of docs in some packages)
* final provides and requires are sane:
    cdrskin-0.1.4-2.20060823svn.fc6.x86_64.rpm
    cdrskin = 0.1.4-2.20060823svn.fc6
    =
    libburn = 0.2-2.20060823svn.fc6

    libburn-0.2-2.20060823svn.fc6.x86_64.rpm
    libburn.so.1()(64bit)  
    libburn = 0.2-2.20060823svn.fc6
    =
    /sbin/ldconfig  
    /sbin/ldconfig  
    libburn.so.1()(64bit)  

    libburn-devel-0.2-2.20060823svn.fc6.x86_64.rpm
    libburn-devel = 0.2-2.20060823svn.fc6
    =
    libburn = 0.2-2.20060823svn.fc6
    libburn.so.1()(64bit)  
    pkgconfig  

    libisofs-0.2-2.20060823svn.fc6.x86_64.rpm
    libisofs.so.1()(64bit)  
    libisofs = 0.2-2.20060823svn.fc6
    =
    /sbin/ldconfig  
    /sbin/ldconfig  
    libisofs.so.1()(64bit)  

    libisofs-devel-0.2-2.20060823svn.fc6.x86_64.rpm
    libisofs-devel = 0.2-2.20060823svn.fc6
    =
    libisofs = 0.2-2.20060823svn.fc6
    libisofs.so.1()(64bit)  
    pkgconfig  

* shared libraries are present, properly named (unversioned symlinks in -devel packages, versioned libs 
in the right places, with appropriate calls to ldconfig in scriptlets)
* package is not relocatable
* owns the directories it creates
* doesn't own any directories it shouldn't

* FIXME - no duplicates in %files
   libisofs-devel and libburn-devel both install the same include files, /usr/include/libburn/*

* file permissions are appropriate
* %clean is present
* %check is present and all tests pass:
        n/a, there is a 'make check' target, but it doesn't appear to do anything useful...
* scriptlets present -- proper ldconfig's
* code, not content
* documentation is small, so no -docs subpackage is necessary
* %docs are not necessary for the proper functioning of the package

* FIXME - headers properly in -devel packages, but currently duplicated

* pkgconfig files in -devel packages, properly Requires: pkgconfig
* no libtool .la droppings
* not a GUI app
* not a web app


Looks like the only thing to fix is the duplication of the libburn header files. I presume just drop them 
from libisofs-devel.
Comment 6 Jarod Wilson 2006-08-27 15:18:10 EDT
(In reply to comment #4)
> Sorry for making your life bitter with different cdrskin version number.

Doesn't make my life bitter, just makes it a little wonky to package.

> Current status is that while cdrskin may make a major (or minor) step in a
> period of time valuable enough to increase version number, such thing doesn't
> have to be true in the same period for libburn library (and cdrskin depends on
> libburn). The most obvious reason for this is that cdrskin is still not using
> all of libburn options, and that libburn development is much more complicated
> then the one of cdrskin itself, altought even cdrskin development isn't without
> it's problems.

I would say that they either really ought to be in separate tarballs, each with their own version number, 
or the version numbers sync'd up. Two differently versioned components in a single tarball is just plain 
confusing.
Comment 7 Jarod Wilson 2006-08-27 15:20:44 EDT
(In reply to comment #5)
> ... Ah, and your changelog entry refers to cdrtools instead of cdrskin.
[...]
> Looks like the only thing to fix is the duplication of the libburn header files. I presume just drop them 
> from libisofs-devel.

Well, not the only thing, I'd fix the ref in the changelog too. :)

/me goes off to play video games with son...
Comment 8 Jesse Keating 2006-08-27 17:06:58 EDT
* Sun Aug 27 2006 Jesse Keating <jkeating@redhat.com> - 0.2-3.20060823svn
- don't install dupe headers in -devel packages
- libisofs requires libburn devel for directory ownership

http://people.redhat.com/jkeating/extras/libburn/libburn.spec
http://people.redhat.com/jkeating/extras/libburn/libburn-0.2-3.20060823svn.fc6.src.rpm
Comment 9 Jarod Wilson 2006-08-27 22:21:37 EDT
(In reply to comment #8)
> - libisofs requires libburn devel for directory ownership

Personally, I don't see a huge problem with multiple packages laying claim to a
directory, not sure what the official stance on this is. It'd allow you to only
install libisofs-devel without having to install libburn-devel, but how much we
really care about being able to do that, I dunno. One minor thing, you used
%mainver for that added Requires:, right below another Requires: that uses
%version. No biggie though, everything else is golden.

My name is Jarod Wilson, and I APPROVE this package. :)
Comment 10 Jesse Keating 2006-08-27 22:59:24 EDT
Wheee!  I was thinking that cdrskin version had been set by then and would be
used.  Silly me.  This extra version stuff is fun (:

I don't want there to be two packages owning the libburn include dir, perhaps
upstream could be convinced at some point to drop libisofs.h into a libisofs
folder instead of a libburn folder... (hint hint (: )

Package imported and built for development.  Branch requested for FC-5.
Comment 11 Jesse Keating 2007-03-13 13:07:27 EDT
Please add 'denis' as a co-owner of this module.
Comment 12 Warren Togami 2007-03-13 17:54:29 EDT
Could you please provide Bugzilla names instead of FAS names?  We don't
currently use the FAS names there yet. =(
Comment 13 Jesse Keating 2007-03-13 20:59:00 EDT
denis@poolshark.org
Comment 15 Robert Scheck 2015-05-19 19:04:53 EDT
Package Change Request
======================
Package Name: libburn
New Branches: epel7
Owners: robert
Comment 16 Gwyn Ciesla 2015-05-20 07:01:14 EDT
Comments from Fedora maintainers?
Comment 17 Robert Scheck 2015-05-20 09:53:20 EDT
Frantisek, you are the Fedora maintainer. Preferences? I would take ownership
if you are not interested. Just let us know here shortly that we can procceed
quickly - thank you.
Comment 18 Frantisek Kluknavsky 2015-05-22 09:51:49 EDT
I am not completely sure what is the question. Whether to include libburn in epel7?
I maintain libburn also in rhel7. Unfortunately it will probably get no update whatsoever for the whole lifetime, slowly becoming obsolete. If you want to keep up-to-date version of libburn in epel7, feel free to do it.
Comment 19 Robert Scheck 2015-05-22 10:06:05 EDT
Given that I was not able to locate libburn in RHEL 7 via "yum search" or via
the Red Hat portal (RPM search), I thought the package would not be in RHEL 7
anymore and wanted to branch it (comment #15). However for libisofs I was now
notified that the package is already in RHEL 7 (via a Bodhi comment!) and thus
I filed https://fedorahosted.org/rel-eng/ticket/6183 to avoid situations like
this hopefully in the future again.

Sorry for all the noise, libburn is not possible for EPEL 7 because the goal
of EPEL is to (usually) not replace RHEL packages.

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