Bug 174320 - Review Request: gcdmaster - Gnome Audio CD mastering
Review Request: gcdmaster - Gnome Audio CD mastering
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael A. Peters
David Lawrence
http://www.poolshark.org/src/gcdmaste...
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-11-27 16:29 EST by Denis Leroy
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-03-04 17:36:08 EST
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 Denis Leroy 2005-11-27 16:29:42 EST
Spec Url: http://www.poolshark.org/src/gcdmaster.spec
SRPM Url: http://www.poolshark.org/src/gcdmaster-1.2.1-1.src.rpm

Description: GCDMaster is the Gnome GUI front-end to cdrdao, the Disk-At-Once CD mastering project. GCDMaster makes it easy to visualize and manipulate audio information before burning it onto CD. Features include: cut/copy/paste of sound samples, track marks edition and silence insertion. Writes audio CD-Rs in disc-at-once (DAO) mode allowing control over pre-gaps (length down to 0, nonzero audio data) and sub-channel information like ISRC codes and CDTEXT. GCDMaster also supports on-the-fly CD copying.
Comment 1 Michael A. Peters 2005-12-16 05:18:44 EST
Good

* Named according to PackageNamingGuidelines
* Spec file matches base package name
* Package meets packaging guidelines
* Licensed with appropriate licence (GPL), matches license in upstream package
* Spec file written in understandable americano english
* md5sum of source tarball matches upstream
* builds in mock on fc4 x86
* No un-necessary BuildRequires
* No locales
* Package owns all directories it creates
* Proper perms and %defattr()
* No libtool files packaged
* Proper desktop file - proper update-mime-database & update-desktop-database in
scriptlets

* rpmlint output:
[mpeters@utility result]$ ls *.rpm
gcdmaster-1.2.1-1.fc4.i386.rpm  gcdmaster-debuginfo-1.2.1-1.fc4.i386.rpm
gcdmaster-1.2.1-1.fc4.src.rpm
[mpeters@utility result]$ rpmlint *.rpm
E: gcdmaster zero-length /usr/share/doc/gcdmaster-1.2.1/NEWS
[mpeters@utility result]$

build pretty clean:
[mpeters@utility result]$ grep "warning" build.log 
dlg_a.c:255: warning: ignoring return value of 'sscanf', declared with attribute
warn_unused_result
dlg_a.c:262: warning: ignoring return value of 'sscanf', declared with attribute
warn_unused_result
dlg_a.c:269: warning: ignoring return value of 'sscanf', declared with attribute
warn_unused_result
AudioCDView.cc:888: warning: ignoring return value of 'int sscanf(const char*,
const char*, ...)', declared with attribute warn_unused_result
warning: Could not canonicalize hostname: utility.mpeters.local
[mpeters@utility result]$ 

Needs Work

* Please remove the INSTALL file from %doc
It is meaningless to the end user.
* Please remove NEWS file - it's empty

Suggestions

Not required, would be nice though -

Allow for a user defined macro that will build with mp3 support if user has the
needed stuff for mp3 support. IE -

rpmbuild --define 'mp3 1' --rebuild src.rpm

would try to rebuild w/ mp3 support enabled.
I seem to remember some other packages that did this in the past, I think an
audio editing app did.

-=-
At any rate - with the removal of the INSTALL and NEWS from %doc, I'll approve
Comment 2 Denis Leroy 2005-12-17 16:49:04 EST
Michael, thanks for your review.

http://www.poolshark.org/src/gcdmaster.spec
http://www.poolshark.org/src/gcdmaster-1.2.1-2.src.rpm

I'll import shortly.

I removed the explicit disabling of MP3 support so that configure will fall back
on autodetection. That means that if you compile the src.rpm with libmad-devel
installed, mp3 support will be built in (that support is "neutral" wrt
packaging, it doesn't add files to the package or anything like that).
Comment 3 Denis Leroy 2005-12-17 17:17:26 EST
http://www.poolshark.org/src/gcdmaster.spec
http://www.poolshark.org/src/gcdmaster-1.2.1-3.src.rpm

Damn i should finish my coffee before i do these things. Mp3 support is
obviously not RPM-neutral since you might accidentally tie the built RPM to
libmad.so.0 if libmad-devel happens to be installed.

I changed my mind here and implement your idea. Compiling with "rpmbuild
--define '_with_mp3 1'" will enable the mp3 support auto-detection.
Comment 4 Michael A. Peters 2005-12-17 20:24:43 EST
Approved
Comment 5 Warren Togami 2005-12-19 15:36:34 EST
> Damn i should finish my coffee before i do these things. Mp3 support is
> obviously not RPM-neutral since you might accidentally tie the built RPM to
> libmad.so.0 if libmad-devel happens to be installed.

This is not an issue for Extras, because each build happens from a minimal
buildroot with only listed deps installed.
Comment 6 Michael Schwendt 2006-01-12 09:38:11 EST
> --define '_with_mp3 1'" will enable the mp3 support auto-detection.

Better: pass "--with mp3" as an argument to rpmbuild. It will set
the _with_mp3 macro to 1.
Comment 7 Denis Leroy 2006-03-04 17:36:08 EST
Built :-)

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