Bug 435954 - Review Request: cuetools - Utilities to work with cue and TOC files
Review Request: cuetools - Utilities to work with cue and TOC files
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Peter Lemenkov
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-03-04 12:02 EST by Paul P Komkoff Jr
Modified: 2008-05-23 15:40 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-05-23 15:40:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lemenkov: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Paul P Komkoff Jr 2008-03-04 12:02:33 EST
Spec URL: http://stingr.net/reviews/cuetools.spec
SRPM URL: http://stingr.net/reviews/cuetools-1.4.0-0.1.svn305.fc9.src.rpm
Cuetools is a set of utilities for working with cue files and TOC files.
It includes programs for conversion between the formats, file renaming based
on cue/TOC information, and track breakpoint printing.

=== cut here ===
I made some substantial changes to the package: updated to latest trunk, changed it to contain/link to a shared library to reduce binary bloat, made executables a PIE, fixed some minor issues with build system and one bounds-checking bug.
All those changes are folded into bzipped patch here: http://stingr.net/reviews/cuetools-1.3.1-svn305-fixes.patch.bz2
Upstream is being notified with the patch heading their way: http://stingr.net/reviews/cuetools-head-fixes.patch
Comment 1 Peter Lemenkov 2008-03-19 09:12:40 EDT
I'll review it.
Comment 2 Peter Lemenkov 2008-03-19 10:42:47 EDT


+ rpmlint silent on every package. 
+ The package named according to the Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package licensed with a Fedora approved license and meet the Licensing
+ The License field in the package spec file matches the actual license.

[FAIL] If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the
package must be included in %doc.

+ The spec file written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package matches the upstream source,

[petro@host-12-116 SOURCES]$ md5sum cuetools-1.3.1.tar.gz*
45575f7a1bdc6615599fa6cb49845cca  cuetools-1.3.1.tar.gz
45575f7a1bdc6615599fa6cb49845cca  cuetools-1.3.1.tar.gz.1
[petro@host-12-116 SOURCES]$

+ The package successfully compiled into binary rpms on at least one supported
architecture (i386).
+ No build dependencies.
+ Package calls ldconfig in %post and %postun with correct syntax.
+ A package doesn't create directories.
+ A package doesn't contain any duplicate files in the %files listing.
+ Permissions on files set properly.
+ A package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT.
+ Each package consistently uses macros, as described in the macros section of
Packaging Guidelines.
+ The package contains code, or permissible content.
+ No large documentation.
+ The stuff, which a package includes as %doc, does not affect the runtime of
the application.

[FAIL] Header files must be in a -devel package.

+ No static libraries.
+ Packages doesn't containing pkgconfig(.pc) files

[FAIL] If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel package.

+ Packages does NOT contain any .la libtool archives.
+ Packages doenn't containing GUI applications
+ Packages does not own files or directories already owned by other packages.  
    + At the beginning of %install, package runs rm -rf $RPM_BUILD_ROOT.
+ All filenames in rpm packages are valid UTF-8.

OK. Only 2 things are left:

* You must include COPYING file in %docs.
* We must decide what to do with devel-files (*.h and *.so)? Upstream doesn't
allow us to  install them using standard make && make install procedure, so
maybe we should leave the ./configure file intact and don't try to provide
devel-files at all (as we do now)?

All other thins looks good.

Comment 3 Paul P Komkoff Jr 2008-03-19 11:47:46 EDT
I'll just delete devel files after install (the changes to the library were to
reduce binary size, I don't want to revert them)
Comment 4 Peter Lemenkov 2008-03-20 11:01:59 EDT
Actually, I talked about *.h rather than about *.la files. 
OK, upstream developer(s) decided not to provide devel-files so we do.

Please add COPYING in %docs, create and upload updated srpm and spec-files.

BTW consider to pack doc/formats.txt - it looks useful.
Comment 5 Paul P Komkoff Jr 2008-03-26 14:21:28 EDT
spec file updated.
http://stingr.net/reviews/cuetools-1.4.0-0.2.svn305.fc9.src.rpm created.
Comment 6 Peter Lemenkov 2008-03-26 15:10:26 EDT
Comment 7 Paul P Komkoff Jr 2008-04-04 06:06:22 EDT
New Package CVS Request
Package Name: cuetools
Short Description: Utilities to work with cue and TOC files
Owners: stingray
Branches: F-7 F-8 F-9
Cvsextras Commits: yes
Comment 8 Kevin Fenzi 2008-04-04 11:42:39 EDT
cvs done.
Comment 9 Peter Lemenkov 2008-05-05 07:04:45 EDT
Ping, Paul.
Please import this package into CVS - we're waiting :)

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