Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 203964
Summary: | Review Request: libburn - Library for reading, mastering and writing optical discs | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jesse Keating <jkeating> |
Component: | Package Review | Assignee: | Jarod Wilson <jarod> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | dcantrell, fkluknav, lemenkov, mario, redhat-bugzilla |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-08-28 02:59:24 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Jesse Keating
2006-08-24 18:42:15 UTC
Herm, proper url: http://people.redhat.com/jkeating/extras/libburn/libburn-0.2-1.20060823svn.fc6.src.rpm 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> - 0.2-1.20060823svn +* Wed Aug 23 2006 Jesse Keating <jkeating> - 0.2-0.1.20060823svn - Initial package for review 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 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 (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. (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. (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... * Sun Aug 27 2006 Jesse Keating <jkeating> - 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 (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. :) 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. Please add 'denis' as a co-owner of this module. Could you please provide Bugzilla names instead of FAS names? We don't currently use the FAS names there yet. =( denis Package Change Request ====================== Package Name: libburn New Branches: epel7 Owners: robert Comments from Fedora maintainers? 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. 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. 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. |