Bug 554711 - Package review: libcdio
Summary: Package review: libcdio
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: libcdio
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Adrian Reber
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-01-12 13:22 UTC by Roman Rakus
Modified: 2014-01-13 00:10 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-01-22 15:10:40 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
diff of spec file (1.90 KB, patch)
2010-01-20 14:36 UTC, Roman Rakus
no flags Details | Diff

Description Roman Rakus 2010-01-12 13:22:52 UTC
All in fedora cvs.

Comment 1 Vitezslav Crhonek 2010-01-12 13:26:20 UTC
I'll take it.

Comment 2 Vitezslav Crhonek 2010-01-18 13:46:36 UTC
OK source files match upstream:
2ad1622b672ccf53a3444a0c55724d38  libcdio-0.81.tar.gz
OK source contains full URL
OK package meets naming and versioning guidelines.

BAD specfile is properly named, is cleanly written and uses macros consistently.
rpmlint warns:
libcdio.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 28, tab: line 47)
(minor issue, can be easily fixed)

OK dist tag is present.
OK build root is correct.
OK license field matches the actual license (GPLv3+).
OK license is open source-compatible. License text included in package.

BAD latest version is being packaged.
  Latest version is libcdio-0.82, why it's not packed?

OK BuildRequires are proper.
(It will build with just doxygen in BuildRequires, but other BuildRequires are not in Exceptions list, so it's probably fine to have them in the spec file.)

OK compiler flags are appropriate.
OK %clean is present.
OK package builds in mock.
OK debuginfo package looks complete.

BAD rpmlint is silent.
libcdio.src: W: mixed-use-of-spaces-and-tabs (spaces: line 28, tab: line 47)
  - can be easily fixed
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/cd-info ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/cdda-player ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libudf.so.0.0.0 ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libcdio_paranoia.so.0.0.3 ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/iso-info ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libcdio_cdda.so.0.0.5 ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libiso9660.so.7.0.0 ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/iso-read ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/cd-paranoia ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/cd-read ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/cd-drive ['/usr/lib64']
libcdio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/mmc-tool ['/usr/lib64']
  - see https://fedoraproject.org/wiki/Packaging:Guidelines#Beware_of_Rpath, this will help you remove rpath
libcdio.x86_64: W: file-not-utf8 /usr/share/doc/libcdio-0.81/THANKS
  - please convert it to the utf8

OK final provides and requires look sane.
OK %check is present and all tests pass.
OK every binary RPM package which contains shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in
%post and %postun.
OK owns the directories it creates.
OK doesn't own any directories it shouldn't.
OK no duplicates in %files.
OK file permissions are appropriate.
OK no scriptlets present.
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK headers in -devel.
OK packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability).
OK no libtool .la droppings.
OK not a GUI app.

Summary: pack latest upstream version, remove rpath, don't mix tabs and spaces in the spec file, convert THANKS file to the utf8 encoding

Comment 3 Adrian Reber 2010-01-19 12:04:24 UTC
Sorry, but I do not understand... I will try to fix the warnings and errors but why is this being reviewed? I thought merge reviews do only exist for former Fedora Core packages...

Comment 4 Roman Rakus 2010-01-19 13:49:34 UTC
I'm sorry. The right Summary should be Package Review.

Comment 5 Terje Røsten 2010-01-19 16:06:11 UTC
What is this? 

libcdio has been in Fedora for ages?

https://admin.fedoraproject.org/pkgdb/packages/name/libcdio

Is this going to RHEL or something?

If so, this is strange, you let Adrian fix the bugs so you can take
the fixes and nice and clean package to RHEL?

If there are bugs in packing, open bugs against the package and send patches.
Don't start a review.


 . Terje

Comment 6 Roman Rakus 2010-01-20 14:33:24 UTC
I'm sorry for puzzling you. I didn't see any review on libcdio, every package should have review and therefore I have created this bug. Well, the component hasn't been chosen wisely. I will change it to libcdio.

Comment 7 Roman Rakus 2010-01-20 14:34:39 UTC
I have changed spec file and local (and scratch) build looks sane.

Comment 8 Roman Rakus 2010-01-20 14:36:05 UTC
Created attachment 385691 [details]
diff of spec file

This diff changes version, removes rpath and change encoding of one file

Comment 9 Adrian Reber 2010-01-20 14:37:57 UTC
(In reply to comment #6)
> I'm sorry for puzzling you. I didn't see any review on libcdio, every package
> should have review and therefore I have created this bug. Well, the component
> hasn't been chosen wisely. I will change it to libcdio.    

Packages imported from fedora.us do not have a review in Red Hat's bugzilla. Those are in the no longer existing bugzilla.fedora.us.

Comment 10 Roman Rakus 2010-01-20 16:08:17 UTC
I see.
Looks like no objections with update to 0.82. Do you have any objections with changes? If not, I will commit changes...

Comment 11 Adrian Reber 2010-01-20 17:23:43 UTC
(In reply to comment #10)
> I see.
> Looks like no objections with update to 0.82. Do you have any objections with
> changes? If not, I will commit changes...    

Will you also do all the necessary rebuilds?

Comment 12 Adrian Reber 2010-01-22 15:10:40 UTC
I am still puzzled about this bug report. But thanks for your patch; I have applied it; libcdio has been rebuilt as well as all dependencies.


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